From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 810261FF14C for ; Fri, 15 May 2026 14:04:02 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7BB75B6CD; Fri, 15 May 2026 14:04:00 +0200 (CEST) From: Gabriel Goller To: pve-devel@lists.proxmox.com Subject: [PATCH frr v2 1/3] frr: backport "bgpd: fix valgrind memory leaks on daemon shutdown" (#21511) Date: Fri, 15 May 2026 14:03:45 +0200 Message-ID: <20260515120351.395649-2-g.goller@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260515120351.395649-1-g.goller@proxmox.com> References: <20260515120351.395649-1-g.goller@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778846628841 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.028 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: CA33FWH7KB36KV6MEETBAA7FAQXXCYJA X-Message-ID-Hash: CA33FWH7KB36KV6MEETBAA7FAQXXCYJA X-MailFrom: g.goller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Backport the "bgpd: fix valgrind memory leaks on daemon shutdown" (#21511) PR [1], which is a prerequisite for the PR [2] that fixes the EVPN local RT2 leaking data-races. [1]: https://github.com/FRRouting/frr/pull/21511 [2]: https://github.com/FRRouting/frr/pull/21844 Signed-off-by: Gabriel Goller --- debian/patches/series | 1 + ...rind-memory-leaks-on-daemon-shutdown.patch | 89 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 debian/patches/upstream/0005-bgpd-fix-valgrind-memory-leaks-on-daemon-shutdown.patch diff --git a/debian/patches/series b/debian/patches/series index fed297922f2d..89587e528948 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -2,6 +2,7 @@ upstream/0001-bgpd-fix-EVPN-VRF-auto-RT-deletion-collision.patch upstream/0002-bgpd-export-local-rt2-mac-ip-entries-to-unicast.patch upstream/0003-bgpd-do-not-add-local-vtep-as-remote.patch upstream/0004-topotests-add-bgp_evpn_rt2_local_leak.patch +upstream/0005-bgpd-fix-valgrind-memory-leaks-on-daemon-shutdown.patch pve/0001-enable-bgp-bfd-daemons.patch pve/0002-bgpd-add-an-option-for-RT-auto-derivation-to-force-A.patch pve/0003-tests-add-bgp-evpn-autort-test.patch diff --git a/debian/patches/upstream/0005-bgpd-fix-valgrind-memory-leaks-on-daemon-shutdown.patch b/debian/patches/upstream/0005-bgpd-fix-valgrind-memory-leaks-on-daemon-shutdown.patch new file mode 100644 index 000000000000..12eb4e2b3b16 --- /dev/null +++ b/debian/patches/upstream/0005-bgpd-fix-valgrind-memory-leaks-on-daemon-shutdown.patch @@ -0,0 +1,89 @@ +From 3e5d7c3b53175b8ce44bc4e34d217e745a1d5e32 Mon Sep 17 00:00:00 2001 +From: souroy +Date: Fri, 10 Apr 2026 22:11:51 -0700 +Subject: [PATCH 01/21] bgpd: fix valgrind memory leaks on daemon shutdown + +During daemon termination, the default BGP instance leaks due to +two issues: + +First, bgp_cleanup_routes() skips EVPN and ENCAP two-level table +cleanup for hidden instances, leaving route entries. Add a terminating +check so these tables are always cleaned during shutdown. + +Second, a circular dependency exists between bgp_free() and VNI lock +release: each L2VNI holds a bgp_lock on the default instance via +bgpevpn_link_to_l3vni(), but the only code that releases these locks +(bgp_evpn_cleanup -> free_vni_entry -> bgpevpn_unlink_from_l3vni) lives +inside bgp_free(), which only runs when the lock count reaches zero. +Break this cycle by calling bgp_evpn_cleanup() from bgp_delete() during +termination, before the final bgp_unlock(). This releases VNI-held +locks so the refcount can reach zero and bgp_free() actually executes. + +Signed-off-by: Soumya Roy +--- + bgpd/bgp_evpn.c | 7 ++++++- + bgpd/bgp_route.c | 2 +- + bgpd/bgpd.c | 9 +++++++++ + 3 files changed, 16 insertions(+), 2 deletions(-) + +diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c +index 4e5bd96c5b54..5d1122e34554 100644 +--- a/bgpd/bgp_evpn.c ++++ b/bgpd/bgp_evpn.c +@@ -5370,7 +5370,8 @@ static void evpn_mpattr_encode_type5(struct stream *s, const struct prefix *p, + * + * NOTE: NO need to pop the VPN routes in two cases + * 1) In free_vni_entry +- * - Called by bgp_free()->bgp_evpn_cleanup(). ++ * - Called by bgp_free()->bgp_evpn_cleanup() or ++ * bgp_delete()->bgp_evpn_cleanup() when terminating. + * - Since bgp_delete is called before bgp_free and we pop all the dest + * pertaining to bgp under delete. + * 2) evpn_delete_vni() when user configures "no vni" since the withdraw +@@ -7741,6 +7742,10 @@ void bgp_evpn_cleanup_on_disable(struct bgp *bgp) + */ + void bgp_evpn_cleanup(struct bgp *bgp) + { ++ /* Guard against double-call during termination */ ++ if (!bgp->vnihash) ++ return; ++ + hash_iterate(bgp->vnihash, + (void (*)(struct hash_bucket *, void *))free_vni_entry, + bgp); +diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c +index 351f01ef4077..778b1ce214c8 100644 +--- a/bgpd/bgp_route.c ++++ b/bgpd/bgp_route.c +@@ -7973,7 +7973,7 @@ void bgp_cleanup_routes(struct bgp *bgp) + /* + * VPN and ENCAP and EVPN tables are two-level (RD is top level) + */ +- if (safi != SAFI_MPLS_VPN && IS_BGP_INSTANCE_HIDDEN(bgp)) ++ if (safi != SAFI_MPLS_VPN && IS_BGP_INSTANCE_HIDDEN(bgp) && !bm->terminating) + continue; + + for (dest = bgp_table_top(bgp->rib[afi][safi]); dest; dest = bgp_route_next(dest)) { +diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c +index 116bef933995..ef8ebea216d6 100644 +--- a/bgpd/bgpd.c ++++ b/bgpd/bgpd.c +@@ -4418,6 +4418,15 @@ int bgp_delete(struct bgp *bgp) + + bgp_cleanup_routes(bgp); + ++ if (bm->terminating) ++ /* ++ * Release EVPN VNI bgp_lock references so the ++ * subsequent bgp_unlock() can drive refcount to ++ * zero and trigger bgp_free(). ++ */ ++ bgp_evpn_cleanup(bgp); ++ } ++ + for (afi = 0; afi < AFI_MAX; ++afi) { + if (!bgp->vpn_policy[afi].import_redirect_rtlist) + continue; +-- +2.47.3 + -- 2.47.3