-
Notifications
You must be signed in to change notification settings - Fork 1.3k
zebra: Allow multiple Explicit Sids support with behavior uN #18736
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
zebra: Allow multiple Explicit Sids support with behavior uN #18736
Conversation
4693ab3
to
125ea35
Compare
125ea35
to
ebbae8c
Compare
ci:rerun |
ebbae8c
to
0901ffc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'm seeing some errors reported by Address Sanitizer after applying your patch when I try to delete and re-allocate a SID.
|
0901ffc
to
fe06893
Compare
fe06893
to
f70e05c
Compare
Thanks Carmine.. Can you check now? Let me know now if things look good. |
In the current code, we cannot allocate more than one explicit Sid for same uN behavior even if they are from different/same(with differente node len) locator blocks. For example, if we have the following config segment-routing srv6 static-sids sid fcbb:bbbb:1::/48 locator MAIN behavior uN sid 1234:5678:1::/48 locator RAJA behavior uN srv6 locators locator MAIN prefix fcbb:bbbb:1::/48 block-len 32 node-len 16 func-bits 16 locator RAJA prefix 1234:5678:1::/48 block-len 32 node-len 16 func-bits 16 Although the locators blocks are different, and staticd request to allocate multiple sid seems valid, zebra allocates only the first static-sid and returns the below error for others. ZEBRA: [NGMNY-JWMWQ] get_srv6_sid_explicit: cannot alloc SID 1234:5678:1:: for ctx End USP: ctx already associated with SID fcbb:bbbb:1:: But we should allow a node behave as a multiple Endpoints expecially if we have a topology where the node resides at the intersection of multiple zones/domain. Fix is to skip the 'if' condition check to bail out of the behavior is END. With fix: ip -6 route show 1234:5678:1::/48 nhid 9 encap seg6local action End dev sr0 proto 196 metric 20 pref medium fcbb:bbbb:1::/48 nhid 9 encap seg6local action End dev sr0 proto 196 metric 20 pref medium Signed-off-by: Rajasekar Raja <[email protected]>
f70e05c
to
6913929
Compare
ci:rerun |
1 similar comment
ci:rerun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also another issue.
Start from this configuration:
segment-routing
srv6
static-sids
sid fcbb:bbbb:1::/48 locator MAIN behavior uN
sid fcbb:cccc:1::/48 locator MAIN1 behavior uN
!
locators
locator MAIN1
prefix fcbb:bbbb:1::/48 block-len 32 node-len 16 func-bits 16
prefix fcbb:cccc:1::/48 block-len 32 node-len 16 func-bits 0
The two SIDs are allocated correctly:
router1# show segment-routing srv6 sid
SID Behavior Context Daemon/Instance Locator AllocationType
------------------------- --------- ----------------- --------- ----------------
fcbb:bbbb:1:: End - static(0) MAIN explicit
fcbb:cccc:1:: End - static(0) MAIN1 explicit
But when you try to remove a SID, FRR removes the wrong one:
router1# no sid fcbb:cccc:1::/48 locator MAIN1 behavior uN
router1#
router1#
router1# show segment-routing srv6 sid
SID Behavior Context Daemon/Instance Locator AllocationType
------------------------- --------- ----------------- --------- ----------------
fcbb:cccc:1:: End - static(0) MAIN1 explicit
@cscarpitta I think that's an excellent test case. The issue is that the i think in the ctx, we need to add something extra in case of multiple uN as a differentiating factor when requesting from clients.. maybe some id which we need to keep track of or do you have something in your mind ? for example in static_zebra_request_srv6_sid()
so nothing to differentiate.., maybe something like ctx.uN_id or maybe locator etc...?? |
#18806 covers this + more scenarios . So closing this. |
In the current code, we cannot allocate more than one explicit Sid for same uN behavior even if they are from different/same(with differente node len) locator blocks.
Although the locators blocks are different, and staticd request to allocate multiple sid seems valid, zebra allocates only the first static-sid and returns the below error for others.
Fix is to skip the 'if' condition check to bail out of the behavior is END.