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.

Sign up for GitHub

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

Jump to bottom

bgpd: Extend BGP to communicate with the SRv6 SID Manager to allocate/release SRv6 SIDs #15676

Merged
merged 10 commits into from
Sep 5, 2024
Merged

bgpd: Extend BGP to communicate with the SRv6 SID Manager to allocate/release SRv6 SIDs #15676

merged 10 commits into from
Sep 5, 2024

Conversation

cscarpitta
Copy link
Contributor

PR #15604 introduces the SRv6 SID Manager, a zebra component responsible for SID allocation/management. The SRv6 SID Manager exposes a SID allocation/release APIs, allowing clients to request and release an SRv6 SID.

This PR extends the BGP daemon to communicate with the SRv6 SID Manager to allocate/release SRv6 SIDs.

@frrbot frrbot bot added the bgp label Apr 3, 2024
@ton31337 ton31337 self-assigned this Apr 5, 2024
@ton31337 ton31337 self-requested a review April 5, 2024 08:24
@ton31337 ton31337 removed their assignment Apr 5, 2024
@riw777 riw777 self-requested a review April 9, 2024 14:37
@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from 025a328 to 1426639 Compare May 3, 2024 14:58
@github-actions GitHub Actions
Copy link

github-actions bot commented May 6, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from 1426639 to eea365a Compare May 6, 2024 09:36
@github-actions github-actions bot removed the conflicts label May 6, 2024
@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from eea365a to 8a0bf18 Compare May 8, 2024 16:10
@frrbot frrbot bot added the libfrr label May 8, 2024
@github-actions github-actions bot added size/XXL and removed size/XL labels May 8, 2024
@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from 8a0bf18 to a1d9830 Compare May 8, 2024 16:20
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

1 similar comment
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from a1d9830 to ad2698a Compare May 10, 2024 16:56
riw777
riw777 approved these changes May 13, 2024
View reviewed changes
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.

one really minor nit ... code looks okay to me, but it would be good to have @mjstapp at least look at the zebra bits as well

bgpd/bgp_mplsvpn.c Outdated Show resolved Hide resolved
@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from ad2698a to 9b6ec7c Compare May 13, 2024 18:47
@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch 2 times, most recently from 817303e to 403d544 Compare June 7, 2024 10:44
@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from c6de540 to 5415f9c Compare June 18, 2024 16:26
@github-actions github-actions bot added size/XL and removed size/XXL labels Jun 18, 2024
@cscarpitta
Copy link
Contributor Author

@riw777 Many thanks for the review. I rebased the PR based on the merged SID manager. Can you please take a look again?

@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from 3c6900e to 7c2a296 Compare July 8, 2024 10:29
@cscarpitta
Copy link
Contributor Author

cscarpitta commented Jul 8, 2024 •
edited
Loading

ci:retry unrelated OSPF failure

@dmytroshytyi-6WIND
Copy link
Contributor

dmytroshytyi-6WIND commented Jul 8, 2024 •
edited
Loading

@cscarpitta ,
This comment should be addressed in this PR, as this PR considers BGP related extensions #15679 (comment) of #15679

It was first mentioned in the topotest to better describe the context.

Thanks

@cscarpitta
Copy link
Contributor Author

@cscarpitta , This comment should be addressed in this PR, as this PR considers BGP related extensions #15679 (comment) of #15679

It was first mentioned in the topotest to better describe the context.

Thanks

@dmytroshytyi-6WIND

Thanks for the comment.

In this PR, we report issue in the FRR log when the user tries to allocate a SID function that is out of the valid range.

I plan to submit also another PR to reject the configuration and notify the user.

@cscarpitta
Copy link
Contributor Author

ci:retry unrelated ospf topotest failure

@dmytroshytyi-6WIND
Copy link
Contributor

@cscarpitta , This comment should be addressed in this PR, as this PR considers BGP related extensions #15679 (comment) of #15679
It was first mentioned in the topotest to better describe the context.
Thanks

@dmytroshytyi-6WIND

Thanks for the comment.

In this PR, we report issue in the FRR log when the user tries to allocate a SID function that is out of the valid range.

I plan to submit also another PR to reject the configuration and notify the user.

