From 40dd3946a664695396b471a4dc4e03272a0e7739 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 6 Nov 2023 13:02:01 -0500 Subject: [PATCH] bgpd: Ensure BGP does not stop monitoring nexthops In some cases BGP can be monitoring the same prefix in both the nexthop and import check tables. If this is the case, when unregistering one bnc from one table make sure we are not still registered in the other Example of the problem: r1(config-router)# address-family ipv4 uni r1(config-router-af)# no network 192.168.100.41/32 r1(config-router-af)# exit r1# show bgp import-check-table Current BGP import check cache: r1# show bgp nexthop Current BGP nexthop cache: 192.168.100.41 valid [IGP metric 0], #paths 1, peer 192.168.100.41 if r1-eth0 Last update: Wed Dec 6 11:01:40 2023 BGP now believes it is only watching 192.168.100.41 in the nexthop cache, but zebra doesn't have anything: r1# show ip import-check VRF default: Resolve via default: on r1# show ip nht VRF default: Resolve via default: on So if anything happens to the route that is being matched for 192.168.100.41 bgp is no longer going to be notified about this. The source of this problem is that zebra has dropped the two different tables into 1 table, while bgp has 2 tables to track this. The solution to this problem (other than the rewrite that is being done ) is to have BGP have a bit of smarts about looking in both tables for the bnc and if found in both don't send the delete of the prefix tracking to zebra. Signed-off-by: Donald Sharp --- bgpd/bgp_nexthop.h | 2 ++ bgpd/bgp_nht.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h index 49cbbaf885f4..90e0074e358d 100644 --- a/bgpd/bgp_nexthop.h +++ b/bgpd/bgp_nexthop.h @@ -26,6 +26,8 @@ PREDECL_RBTREE_UNIQ(bgp_nexthop_cache); /* BGP nexthop cache value structure. */ struct bgp_nexthop_cache { + afi_t afi; + /* The ifindex of the outgoing interface *if* it's a v6 LL */ ifindex_t ifindex_ipv6_ll; diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 9a2166c86bd6..10c0382f6f51 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -386,6 +386,7 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, bnc = bnc_find(tree, &p, srte_color, ifindex); if (!bnc) { bnc = bnc_new(tree, &p, srte_color, ifindex); + bnc->afi = afi; bnc->bgp = bgp_nexthop; if (BGP_DEBUG(nht, NHT)) zlog_debug("Allocated bnc %pFX(%d)(%u)(%s) peer %p", @@ -1162,6 +1163,11 @@ static void register_zebra_rnh(struct bgp_nexthop_cache *bnc) */ static void unregister_zebra_rnh(struct bgp_nexthop_cache *bnc) { + struct bgp_nexthop_cache *import; + struct bgp_nexthop_cache *nexthop; + + struct bgp *bgp = bnc->bgp; + /* Check if we have already registered */ if (!CHECK_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED)) return; @@ -1171,6 +1177,19 @@ static void unregister_zebra_rnh(struct bgp_nexthop_cache *bnc) return; } + import = bnc_find(&bgp->import_check_table[bnc->afi], &bnc->prefix, 0, + 0); + nexthop = bnc_find(&bgp->nexthop_cache_table[bnc->afi], &bnc->prefix, 0, + 0); + + /* + * If this entry has both a import and a nexthop entry + * then let's not send the unregister quite as of yet + * wait until we only have 1 left + */ + if (import && nexthop) + return; + sendmsg_zebra_rnh(bnc, ZEBRA_NEXTHOP_UNREGISTER); }