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: Make suppress-fib-pending clear peering #14982

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

donaldsharp
Copy link
Member

When a peer has come up and already started installing routes into the rib and suppress-fib-pending is either turned on or off. BGP is left with some routes that may need to be withdrawn from peers and routes that it does not know the status of. Clear the BGP peers for the interesting parties and let's let us come
up to speed as needed.

@donaldsharp
Copy link
Member Author

@Mergifyio backport stable/9.0 stable/9.1 stable/8.5

Copy link

mergify bot commented Dec 11, 2023

backport stable/9.0 stable/9.1 stable/8.5

✅ Backports have been created

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.

A couple of nits from my side (more like cosmetic, but yeah).

bgpd/bgp_fsm.c Outdated
"Reached received prefix count",
"Socket Error",
"Admin. shutdown (RTT)",
"Suppress Fib Turned on or Off",
Copy link
Member

Choose a reason for hiding this comment

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

nit: Just to be consistent: on or off (lowercase, but not mixed).

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

bgpd/bgpd.c Outdated
@@ -445,6 +468,7 @@ void bgp_suppress_fib_pending_set(struct bgp *bgp, bool set)
if (bgp_suppress_fib_count == 0)
send_msg = true;
bgp_suppress_fib_count++;

Copy link
Member

Choose a reason for hiding this comment

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

nit: can we drop this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@@ -1757,6 +1757,7 @@ struct peer {
#define PEER_DOWN_PFX_COUNT 33U /* Reached received prefix count */
#define PEER_DOWN_SOCKET_ERROR 34U /* Some socket error happened */
#define PEER_DOWN_RTT_SHUTDOWN 35U /* Automatically shutdown due to RTT */
#define PEER_DOWN_SUPPRESS_FIB_PENDING 36U /* Suppress fib pending changed */
Copy link
Member

Choose a reason for hiding this comment

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

Seems we have a bit unaligned here.

Copy link
Member Author

Choose a reason for hiding this comment

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

this is what clang-format has decided on. I have no idea why it's not aligned

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, just waiting on @ton31337 's comment

When a peer has come up and already started installing
routes into the rib and `suppress-fib-pending` is either
turned on or off.  BGP is left with some routes that
may need to be withdrawn from peers and routes that
it does not know the status of.  Clear the BGP peers
for the interesting parties and let's let us come
up to speed as needed.

Signed-off-by: Donald Sharp <[email protected]>
@donaldsharp donaldsharp force-pushed the bgp_suppress_fib_clear_peers branch from 8be2099 to bdb5ae8 Compare December 12, 2023 18:49
@riw777 riw777 merged commit c45a9da into FRRouting:master Dec 12, 2023
7 of 8 checks passed
donaldsharp added a commit that referenced this pull request Dec 12, 2023
bgpd: Make `suppress-fib-pending` clear peering (backport #14982)
donaldsharp added a commit that referenced this pull request Dec 12, 2023
bgpd: Make `suppress-fib-pending` clear peering (backport #14982)
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