From 4395fcd8e120958a91d3a11f918e9071b1cb5619 Mon Sep 17 00:00:00 2001 From: Rajasekar Raja Date: Wed, 10 Jul 2024 16:46:29 -0700 Subject: [PATCH 1/2] bgpd: backpressure - fix to properly remove dest for bgp under deletion In case of imported routes (L3vni/vrf leaks), when a bgp instance is being deleted, the peer->bgp comparision with the incoming bgp to remove the dest from the pending fifo is wrong. This can lead to the fifo having stale entries resulting in crash. Two changes are done here. - Instead of pop/push items in list if the struct bgp doesnt match, simply iterate the list and remove the expected ones. - Corrected the way bgp is fetched from dest rather than relying on path_info->peer so that it works for all kinds of routes. Ticket :#3980988 Signed-off-by: Chirag Shah Signed-off-by: Rajasekar Raja --- bgpd/bgp_evpn.c | 12 ++++++------ bgpd/bgpd.c | 20 +++++++++++++------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 75a7d85e88fc..baf1d4ca6de7 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -6333,16 +6333,16 @@ struct bgpevpn *bgp_evpn_new(struct bgp *bgp, vni_t vni, void bgp_evpn_free(struct bgp *bgp, struct bgpevpn *vpn) { struct bgp_dest *dest = NULL; - uint32_t ann_count = zebra_announce_count(&bm->zebra_announce_head); + struct bgp_dest *dest_next = NULL; - while (ann_count) { - dest = zebra_announce_pop(&bm->zebra_announce_head); - ann_count--; + for (dest = zebra_announce_first(&bm->zebra_announce_head); dest; + dest = dest_next) { + dest_next = zebra_announce_next(&bm->zebra_announce_head, dest); if (dest->za_vpn == vpn) { bgp_path_info_unlock(dest->za_bgp_pi); bgp_dest_unlock_node(dest); - } else - zebra_announce_add_tail(&bm->zebra_announce_head, dest); + zebra_announce_del(&bm->zebra_announce_head, dest); + } } bgp_evpn_remote_ip_hash_destroy(vpn); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 3810413adcf0..b6f7e29db298 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3936,19 +3936,25 @@ int bgp_delete(struct bgp *bgp) safi_t safi; int i; struct bgp_dest *dest = NULL; + struct bgp_dest *dest_next = NULL; + struct bgp_table *dest_table = NULL; struct graceful_restart_info *gr_info; - uint32_t ann_count = zebra_announce_count(&bm->zebra_announce_head); assert(bgp); - while (ann_count) { - dest = zebra_announce_pop(&bm->zebra_announce_head); - ann_count--; - if (dest->za_bgp_pi->peer->bgp == bgp) { + /* + * Iterate the pending dest list and remove all the dest pertaininig to + * the bgp under delete. + */ + for (dest = zebra_announce_first(&bm->zebra_announce_head); dest; + dest = dest_next) { + dest_next = zebra_announce_next(&bm->zebra_announce_head, dest); + dest_table = bgp_dest_table(dest); + if (dest_table->bgp == bgp) { bgp_path_info_unlock(dest->za_bgp_pi); bgp_dest_unlock_node(dest); - } else - zebra_announce_add_tail(&bm->zebra_announce_head, dest); + zebra_announce_del(&bm->zebra_announce_head, dest); + } } bgp_soft_reconfig_table_task_cancel(bgp, NULL, NULL); From 186db96c06e4f44b4450fcba88f0fa680ee0b92d Mon Sep 17 00:00:00 2001 From: Rajasekar Raja Date: Wed, 10 Jul 2024 20:17:14 -0700 Subject: [PATCH 2/2] bgpd: backpressure - Improve debuggability Improve debuggability in backpressure code. Ticket :#3980988 Signed-off-by: Rajasekar Raja --- bgpd/bgpd.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index b6f7e29db298..476a01b8ef08 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3939,6 +3939,7 @@ int bgp_delete(struct bgp *bgp) struct bgp_dest *dest_next = NULL; struct bgp_table *dest_table = NULL; struct graceful_restart_info *gr_info; + uint32_t cnt_before, cnt_after; assert(bgp); @@ -3946,6 +3947,7 @@ int bgp_delete(struct bgp *bgp) * Iterate the pending dest list and remove all the dest pertaininig to * the bgp under delete. */ + cnt_before = zebra_announce_count(&bm->zebra_announce_head); for (dest = zebra_announce_first(&bm->zebra_announce_head); dest; dest = dest_next) { dest_next = zebra_announce_next(&bm->zebra_announce_head, dest); @@ -3957,6 +3959,11 @@ int bgp_delete(struct bgp *bgp) } } + cnt_after = zebra_announce_count(&bm->zebra_announce_head); + if (BGP_DEBUG(zebra, ZEBRA)) + zlog_debug("Zebra Announce Fifo cleanup count before %u and after %u during BGP %s deletion", + cnt_before, cnt_after, bgp->name_pretty); + bgp_soft_reconfig_table_task_cancel(bgp, NULL, NULL); /* make sure we withdraw any exported routes */