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

isis, lib: add isis srv6 end sid to ls_prefix #15797

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

pguibert6WIND
Copy link
Member

@pguibert6WIND pguibert6WIND commented Apr 19, 2024

According to draft-ietf-lsr-isis-srv6-extensions draft, the End SID should be available in link state prefix information. Add the SID information in the link state prefix.

I have however a problem. how to test it .. I tried , but without success:

srv6_sr_added_on_option_222

Observed

r3# show isis database  detail r3.00-00
Area TE:
IS-IS Level-2 link-state database:
LSP ID                  PduLen  SeqNumber   Chksum  Holdtime  ATT/P/OL
r3.00-00             *    628   0x0000000a  0xf151     326    0/0/0
[..]
  MT Reachability: 0000.0000.0002.00 (Metric: 10) ipv6-unicast
    Admin Group: 0x00000020
      Bit positions: 5
    Local Interface IPv6 Address(es): 2001:db8:3::3:3
    Remote Interface IPv6 Address(es): 2001:db8:3::3:2
[..]
    Adjacency-SID: 15001, Weight: 0, Flags: F:1 B:0, V:1, L:1, S:0, P:0
    SRv6 End.X SID: fc00:0:3:1::, Algorithm: SPF, Weight: 0, Endpoint Behavior: End.DX6, Flags: B:0, S:0, P:0
        SRv6 SID Structure Locator Block length: 24, Locator Node length: 24, Function length: 16, Argument length: 0,

# show isis mpls-te database subnet detail
[..]

  Subnet: 2001:db8:ffff::4/128  Adv. Vertex: 0000.0000.0004     Metric: 10      Status: Sync
    Origin: ISIS_L2
    SID: 1400   Algorithm: 0    Flags: 0x60
[..]

Expected:

# show isis mpls-te database subnet detail
[..]
  Subnet: 2001:db8:ffff::4/128  Adv. Vertex: 0000.0000.0004     Metric: 10      Status: Sync
    Origin: ISIS_L2
    SID: 1400   Algorithm: 0    Flags: 0x60
    SIDv6: fc00:0:3:1::   Algorithm: 0    Flags: 0x00
[..]

@frrbot frrbot bot added the libfrr label Apr 19, 2024
@pguibert6WIND pguibert6WIND marked this pull request as draft April 23, 2024 15:39
@riw777 riw777 requested review from odd22 and riw777 and removed request for odd22 April 30, 2024 11:20
@pguibert6WIND
Copy link
Member Author

Found it, I only had to parse the locator TLV from the LSP.

  Subnet: fc00:0:6::/48 Adv. Vertex: 0000.0000.0006     Metric: 0       Status: Sync
    Origin: ISIS_L1
    SIDv6: fc00:0:6::   Endpoint behavior: End  Flags: 0x0

@github-actions github-actions bot added size/L and removed size/M labels Jun 21, 2024
Copy link
Contributor

@cscarpitta cscarpitta left a comment

Choose a reason for hiding this comment

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

Overall looks good. I have a few comments.

lib/link_state.c Outdated
json_object_object_add(json, "segment-routing-ipv6", jsrv6);
snprintfrr(buf, INET6_BUFSIZ, "%pI6", &pref->srv6.sid);
json_object_string_add(jsrv6, "sid", buf);
json_object_string_add(jsrv6, "sid",
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of this json field is incorrect. It should be "endpoint-behavior" or "behavior".

Copy link
Member Author

Choose a reason for hiding this comment

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

replaced with behavior

isisd/isis_te.c Outdated
memcpy(&sr.sid, &psid->sid, sizeof(struct in6_addr));

if (!CHECK_FLAG(ls_pref->flags, LS_PREF_SRV6) ||
!memcmp(&ls_pref->srv6, &sr, sizeof(struct ls_srv6_sid))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, shouldn't be the opposite?

Suggested change
!memcmp(&ls_pref->srv6, &sr, sizeof(struct ls_srv6_sid))) {
memcmp(&ls_pref->srv6, &sr, sizeof(struct ls_srv6_sid))) {

We are comparing the new SID sr that we received from the LSP with the SID we already had ls_pref->srv6.

If memcmp(...) != 0, it means the SID has changed, and therefore we need to update ls_pref->srv6.

Copy link
Member Author

Choose a reason for hiding this comment

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

right. modified

@pguibert6WIND
Copy link
Member Author

ci:rerun

Copy link
Contributor

@cscarpitta cscarpitta left a comment

Choose a reason for hiding this comment

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

LGTM

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

According to draft-ietf-lsr-isis-srv6-extensions draft,
the End SID should be available in link state prefix
information.

Add the SID information in the link state prefix, by
getting the END SID from the locator TLV information.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND pguibert6WIND marked this pull request as ready for review July 19, 2024 13:13
@riw777 riw777 merged commit 7f10381 into FRRouting:master Jul 26, 2024
11 checks passed
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.

3 participants