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

staticd: Fix SRv6 SID installation and deletion #18064

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

cscarpitta
Copy link
Contributor

The SRv6 support in staticd (PR #16894) does not set the correct SID parameters (block length, node length, function length).

This PR fixes the issue and computes the correct parameters.

The bug was reported by SONiC testing.

Copy link

@ahsalam ahsalam left a comment

Choose a reason for hiding this comment

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

The bug fix has been tested in SONiC and solves the problem. Thanks!

@mjstapp
Copy link
Contributor

mjstapp commented Feb 7, 2025

So if I'm reading this correctly there is no topotest that actually validates this path? Could we please add a test, so we don't lose the correct functionality?

@Jafaral
Copy link
Member

Jafaral commented Feb 8, 2025

@Mergifyio backport dev/10.3 stable/10.2

Copy link

mergify bot commented Feb 8, 2025

backport dev/10.3 stable/10.2

✅ Backports have been created

The SRv6 support in staticd (PR FRRouting#16894) does not set the correct SID
parameters (block length, node length, function length).

This commit fixes the issue and computes the correct parameters.

Signed-off-by: Carmine Scarpitta <[email protected]>
The `show ipv6 route json` command displays the IPv6 routing table in
JSON format, including SRv6 SIDs. For each SRv6 SID, it provides
behavior and SID attributes. However, it does not include the SID
structure.

This commit adds the SID structure to the SRv6 SID JSON output.

Signed-off-by: Carmine Scarpitta <[email protected]>
The `static_srv6_sids` topotest verifies that staticd correctly
programs the SIDs in the zebra RIB. Currently, the topotest only
validates the programmed behavior and SID attributes.

This commit extends the topotest to also validate the SID structure.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta cscarpitta force-pushed the fix/fix_srv6_sid_parameters branch from 86e435b to a6d02fe Compare February 8, 2025 21:32
@frrbot frrbot bot added libfrr tests Topotests, make check, etc labels Feb 8, 2025
@github-actions github-actions bot added size/L and removed size/M labels Feb 8, 2025
@cscarpitta
Copy link
Contributor Author

So if I'm reading this correctly there is no topotest that actually validates this path? Could we please add a test, so we don't lose the correct functionality?

@mjstapp Thanks for the suggestion.

I extended the static_srv6_sids topotest to validate the SID structure (block length, node length, function length, argument length).

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

@donaldsharp donaldsharp merged commit 8a661e5 into FRRouting:master Feb 13, 2025
17 checks passed
Jafaral added a commit that referenced this pull request Feb 14, 2025
staticd: Fix SRv6 SID installation and deletion (backport #18064)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants