From 7c603146529dc90f2d948e1943750efb1911c8ba Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 28 Mar 2024 12:25:05 -0400 Subject: [PATCH 1/2] bgpd: Note when receiving but not understanding a route notification When BGP has been asked to wait for FIB installation, on route removal a return call is likely to not have the dest since BGP will have cleaned up the node, entirely. Let's just note that the prefix cannot be found if debugs are turned on and move on. Signed-off-by: Donald Sharp --- bgpd/bgp_zebra.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 04d520a9233e..fda92270a867 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -2749,8 +2749,12 @@ static int bgp_zebra_route_notify_owner(int command, struct zclient *zclient, /* Find the bgp route node */ dest = bgp_safi_node_lookup(bgp->rib[afi][safi], safi, &p, &bgp->vrf_prd); - if (!dest) + if (!dest) { + if (BGP_DEBUG(zebra, ZEBRA)) + zlog_debug("%s: %pFX does not exist in the BGP table, nothing to do for %u", + __func__, &p, note); return -1; + } switch (note) { case ZAPI_ROUTE_INSTALLED: From 329d5a5cbba23ec740b6ee3e223be5b13a631eb8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 28 Mar 2024 12:27:38 -0400 Subject: [PATCH 2/2] bgpd: Arrange peer notification to after zebra announce 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 --- bgpd/bgp_route.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 88baa535100d..a9c630f8c190 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -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)) { @@ -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);