Skip to content

Commit

Permalink
bgpd: Arrange peer notification to after zebra announce
Browse files Browse the repository at this point in the history
Currently BGP attempts to send route change information
to it's peers *before* the route is installed into zebra.
This creates a bug in suppress-fib-pending in the following
scenario:

a) bgp suppress-fib-pending and bgp has a route with
2 way ecmp.
b) bgp receives a route withdraw from peer 1.  BGP
will send the route to zebra and mark the route as
FIB_INSTALL_PENDING.
c) bgp receives a route withdraw from peer 2.  BGP
will see the route has the FIB_INSTALL_PENDING and
not send the withdrawal of the route to the peer.
bgp will then send the route deletion to zebra and
clean up the bgp_path_info's.

At this point BGP is stuck where it has not sent
a route withdrawal to downstream peers.

Let's modify the code in bgp_process_main_one to
send the route notification to zebra first before
attempting to announce the route.  The route withdrawal
will remove the FIB_INSTALL_PENDING flag from the dest
and this will allow group_announce_route to believe
it can send the route withdrawal.

For the master branch this is ok because the recent
backpressure commits are in place and nothing is going
to change from an ordering perspective in that regards.
Ostensibly this fix is also for operators of Sonic and
will be backported to the 8.5 branch as well.  This will
change the order of the send to peers to be after the
zebra installation but sonic users are using suppress-fib-pending
anyways so updates won't go out until rib ack has been
received anyways.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
  • Loading branch information
donaldsharp committed Mar 28, 2024
1 parent 7c60314 commit 329d5a5
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions bgpd/bgp_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -3498,14 +3498,6 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
}
#endif

group_announce_route(bgp, afi, safi, dest, new_select);

/* unicast routes must also be annouced to labeled-unicast update-groups
*/
if (safi == SAFI_UNICAST)
group_announce_route(bgp, afi, SAFI_LABELED_UNICAST, dest,
new_select);

/* FIB update. */
if (bgp_fibupd_safi(safi) && (bgp->inst_type != BGP_INSTANCE_TYPE_VIEW)
&& !bgp_option_check(BGP_OPT_NO_FIB)) {
Expand Down Expand Up @@ -3537,6 +3529,15 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
}
}

group_announce_route(bgp, afi, safi, dest, new_select);

/* unicast routes must also be annouced to labeled-unicast update-groups
*/
if (safi == SAFI_UNICAST)
group_announce_route(bgp, afi, SAFI_LABELED_UNICAST, dest,
new_select);


bgp_process_evpn_route_injection(bgp, afi, safi, dest, new_select,
old_select);

Expand Down

0 comments on commit 329d5a5

Please sign in to comment.