Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Add topotest to validate the sharing of an SRv6 locator among multiple protocols #15679

Merged
merged 81 commits into from
Sep 8, 2024

Conversation

cscarpitta
Copy link
Contributor

PR #15604 introduces the SRv6 SID Manager, a zebra component responsible for SID allocation/management. The SRv6 SID Manager exposes a SID allocation/release APIs, allowing clients to request and release an SRv6 SID. The SID manager allows the SRv6 Locator to be shared among many clients such as IS-IS, BGP and others.

This PR adds a new topotest to validate the sharing of an SRv6 locator among multiple clients

It sets up a mixed BGP and IS-IS topology and verifies that the locator can be shared by BGP and IS-IS, and that both daemons are able to request and obtain SIDs from the same locator.

@frrbot frrbot bot added the tests Topotests, make check, etc label Apr 3, 2024
@github-actions github-actions bot added the master label Apr 3, 2024
@cscarpitta cscarpitta changed the title tests: Add topotest to validate the sharing of an SRv6 locator among multiple clients tests: Add topotest to validate the sharing of an SRv6 locator among multiple protocols Apr 3, 2024
@riw777 riw777 self-requested a review April 9, 2024 14:29
@cscarpitta cscarpitta force-pushed the test-srv6-sid-manager branch from ce0d1ca to 77b8133 Compare May 3, 2024 14:59
@cscarpitta cscarpitta force-pushed the test-srv6-sid-manager branch 2 times, most recently from faa5c70 to af9bedf Compare May 3, 2024 15:04
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@cscarpitta cscarpitta force-pushed the test-srv6-sid-manager branch from af9bedf to 480c9d4 Compare June 7, 2024 10:35
@riw777
Copy link
Member

riw777 commented Jun 11, 2024

looks like the docker build is failing ...

Copy link
Contributor

@dmytroshytyi-6WIND dmytroshytyi-6WIND left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

Generally SRv6 uSID test looks ok. 1 minor comment.

Resume: This PR demonstrates the advantage to use same SRv6 locator for BGP SRv6 and ISIS SRv6 on the same node.

I retain the next from this PR:

1. BGP SRv6 session is established between rt1 and rt6. 
2. ISIS SRv6 on rt1,rt2,rt3,rt4,rt5,rt6.
3. Same locator shared between BGP SRv6 and ISIS SRv6 

I assume that the failed status of PR is related to the fact this PR waits before BGP + ISIS extension are going to be merged? Please confirm if the case or provide a fix if there is an issue.

no bgp default ipv4-unicast
!
address-family ipv6 unicast
sid vpn export 65024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment: Why this func value? Is there is anything blocking us to use 65324? The value 65024 might be "out-of-range" for default value range of func (starting from "0xff00" if I remember correctly) particular format uncompressed-f4024? It might be a good thing to avoid confusion in case when user switches the uncompressed-f4024 and consequently there is a risk that related func are failed to allocate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tu further evolve this point: user should be notified directly if (s)he types the value of out-of-range func.

@riw777
Copy link
Member

riw777 commented Jul 9, 2024

ci:rerun

@riw777
Copy link
Member

riw777 commented Jul 16, 2024

still failing just on docker ... ci:rerun

@riw777
Copy link
Member

riw777 commented Jul 23, 2024

ci:rerun

@riw777
Copy link
Member

riw777 commented Jul 30, 2024

I don't know if this is a real problem or not:

self = <lib.topotest.Router object at 0x7f3579180e20>
p = <Popen: returncode: 2 args: ['/usr/bin/nsenter', '--mount=/proc/116/ns/mnt',...>
c = ['/usr/sbin/ip', 'link', 'add', 'vrf1', 'type', 'vrf', ...]
ac = ['/usr/bin/nsenter', '--mount=/proc/116/ns/mnt', '--net=/proc/116/ns/net', '--uts=/proc/116/ns/uts', '-F', '/usr/sbin/ip', ...]
o = '', e = 'Error: Unknown device type.\n', raises = True, warn = True

Can y'all look at this to see what's going on?

@dmytroshytyi-6WIND
Copy link
Contributor

Hi @riw777,

I don't know if this is a real problem or not:

self = <lib.topotest.Router object at 0x7f3579180e20>
p = <Popen: returncode: 2 args: ['/usr/bin/nsenter', '--mount=/proc/116/ns/mnt',...>
c = ['/usr/sbin/ip', 'link', 'add', 'vrf1', 'type', 'vrf', ...]
ac = ['/usr/bin/nsenter', '--mount=/proc/116/ns/mnt', '--net=/proc/116/ns/net', '--uts=/proc/116/ns/uts', '-F', '/usr/sbin/ip', ...]
o = '', e = 'Error: Unknown device type.\n', raises = True, warn = True

Can y'all look at this to see what's going on?

After brief check, it looks like this error starts in the bfd_vrflite_topo1 context.

The command itself looks ok ('/usr/sbin/ip', 'link', 'add', 'vrf1', 'type', 'vrf', 'table', '10')

The error might be seen in case, when vrf module is not inserted into the kernel.

@riw777
Copy link
Member

riw777 commented Aug 7, 2024

ci:rerun
current failure isn't related

@riw777
Copy link
Member

riw777 commented Aug 27, 2024

ci:rerun

Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta cscarpitta force-pushed the test-srv6-sid-manager branch from 480c9d4 to c7f4753 Compare September 7, 2024 06:33
@cscarpitta
Copy link
Contributor Author

ci:rerun

@riw777 riw777 merged commit 80d3a2b into FRRouting:master Sep 8, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master size/XXL tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants