Skip to content

staticd: Avoid requesting SRv6 sid from zebra when loc and sid block dont match#18580

Merged
cscarpitta merged 1 commit intoFRRouting:masterfrom
raja-rajasekar:rajasekarr/check_sid_loc_block_beforehand
Apr 8, 2025
Merged

staticd: Avoid requesting SRv6 sid from zebra when loc and sid block dont match#18580
cscarpitta merged 1 commit intoFRRouting:masterfrom
raja-rajasekar:rajasekarr/check_sid_loc_block_beforehand

Conversation

@raja-rajasekar
Copy link
Copy Markdown
Contributor

@raja-rajasekar raja-rajasekar commented Apr 4, 2025

Currently, when the locator block and sid block differs, staticd would still go ahead and request zebra to allocate the SID which it does if there is atleast one match (from any locators).

Only when staticd tries to install the route, it sees that the locator block and sid block are different and avoids installing the route.

Fix:
Check if the locator block and sid block match before even requesting Zebra to allocate one.

@frrbot frrbot bot added the staticd label Apr 4, 2025
@raja-rajasekar
Copy link
Copy Markdown
Contributor Author

raja-rajasekar commented Apr 4, 2025 •
edited
Loading

Testing:

Config:

 srv6
  static-sids
   sid fcbb:bbbb:1:fe10::/64 locator MAIN behavior uDT4 vrf Vrf10
   sid fcbb:bbbb:1:fe30::/64 locator MAIN behavior uDT46 vrf Vrf30
   sid fcbb:bbbb:1:fe40::/64 locator MAIN behavior uA interface sr0 nexthop 2001::2
   sid fcbb:bbbb:1:fe20::/64 locator MAIN behavior uDT6 vrf Vrf20
   sid fcbb:bbbb:1::/48 locator MAIN1 behavior uN
  exit
  !
 exit
 !
 srv6
  locators
   locator MAIN
    prefix fcbb:bbbb:1::/48 block-len 32 node-len 16 func-bits 16
   exit
   !
   locator MAIN1
    prefix fcbb:1234:1::/48 block-len 32 node-len 16 func-bits 16
   exit

Without Fix:

root@rajasekarr:/tmp/topotests/static_srv6_sids.test_static_srv6_sids/r1# cat staticd.log 
2025/04/04 19:48:55.468443 STATIC: [J9AAH-6B3EE] static_zebra_srv6_sid_notify: received SRv6 SID notify: ctx End USP sid_value fcbb:bbbb:1:: sid_func 0 note ZAPI_SRV6_SID_ALLOCATED >>>>>>>>>>>>
2025/04/04 19:48:55.468452 STATIC: [KPNK8-A9B1R] static_zebra_srv6_sid_notify: SRv6 SID fcbb:bbbb:1:: End USP ALLOCATED
2025/04/04 19:48:55.468454 STATIC: [Q40TD-ZMCDR] SID block fcbb:bbbb::/32 does not match locator block fcbb:1234::/32
root@rajasekarr:/tmp/topotests/static_srv6_sids.test_static_srv6_sids/r1# cat zebra.log 
2025/04/04 19:48:55.468166 ZEBRA: [MZYPC-GBDGR] srv6_manager_get_sid_internal: getting SRv6 SID for ctx End USP, sid_value=fcbb:bbbb:1::, locator_name=MAIN1
2025/04/04 19:48:55.468185 ZEBRA: [XMBTQ-GE6EY] get_srv6_sid: received SRv6 SID alloc request: SID ctx End USP (fcbb:bbbb:1::), mode=explicit
2025/04/04 19:48:55.468189 ZEBRA: [GKEYQ-X8WG5] get_srv6_sid_explicit: allocated explicit SRv6 SID fcbb:bbbb:1:: for context End USP
2025/04/04 19:48:55.468189 ZEBRA: [XBBYD-T1Q7P] srv6_manager_get_sid_internal: got new SRv6 SID for ctx End USP: sid_value=fcbb:bbbb:1:: (func=0) (proto=4, instance=0, sessionId=0), notifying all clients

With Fix:

2025/04/04 19:54:26.333391 STATIC: [M7Q4P-46WDR] vty[5]@(config)# do write terminal
2025/04/04 19:54:32.878156 STATIC: [Q40TD-ZMCDR] SID block fcbb:bbbb::/32 does not match locator block fcbb:1234::/32 >>>>>>>>>
root@rajasekarr:/tmp/topotests/static_srv6_sids.test_static_srv6_sids/r1# cat zebra.log 
2025/04/04 19:54:26.332057 ZEBRA: [M7Q4P-46WDR] vty[5]@(config-srv6)# do write terminal
root@rajasekarr:/tmp/topotests/static_srv6_sids.test_static_srv6_sids/r1# 

