From 9f9c0bc2447e3a878c1edfda8266c14ff0c9f287 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 (cherry picked from commit 4395fcd8e120958a91d3a11f918e9071b1cb5619) --- 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 6680b54f7665..9a796ef91b4b 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -6332,16 +6332,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 1949ede1244f..a5ba54013531 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3913,19 +3913,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 25d515cb2597a2d7963393b6c6b7aa6c8f43e067 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 (cherry picked from commit 186db96c06e4f44b4450fcba88f0fa680ee0b92d) --- bgpd/bgpd.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a5ba54013531..08d1268e9e6b 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3916,6 +3916,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); @@ -3923,6 +3924,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); @@ -3934,6 +3936,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 */