bgpd: combine import_check_table and nexthop_check_table

In zebra, the import check table and the nexthop check tables
were combined.  This leaves an issue where when bgp happens
to have a tracked address in both the import check table
and the nexthop track table that are the same address.
When the the item is removed from one table the call
to remove it from zebra removes tracking for the other
table.

Combine the two tables together and keep track where
they came from for processing in bgpd.

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

diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
index d12dc22330133eb196f6e641f86fd128d00d17f0..44241b858208db704126c929a9186f77125b6c9f 100644 (file)
--- a/bgpd/bgp_nexthop.c
+++ b/bgpd/bgp_nexthop.c
@@ -58,7 +58,8 @@ void bnc_nexthop_free(struct bgp_nexthop_cache *bnc)
 
 struct bgp_nexthop_cache *bnc_new(struct bgp_nexthop_cache_head *tree,
                                  struct prefix *prefix, uint32_t srte_color,
-                                 ifindex_t ifindex)
+                                 ifindex_t ifindex, bool import_check_table,
+                                 bool nexthop_check_table)
 {
        struct bgp_nexthop_cache *bnc;
 
@@ -68,6 +69,9 @@ struct bgp_nexthop_cache *bnc_new(struct bgp_nexthop_cache_head *tree,
        bnc->ifindex_ipv6_ll = ifindex;
        bnc->srte_color = srte_color;
        bnc->tree = tree;
+       bnc->import_check_table = import_check_table;
+       bnc->nexthop_check_table = nexthop_check_table;
+
        LIST_INIT(&(bnc->paths));
        bgp_nexthop_cache_add(tree, bnc);
 
@@ -968,7 +972,7 @@ static void bgp_show_nexthops_detail(struct vty *vty, struct bgp *bgp,
 
 static void bgp_show_nexthop(struct vty *vty, struct bgp *bgp,
                             struct bgp_nexthop_cache *bnc, bool specific,
-                            json_object *json)
+                            bool import_check_table, json_object *json)
 {
        char buf[PREFIX2STR_BUFFER];
        time_t tbuf;
@@ -977,6 +981,12 @@ static void bgp_show_nexthop(struct vty *vty, struct bgp *bgp,
        json_object *json_last_update = NULL;
        json_object *json_nexthop = NULL;
 
+       if (bnc->import_check_table && !import_check_table)
+               return;
+
+       if (bnc->nexthop_check_table && import_check_table)
+               return;
+
        peer = (struct peer *)bnc->nht_info;
 
        if (json)
@@ -1103,16 +1113,14 @@ static void bgp_show_nexthops(struct vty *vty, struct bgp *bgp,
                else
                        vty_out(vty, "Current BGP nexthop cache:\n");
        }
-       if (import_table)
-               tree = &bgp->import_check_table;
-       else
-               tree = &bgp->nexthop_cache_table;
 
+       tree = &bgp->nexthop_cache_table;
        if (afi == AFI_IP || afi == AFI_IP6) {
                if (json)
                        json_afi = json_object_new_object();
                frr_each (bgp_nexthop_cache, &(*tree)[afi], bnc) {
-                       bgp_show_nexthop(vty, bgp, bnc, detail, json_afi);
+                       bgp_show_nexthop(vty, bgp, bnc, detail, import_table,
+                                        json_afi);
                        found = true;
                }
                if (found && json)
@@ -1126,7 +1134,8 @@ static void bgp_show_nexthops(struct vty *vty, struct bgp *bgp,
                if (json && (afi == AFI_IP || afi == AFI_IP6))
                        json_afi = json_object_new_object();
                frr_each (bgp_nexthop_cache, &(*tree)[afi], bnc)
-                       bgp_show_nexthop(vty, bgp, bnc, detail, json_afi);
+                       bgp_show_nexthop(vty, bgp, bnc, detail, import_table,
+                                        json_afi);
                if (json && (afi == AFI_IP || afi == AFI_IP6))
                        json_object_object_add(
                                json, (afi == AFI_IP) ? "ipv4" : "ipv6",
@@ -1162,15 +1171,15 @@ static int show_ip_bgp_nexthop_table(struct vty *vty, const char *name,
                                vty_out(vty, "nexthop address is malformed\n");
                        return CMD_WARNING;
                }
-               tree = import_table ? &bgp->import_check_table
-                                   : &bgp->nexthop_cache_table;
+               tree = &bgp->nexthop_cache_table;
                if (json)
                        json_afi = json_object_new_object();
                frr_each (bgp_nexthop_cache, &(*tree)[family2afi(nhop.family)],
                          bnc) {
                        if (prefix_cmp(&bnc->prefix, &nhop))
                                continue;
-                       bgp_show_nexthop(vty, bgp, bnc, true, json_afi);
+                       bgp_show_nexthop(vty, bgp, bnc, true, import_table,
+                                        json_afi);
                        found = true;
                }
                if (json)
@@ -1313,7 +1322,6 @@ void bgp_scan_init(struct bgp *bgp)
 
        for (afi = AFI_IP; afi < AFI_MAX; afi++) {
                bgp_nexthop_cache_init(&bgp->nexthop_cache_table[afi]);
-               bgp_nexthop_cache_init(&bgp->import_check_table[afi]);
                bgp->connected_table[afi] = bgp_table_init(bgp, afi,
                        SAFI_UNICAST);
        }
@@ -1333,7 +1341,6 @@ void bgp_scan_finish(struct bgp *bgp)
        for (afi = AFI_IP; afi < AFI_MAX; afi++) {
                /* Only the current one needs to be reset. */
                bgp_nexthop_cache_reset(&bgp->nexthop_cache_table[afi]);
-               bgp_nexthop_cache_reset(&bgp->import_check_table[afi]);
 
                bgp->connected_table[afi]->route_table->cleanup =
                        bgp_connected_cleanup;

diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h
index 49cbbaf885f48eab2a05711e0cac226bdbc27b31..c1d4d088a3257ddb26f5ec8912b28d7efd1da0b4 100644 (file)
--- a/bgpd/bgp_nexthop.h
+++ b/bgpd/bgp_nexthop.h
@@ -91,6 +91,9 @@ struct bgp_nexthop_cache {
         * nexthop.
         */
        bool is_evpn_gwip_nexthop;
+
+       bool import_check_table;
+       bool nexthop_check_table;
 };
 
 extern int bgp_nexthop_cache_compare(const struct bgp_nexthop_cache *a,
@@ -132,8 +135,9 @@ extern bool bgp_nexthop_self(struct bgp *bgp, afi_t afi, uint8_t type,
                             struct bgp_dest *dest);
 extern struct bgp_nexthop_cache *bnc_new(struct bgp_nexthop_cache_head *tree,
                                         struct prefix *prefix,
-                                        uint32_t srte_color,
-                                        ifindex_t ifindex);
+                                        uint32_t srte_color, ifindex_t ifindex,
+                                        bool import_check_table,
+                                        bool nexthop_check_table);
 extern bool bnc_existing_for_prefix(struct bgp_nexthop_cache *bnc);
 extern void bnc_free(struct bgp_nexthop_cache *bnc);
 extern struct bgp_nexthop_cache *bnc_find(struct bgp_nexthop_cache_head *tree,

diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index 60d6f74e1478569b63bb3c73209d4ff431f4fac0..aa37303fca48edcc356016791529b768afd23056 100644 (file)
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -378,14 +378,12 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
        } else
                return 0;
 
-       if (is_bgp_static_route)
-               tree = &bgp_nexthop->import_check_table[afi];
-       else
-               tree = &bgp_nexthop->nexthop_cache_table[afi];
+       tree = &bgp_nexthop->nexthop_cache_table[afi];
 
        bnc = bnc_find(tree, &p, srte_color, ifindex);
        if (!bnc) {
-               bnc = bnc_new(tree, &p, srte_color, ifindex);
+               bnc = bnc_new(tree, &p, srte_color, ifindex,
+                             is_bgp_static_route, !is_bgp_static_route);
                bnc->bgp = bgp_nexthop;
                if (BGP_DEBUG(nht, NHT))
                        zlog_debug("Allocated bnc %pFX(%d)(%u)(%s) peer %p",
@@ -393,6 +391,11 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
                                   bnc->srte_color, bnc->bgp->name_pretty,
                                   peer);
        } else {
+               if (is_bgp_static_route)
+                       bnc->import_check_table = true;
+               else
+                       bnc->nexthop_check_table = true;
+
                if (BGP_DEBUG(nht, NHT))
                        zlog_debug(
                                "Found existing bnc %pFX(%d)(%s) flags 0x%x ifindex %d #paths %d peer %p",
@@ -819,12 +822,8 @@ static void bgp_nht_ifp_handle(struct interface *ifp, bool up)
 
        bgp_nht_ifp_table_handle(bgp, &bgp->nexthop_cache_table[AFI_IP], ifp,
                                 up);
-       bgp_nht_ifp_table_handle(bgp, &bgp->import_check_table[AFI_IP], ifp,
-                                up);
        bgp_nht_ifp_table_handle(bgp, &bgp->nexthop_cache_table[AFI_IP6], ifp,
                                 up);
-       bgp_nht_ifp_table_handle(bgp, &bgp->import_check_table[AFI_IP6], ifp,
-                                up);
 }
 
 void bgp_nht_ifp_up(struct interface *ifp)
@@ -900,7 +899,7 @@ void bgp_nht_interface_events(struct peer *peer)
 void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 {
        struct bgp_nexthop_cache_head *tree = NULL;
-       struct bgp_nexthop_cache *bnc_nhc, *bnc_import;
+       struct bgp_nexthop_cache *bnc_nhc;
        struct bgp *bgp;
        struct prefix match;
        struct zapi_route nhr;
@@ -930,19 +929,12 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
                        zlog_debug(
                                "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, false);
-
-       tree = &bgp->import_check_table[afi];
-
-       bnc_import = bnc_find(tree, &match, nhr.srte_color, 0);
-       if (!bnc_import) {
-               if (BGP_DEBUG(nht, NHT))
-                       zlog_debug(
-                               "parse nexthop update %pFX(%u)(%s): bnc info not found for import check",
-                               &nhr.prefix, nhr.srte_color, bgp->name_pretty);
-       } else
-               bgp_process_nexthop_update(bnc_import, &nhr, true);
+       } else {
+               if (bnc_nhc->nexthop_check_table)
+                       bgp_process_nexthop_update(bnc_nhc, &nhr, false);
+               if (bnc_nhc->import_check_table)
+                       bgp_process_nexthop_update(bnc_nhc, &nhr, true);
+       }
 
        /*
         * HACK: if any BGP route is dependant on an SR-policy that doesn't

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 42e4c167f6b469fe38f6c5110efc61819e873b0f..3cfc5fb7afe9223c5fb03a9599a071ae02e6dadb 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -556,9 +556,6 @@ struct bgp {
        /* Tree for next-hop lookup cache. */
        struct bgp_nexthop_cache_head nexthop_cache_table[AFI_MAX];
 
-       /* Tree for import-check */
-       struct bgp_nexthop_cache_head import_check_table[AFI_MAX];
-
        struct bgp_table *connected_table[AFI_MAX];
 
        struct hash *address_hash;