When locator is later configured in zebra
root@rajasekarr:/tmp/topotests/static_srv6_sids.test_static_srv6_sids/r1# cat staticd.log 
2025/04/04 19:55:27.040383 STATIC: [N2CVB-Z7S3N] routing_control_plane_protocols_control_plane_protocol_staticd_segment_routing_srv6_local_sids_sid_apply_finish: Locator MAIN2 not found, trying to get locator information from zebra
2025/04/04 19:56:02.367889 STATIC: [TW2SF-0H2KA] static_zebra_process_srv6_locator_delete: SRv6 locator deleted in zebra: name MAIN2, prefix fcbb:2345:1::/48, block_len 32, node_len 16, func_len 16, arg_len 0
2025/04/04 19:56:02.367963 STATIC: [GQ2NH-J8Q20] static_zebra_process_srv6_locator_internal: Received SRv6 locator MAIN2 fcbb:2345:1::/48, loc-block-len=32, loc-node-len=16 func-len=16, arg-len=0
2025/04/04 19:56:02.367967 STATIC: [JBX66-Y8J43] static_zebra_process_srv6_locator_internal: SRv6 locator (locator MAIN2, prefix fcbb:2345:1::/48) set
2025/04/04 19:56:02.367976 STATIC: [Q40TD-ZMCDR] SID block fcbb:bbbb::/32 does not match locator block fcbb:2345::/32 >>>>>>>>>

@raja-rajasekar raja-rajasekar force-pushed the rajasekarr/check_sid_loc_block_beforehand branch from 6ee834a to 61c0eb6 Compare April 4, 2025 22:04
@cscarpitta cscarpitta self-requested a review April 5, 2025 14:37
cscarpitta
cscarpitta reviewed Apr 5, 2025
View reviewed changes
Comment thread tests/topotests/static_srv6_sids/r1/frr.conf Outdated
sid fcbb:bbbb:1:fe20::/64 locator MAIN behavior uDT6 vrf Vrf20
sid fcbb:bbbb:1:fe30::/64 locator MAIN behavior uDT46 vrf Vrf30
sid fcbb:bbbb:1:fe40::/64 locator MAIN behavior uA interface sr0 nexthop 2001::2
sid fcbb:bbbb:1:fe50::/64 locator MAIN1 behavior uA interface sr0 nexthop 2001::3
Copy link
Copy Markdown
Contributor

@cscarpitta cscarpitta Apr 5, 2025

Choose a reason for hiding this comment

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

Can we keep frr.conf unchanged and move this invalid SID config to a new test case in https://github.com/FRRouting/frr/blob/master/tests/topotests/static_srv6_sids/test_static_srv6_sids.py?

It could be something called called test_srv6_static_sids_wrong_sid_block, and you could add a comment or doc string to make it clear that the purpose of the test is to verify how FRR behaves when the user provides an invalid configuration.

The reason behind this is that people often refer to frr.conf to figure out how to configure features in FRR. Having invalid configuration in frr.conf can lead to confusion.

Copy link
Copy Markdown
Contributor Author

@raja-rajasekar raja-rajasekar Apr 7, 2025

Choose a reason for hiding this comment

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

Done

@cscarpitta
Copy link
Copy Markdown
Contributor

cscarpitta commented Apr 5, 2025

https://github.com/Mergifyio backport stable/10.3

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 5, 2025

backport stable/10.3

☑️ Nothing to do

Details
  • merged [📌 backport requirement]

@raja-rajasekar raja-rajasekar force-pushed the rajasekarr/check_sid_loc_block_beforehand branch from 61c0eb6 to 83ce325 Compare April 7, 2025 17:33
@raja-rajasekar
staticd: Avoid requesting SRv6 sid from zebra when loc and sid block …
dbd9fed 
…dont match

Currently, when the locator block and sid block differs, staticd would
still go ahead and request zebra to allocate the SID which it does if
there is atleast one match (from any locators).

Only when staticd tries to install the route, it sees that the locator
block and sid block are different and avoids installing the route.

Fix:
Check if the locator block and sid block match before even requesting
Zebra to allocate one.

Signed-off-by: Rajasekar Raja <[email protected]>
@raja-rajasekar raja-rajasekar force-pushed the rajasekarr/check_sid_loc_block_beforehand branch from 83ce325 to dbd9fed Compare April 7, 2025 17:34
@cscarpitta cscarpitta merged commit 46a5265 into FRRouting:master Apr 8, 2025
13 checks passed
@cscarpitta
Copy link
Copy Markdown
Contributor

cscarpitta commented Apr 8, 2025

@Mergifyio backport stable/10.3

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 8, 2025 •
edited
Loading

backport stable/10.3

✅ Backports have been created

Details

@mergify mergify bot mentioned this pull request Apr 8, 2025
Closed
@cscarpitta cscarpitta mentioned this pull request Apr 8, 2025
Merged
Jafaral added a commit that referenced this pull request Apr 8, 2025
@Jafaral
Merge pull request #18609 from cscarpitta/fix/backport_check_sid_loc_…
18716fe 
…block_beforehand

staticd: Avoid requesting SRv6 sid from zebra when loc and sid block don't match (manual backport #18580)
@raja-rajasekar raja-rajasekar deleted the rajasekarr/check_sid_loc_block_beforehand branch April 14, 2025 15:36
@cscarpitta cscarpitta mentioned this pull request May 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cscarpitta cscarpitta cscarpitta approved these changes

Assignees

No one assigned

Labels

backport bugfix master size/M staticd

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@raja-rajasekar @cscarpitta