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>
(cherry picked from commit 9f002fa5dd34e7d901b501e7d0306027d85b531a)

diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index eb3366d33769190735b39b0788d02bc4c163fae3..188c7d90cfa7ed48df34e2ea4dfc057304413d48 100644 (file)
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -393,7 +393,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;
@@ -424,7 +425,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 */
@@ -696,7 +711,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];
 
@@ -707,17 +722,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
@@ -740,7 +746,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);
                }
        }
 }