Skip to content

Commit

Permalink
bgpd: Ensure BGP does not stop monitoring nexthops
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
donaldsharp committed Dec 6, 2023
1 parent cad5ee5 commit 40dd394
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
2 changes: 2 additions & 0 deletions bgpd/bgp_nexthop.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 19 additions & 0 deletions bgpd/bgp_nht.c
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down

0 comments on commit 40dd394

Please sign in to comment.