From f9d6087c51fbeafc76f3c7f1b72681cbab7f3651 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 21 Dec 2022 12:11:56 -0500 Subject: [PATCH 1/3] bgpd: static routes are leaked on shutdown Shutdown of bgp results in both the bgp_path_info, bgp_dest and bgp_table's not being freed because the bgp_path_info remains locked. Effectively static routes are scheduled for deletion but bgp_process skips the work because the work queue sees that the bgp router is marked for deletion. Effectively not doing any work and leaving data on the floor. Modify the code when attempting to put into the work queue to notice and not do so but just unlock the path info. This is effectively the same as what goes on for normal peering as that it checks for shutdown and just calls bgp_path_info_free too. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 95493c11f850..d1316dbc4f4e 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -8911,7 +8911,11 @@ void bgp_redistribute_withdraw(struct bgp *bgp, afi_t afi, int type, bgp_aggregate_decrement(bgp, bgp_dest_get_prefix(dest), pi, afi, SAFI_UNICAST); bgp_path_info_delete(dest, pi); - bgp_process(bgp, dest, afi, SAFI_UNICAST); + if (!CHECK_FLAG(bgp->flags, + BGP_FLAG_DELETE_IN_PROGRESS)) + bgp_process(bgp, dest, afi, SAFI_UNICAST); + else + bgp_path_info_reap(dest, pi); } } } From c560f0698f425d4c054e13eccc9df87431cdb73f Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 21 Dec 2022 15:22:24 -0500 Subject: [PATCH 2/3] bgpd: rfapi properly free a couple lists Signed-off-by: Donald Sharp --- bgpd/rfapi/bgp_rfapi_cfg.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/bgpd/rfapi/bgp_rfapi_cfg.c b/bgpd/rfapi/bgp_rfapi_cfg.c index eae9859ba188..b65d90e1b38d 100644 --- a/bgpd/rfapi/bgp_rfapi_cfg.c +++ b/bgpd/rfapi/bgp_rfapi_cfg.c @@ -3847,6 +3847,13 @@ struct rfapi_cfg *bgp_rfapi_cfg_new(struct rfapi_rfp_cfg *cfg) return h; } +static void bgp_rfapi_rfgn_list_delete(void *data) +{ + struct rfapi_rfg_name *rfgn = data; + free(rfgn->name); + rfgn_free(rfgn); +} + void bgp_rfapi_cfg_destroy(struct bgp *bgp, struct rfapi_cfg *h) { afi_t afi; @@ -3858,8 +3865,13 @@ void bgp_rfapi_cfg_destroy(struct bgp *bgp, struct rfapi_cfg *h) if (h->l2_groups != NULL) list_delete(&h->l2_groups); list_delete(&h->nve_groups_sequential); + + h->rfg_export_direct_bgp_l->del = bgp_rfapi_rfgn_list_delete; list_delete(&h->rfg_export_direct_bgp_l); + + h->rfg_export_zebra_l->del = bgp_rfapi_rfgn_list_delete; list_delete(&h->rfg_export_zebra_l); + if (h->default_rt_export_list) ecommunity_free(&h->default_rt_export_list); if (h->default_rt_import_list) From 1c225152c0f19d2d52c38e8cdc982750e50f72e0 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 21 Dec 2022 19:26:58 -0500 Subject: [PATCH 3/3] bgpd: bgp_connected_add memory was being leaked in some cases On shutdown, bgp calls an unlock for the bnc connected table, via the bgp_connected_cleanup function. This function is only ever called on shutdown, so we know that bgp is going away. The refcount for the connected data can be more than 1. Let's not worry about the refcount on shutdown and just delete the nodes instead of leaving them around. Signed-off-by: Donald Sharp --- bgpd/bgp_nexthop.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index 25a4a1b521ae..6bbdbdc1a95b 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -500,11 +500,8 @@ static void bgp_connected_cleanup(struct route_table *table, if (!bc) return; - bc->refcnt--; - if (bc->refcnt == 0) { - XFREE(MTYPE_BGP_CONN, bc); - bgp_dest_set_bgp_connected_ref_info(bn, NULL); - } + XFREE(MTYPE_BGP_CONN, bc); + bgp_dest_set_bgp_connected_ref_info(bn, NULL); } bool bgp_nexthop_self(struct bgp *bgp, afi_t afi, uint8_t type,