Could you please create the new PR-placeholder ("to reject the configuration and notify the user) and cross-reference the two?

@cscarpitta
Copy link
Contributor Author

ci:retry unrelated test failures

@cscarpitta cscarpitta force-pushed the bgp-srv6-sid-manager branch from 7c2a296 to cabe2ca Compare July 21, 2024 05:30
@riw777
Copy link
Member

riw777 commented Jul 23, 2024

I suspect we're going to want a new topotest to cover this addition ... if it's possible to create one. @ton31337 thoughts?

@cscarpitta
Copy link
Contributor Author

I suspect we're going to want a new topotest to cover this addition ... if it's possible to create one. @ton31337 thoughts?

Hi @riw777

Many thanks for the review.

We have another PR with the topotest (#15679). The topotest extensively tests the SID manager and the interaction with the client daemons (BGP, IS-IS).

Originally the SID manager, client daemon extension (BGP, IS-IS) and the topotest were in a single PR.
But the PR was very large, hence we split into SID Manager, BGP extension, IS-IS extension, and topotest.

@riw777
Copy link
Member

riw777 commented Aug 14, 2024

I'm kindof waiting on #15679 to pass before pushing this ...

@ahsalam
Copy link

ahsalam commented Aug 15, 2024

I'm kindof waiting on #15679 to pass before pushing this ...

PR #15679 depends on this one as it tests the code to be merged here.

Once this is merged, We will rebase PR #15679 and rerun the CI.

@cscarpitta
Copy link
Contributor Author

ci:retry CI failure is not related, rerunning

cscarpitta and others added 10 commits September 5, 2024 10:59
@cscarpitta
bgpd: Add API to get SRv6 locator info
c50b4b9 
Add an API to request information from the SRv6 SID Manager (zebra)
regarding a specific SRv6 locator.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Deal with SRv6 locator instead of chunk
fe5037b 
Currently, when SRv6 is enabled in BGP, BGP requests a locator chunk
from Zebra. Zebra assigns a locator chunk to BGP, and then BGP can
allocate SIDs from the locator chunk.

Recently, the implementation of SRv6 in Zebra has been improved, and a
new API has been introduced for obtaining/releasing the SIDs.

Now, the daemons no longer need to request a chunk.

Instead, the daemons interact with Zebra to obtain information about the
locator and subsequently to allocate/release the SIDs.

This commit extends BGP to use the new SRv6 API. In particular, it
removes the chunk throughout the BGP code and modifies BGP to
request/save/advertise the locator instead of the chunk.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Receive SRv6 locator info from zebra
8b0fb91 
This commit extends BGP to process locator information received from
SRv6 Manager (zebra) and save the locator info in the SRv6 database.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Add API to get/release SRv6 SIDs
b0be30b 
Add an API to get/release SRv6 SIDs through the SRv6 SID Manager.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Release SIDs when disabling SRv6 in BGP
831b825 
When SRv6 VPN is unconfigured in BGP, BGP needs to interact with SID Manager to
release the SID and make it available to other daemons

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Request SRv6 SIDs to SID Manager
2ed0ebd 
Currently, BGP allocates SIDs without interacting with Zebra.

Recently, the SRv6 implementation has been improved. Now, the daemons
need to interact with Zebra through ZAPI to obtain and release SIDs.

This commit extends BGP to request SIDs from Zebra instead of allocating
the SIDs on its own.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Make sid_register() non-static
7a9f26c 
Make the `sid_register()` function non-static to allow other BGP modules
(e.g. bgp_zebra.c) to register SIDs.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Receive SRv6 SIDs notification from zebra
7a2e64e 
Zebra sends a `SRV6_SID_NOTIFY` notification to inform clients about the
result of a SID alloc/release operation.  This commit adds a handler to
process a `SRV6_SID_NOTIFY` notification received from zebra.

If the notification indicates that a SID allocation operation was
successful, then it stores the allocated SID in the SRv6 database,
installs the SID into the RIB, and advertises the SID to the other BGP
routers.

If the notification indicates that an operation has failed, it logs the
error.

Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
bgpd: Cleanup related to SRv6
8227268 
Remove unused SRv6 code.

Signed-off-by: Carmine Scarpitta <[email protected]>
@pguibert6WIND @cscarpitta
bgpd: add locator name in sid notify messages
f686609 
In the near future, some daemons may only register SIDs. This may be
the case for the pathd daemon when creating SRv6 binding SIDs.

When a locator is getting deleted at ZEBRA level, the daemon may have
an easy way to find out the SIds to unregister to.

This commit proposes to add the locator name to the SID_SRV6_NOTIFY
message whenever possible. Only case when an allocation failure happens,
the locator will not be present. In all other places, the notify API
at procol levels has the locator name extra-parameter.

Signed-off-by: Philippe Guibert <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
Copy link
Contributor Author

ci:retry

@riw777 riw777 merged commit 5d9ddcc into FRRouting:master Sep 5, 2024
11 checks passed
cscarpitta added a commit to cscarpitta/sonic-buildimage that referenced this pull request Dec 16, 2024
@cscarpitta
[FRR]: Bring PR FRRouting/frr#15676 from FRR mainline
fd1601c 
This commit brings PR FRRouting/frr#15676 from FRR mainline to SONiC

bgpd: Extend BGP to communicate with the SRv6 SID Manager to allocate/release SRv6 SIDs
FRRouting/frr#15676

Signed-off-by: cscarpitta <[email protected]>
cscarpitta added a commit to cscarpitta/sonic-buildimage that referenced this pull request Dec 16, 2024
@cscarpitta
[FRR]: Bring PR FRRouting/frr#15676 from FRR mainline
f8ce5ae 
This commit brings PR FRRouting/frr#15676 from FRR mainline to SONiC

bgpd: Extend BGP to communicate with the SRv6 SID Manager to allocate/release SRv6 SIDs
FRRouting/frr#15676

Signed-off-by: cscarpitta <[email protected]>
@cscarpitta cscarpitta mentioned this pull request Dec 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmytroshytyi-6WIND dmytroshytyi-6WIND dmytroshytyi-6WIND left review comments

@riw777 riw777 riw777 approved these changes

@ton31337 ton31337 Awaiting requested review from ton31337

Assignees
No one assigned
Labels
bgp libfrr master size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cscarpitta @dmytroshytyi-6WIND @riw777 @ahsalam @ton31337 @pguibert6WIND