From: Donald Sharp Date: Tue, 24 Oct 2023 20:14:40 +0000 (-0400) Subject: bgpd: combine import_check_table and nexthop_check_table X-Git-Tag: base_10.0~323^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=4a43b81d7c89f264ab8670f8381238cdc3207d85;p=matthieu%2Ffrr.git 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 --- diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index d12dc22330..44241b8582 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 49cbbaf885..c1d4d088a3 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 60d6f74e14..aa37303fca 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 42e4c167f6..3cfc5fb7af 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;