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

bgpd, lib: Include SID structure in seg6local nexthop #16835

Merged

Conversation

cscarpitta
Copy link
Contributor

@cscarpitta cscarpitta commented Sep 15, 2024

Include SID structure information when installing/removing an SRv6 SID in/from the forwarding plane.

Include SID structure information in seg6local nexthop data structure.

Signed-off-by: Carmine Scarpitta <[email protected]>
@frrbot frrbot bot added the libfrr label Sep 15, 2024
@cscarpitta cscarpitta force-pushed the add-sid-structure-to-seg6local-nh branch from d15eda9 to ec21870 Compare September 15, 2024 16:59
@cscarpitta cscarpitta changed the title bgp, lib: Include SID structure in seg6local nexthop bgpd, lib: Include SID structure in seg6local nexthop Sep 15, 2024
Include SID structure information when installing an SRv6 End.DT6 or End.DT4 SID
in the forwarding plane.

Signed-off-by: Carmine Scarpitta <[email protected]>
Include SID structure information when installing an SRv6 End.DT46 SID
in the forwarding plane.

Signed-off-by: Carmine Scarpitta <[email protected]>
Include SID structure information when removing an SRv6 End.DT4 or End.DT6 SID
from the forwarding plane.

Signed-off-by: Carmine Scarpitta <[email protected]>
Include SID structure information when removing an SRv6 End.DT46 SID
from the forwarding plane.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta cscarpitta force-pushed the add-sid-structure-to-seg6local-nh branch from ec21870 to 1587169 Compare September 15, 2024 19:35
@frrbot frrbot bot added the bgp label Sep 15, 2024
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

Are these new seg6loca_context struct members used by the dataplane somehow or I can't find just?

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.

Changes looks good. It fixes the locator deletion issue in the FPM Module in SONiC. This solution has bee discussed in the SONiC Routing WG.

@cscarpitta
Copy link
Contributor Author

Are these new seg6loca_context struct members used by the dataplane somehow or I can't find just?

@ton31337

In the SONiC Routing WG, Donald and SONiC team agreed to host an extended FPM module under SONiC for some features like SRv6.

In order to install/remove an SRv6 SID in the SONiC dataplane, this extended FPM module needs to know some locator information.

Currently, when a locator is removed in FRR, first zebra frees the locator data structures. Then the extended FPM module under SONiC accesses data structures owned by the master pthread to retrieve the locator information. This operation fails because the locator has already been freed.

To fix the issue, Donald proposed to pass all the information needed to install/remove the SID inside the dplane context. This way the extended FPM module no longer needs to access the master pthread's internal data structures that may already have been freed.

This PR implements the proposal from Donald.

It adds the required locator information to the seg6local_context passed to the FPM module inside the dplane context. This information will be used in the dataplane by the SONiC FPM module.

@ton31337
Copy link
Member

Got it, thanks for the explanation.

@ton31337 ton31337 merged commit 81db47a into FRRouting:master Sep 16, 2024
13 checks passed
@ton31337
Copy link
Member

ton31337 commented Oct 7, 2024

@Mergifyio backport stable/10.1 stable/10.0 stable/9.1

Copy link

mergify bot commented Oct 7, 2024

backport stable/10.1 stable/10.0 stable/9.1

✅ Backports have been created

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