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

bgpv1: Reset BGP session in UpdateNeighbor if necessary #25827

Merged
merged 1 commit into from Jun 6, 2023

Conversation

rastislavs
Copy link
Contributor

As Cilium's user-facing API based on CRDs is intent-based, this PR ensures that updated BGP peer configuration is always applied also on existing BGP sessions immediately, even if it requires a session reset. If the underlying BGP implementation (GoBGP at the moment) does not reset the session for particular configuration items automatically, we enforce the session reset explicitly.

For example, changing HoldTime for a peer will cause the following NOTIFICATION message:

image

Followed by a session re-establishment and the new timer interval exchanged in the OPEN message:

image

@rastislavs rastislavs added release-note/misc This PR makes changes that have no direct user impact. area/bgp labels Jun 1, 2023
@rastislavs rastislavs marked this pull request as ready for review June 2, 2023 07:53
@rastislavs rastislavs requested review from a team as code owners June 2, 2023 07:53
@dylandreimerink dylandreimerink removed their request for review June 2, 2023 09:51
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks!

@rastislavs
Copy link
Contributor Author

/test

Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

LGTM.

@rastislavs
Copy link
Contributor Author

/test

As Cilium's user-facing API based on CRDs is intent-based, this PR
ensures that updated BGP peer configuration is always applied also
on existing BGP sessions immediately, even if it requires a session
reset. If the underlying BGP implementation (GoBGP at the moment)
does not reset the session for particular configuration items
automatically, we enforce the session reset explicitly.

Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
@rastislavs
Copy link
Contributor Author

Cilium L4LB XDP (ci-l4lb) test failing: hitting complexity issue (#25892). Rebasing to pull the fix.

@rastislavs
Copy link
Contributor Author

/test

@rastislavs
Copy link
Contributor Author

All checks passing and reviews are in. Ready to merge.

@rastislavs rastislavs added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 6, 2023
@dylandreimerink dylandreimerink merged commit 50b964e into cilium:main Jun 6, 2023
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants