From 4a43b81d7c89f264ab8670f8381238cdc3207d85 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 24 Oct 2023 16:14:40 -0400 Subject: [PATCH] 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 --- bgpd/bgp_nexthop.c | 33 ++++++++++++++++++++------------- bgpd/bgp_nexthop.h | 8 ++++++-- bgpd/bgp_nht.c | 38 +++++++++++++++----------------------- bgpd/bgpd.h | 3 --- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index d12dc2233013..44241b858208 100644 --- 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 49cbbaf885f4..c1d4d088a325 100644 --- 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 60d6f74e1478..aa37303fca48 100644 --- 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 42e4c167f6b4..3cfc5fb7afe9 100644 --- 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;