bgpd: Fix import check removal

Fix: 06e4e90132ad23815c6f288dd7e6be334f5ab233

Modified BGP to pay more attention the prefix returned from
zebra to ensure that a LPM wasn't accidently causing BGP
import checks to think it had a match when it did not.
This unfortunately removed the check to handle the route
removal.

This sequence of config and events would leave BGP in a bad state:
ip route 100.100.100.0/24 Null0
router bgp 32932
  bgp network import-check
  address-family ipv4 uni
    network 100.100.100.0/24

Then if you removed the static route the import check would
still think the route existed:

donatas-pc(config)# ip route 100.100.100.0/24 Null0

donatas-pc(config)# do sh ip bgp import-check-table
Current BGP import check cache:
 100.100.100.0 valid [IGP metric 0], #paths 1
  blackhole
  Last update: Sat Apr 23 22:51:34 2022

donatas-pc(config)# do sh ip nht
100.100.100.0
 resolved via static
 is directly connected, Null0
 Client list: bgp(fd 17)

donatas-pc(config)# do sh ip bgp neighbors 192.168.10.123 advertised-routes | include 100.100.100.0
*> 100.100.100.0/24 0.0.0.0                  0         32768 i

donatas-pc(config)# no ip route 100.100.100.0/24 Null0

donatas-pc(config)# do sh ip nht
100.100.100.0
 resolved via kernel
 via 192.168.10.1, enp3s0
 Client list: bgp(fd 17)

donatas-pc(config)# do sh ip bgp import-check-table
Current BGP import check cache:
 100.100.100.0 valid [IGP metric 0], #paths 1
  blackhole
  Last update: Sat Apr 23 22:51:34 2022

donatas-pc(config)# do sh ip bgp neighbors 192.168.10.123 advertised-routes | include 100.100.100.0
*> 100.100.100.0/24 0.0.0.0                  0         32768 i
donatas-pc(config)#

Fix this by moving the code to handle the prefix check to the
evaluation function and mark the bnc as not matching and actually
evaluate the bnc.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

1 files changed, 20 insertions, 14 deletions

diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index dc4f30a906..3433e1471c 100644
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -390,7 +390,8 @@ void bgp_delete_connected_nexthop(afi_t afi, struct peer *peer)
 }
 
 static void bgp_process_nexthop_update(struct bgp_nexthop_cache *bnc,
-				       struct zapi_route *nhr)
+				       struct zapi_route *nhr,
+				       bool import_check)
 {
 	struct nexthop *nexthop;
 	struct nexthop *oldnh;
@@ -421,7 +422,21 @@ static void bgp_process_nexthop_update(struct bgp_nexthop_cache *bnc,
 	if (nhr->nexthop_num != bnc->nexthop_num)
 		bnc->change_flags |= BGP_NEXTHOP_CHANGED;
 
-	if (nhr->nexthop_num) {
+	if (import_check && (nhr->type == ZEBRA_ROUTE_BGP ||
+			     !prefix_same(&bnc->prefix, &nhr->prefix))) {
+		SET_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED);
+		UNSET_FLAG(bnc->flags, BGP_NEXTHOP_VALID);
+		UNSET_FLAG(bnc->flags, BGP_NEXTHOP_LABELED_VALID);
+		UNSET_FLAG(bnc->flags, BGP_NEXTHOP_EVPN_INCOMPLETE);
+
+		bnc_nexthop_free(bnc);
+		bnc->nexthop = NULL;
+
+		if (BGP_DEBUG(nht, NHT))
+			zlog_debug(
+				"%s: Import Check does not resolve to the same prefix for %pFX received %pFX or matching route is BGP",
+				__func__, &bnc->prefix, &nhr->prefix);
+	} else if (nhr->nexthop_num) {
 		struct peer *peer = bnc->nht_info;
 
 		/* notify bgp fsm if nbr ip goes from invalid->valid */
@@ -695,7 +710,7 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 				"parse nexthop update(%pFX(%u)(%s)): bnc info not found for nexthop cache",
 				&nhr.prefix, nhr.srte_color, bgp->name_pretty);
 	} else
-		bgp_process_nexthop_update(bnc_nhc, &nhr);
+		bgp_process_nexthop_update(bnc_nhc, &nhr, false);
 
 	tree = &bgp->import_check_table[afi];
 
@@ -706,17 +721,8 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 				"parse nexthop update(%pFX(%u)(%s)): bnc info not found for import check",
 				&nhr.prefix, nhr.srte_color, bgp->name_pretty);
 		return;
-	} else {
-		if (nhr.type == ZEBRA_ROUTE_BGP
-		    || !prefix_same(&bnc_import->prefix, &nhr.prefix)) {
-			if (BGP_DEBUG(nht, NHT))
-				zlog_debug(
-					"%s: Import Check does not resolve to the same prefix for %pFX received %pFX",
-					__func__, &bnc_import->prefix, &nhr.prefix);
-			return;
-		}
-		bgp_process_nexthop_update(bnc_import, &nhr);
 	}
+	bgp_process_nexthop_update(bnc_import, &nhr, true);
 
 	/*
 	 * HACK: if any BGP route is dependant on an SR-policy that doesn't
@@ -739,7 +745,7 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 			    || CHECK_FLAG(bnc_iter->flags, BGP_NEXTHOP_VALID))
 				continue;
 
-			bgp_process_nexthop_update(bnc_iter, &nhr);
+			bgp_process_nexthop_update(bnc_iter, &nhr, false);
 		}
 	}
 }