diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h index 830883872e77..430c8f17e890 100644 --- a/bgpd/bgp_nexthop.h +++ b/bgpd/bgp_nexthop.h @@ -90,6 +90,7 @@ struct bgp_nexthop_cache { struct bgp_nexthop_cache_head *tree; struct prefix prefix; + struct prefix resolved_prefix; void *nht_info; /* In BGP, peer session */ LIST_HEAD(path_list, bgp_path_info) paths; unsigned int path_count; diff --git a/bgpd/bgp_nhg.c b/bgpd/bgp_nhg.c index 64b0edbcdad1..134f2b2bd518 100644 --- a/bgpd/bgp_nhg.c +++ b/bgpd/bgp_nhg.c @@ -749,6 +749,112 @@ static void bgp_nhg_detach_nexthop(struct bgp_nhg_cache *nhg) bgp_nhg_del_nhg(nhg); } +/* called whenever a nhg is resolved over ourselves + * nexthop_active() detects compares prefix to install with the resolved route + * but with nhg, the prefix is not available, and we lose the loop detection + * this code replaces nhgs with regular route_add/delete to avoid this corner case + * for each path to install, use route_add/delete when route resolved matches the prefix + * for each path, we assume dependent paths are active, and are pushed again to zebra + */ +static bool bgp_nhg_try_detach_if_same_prefix(struct bgp_nhg_cache *nhg, + struct prefix *resolved_prefix) +{ + struct bgp_nhg_connected *rb_node_dep = NULL; + struct bgp_nhg_cache *dep_nhg; + struct bgp_path_info *path, *safe; + const struct prefix *p; + + frr_each_safe (bgp_nhg_connected_tree, &nhg->nhg_dependents, + rb_node_dep) { + dep_nhg = rb_node_dep->nhg; + LIST_FOREACH_SAFE (path, &(dep_nhg->paths), nhg_cache_thread, + safe) { + p = bgp_dest_get_prefix(path->net); + if (path->bgp_nhg_nexthop == nhg && + prefix_same(resolved_prefix, p) && + ((p->family == AF_INET && + (p->prefixlen != IPV4_MAX_BITLEN)) || + (p->family == AF_INET6 && + (p->prefixlen != IPV6_MAX_BITLEN)))) { + if (BGP_DEBUG(nexthop_group, NEXTHOP_GROUP)) + zlog_debug(" %s: %pFX, matched against ourself and prefix length is not max bit length", + __func__, p); + /* nhg = pi->nhg_nexthop is detached, + * nhg will not be suppressed when bgp_nhg_path_unlink() is called + */ + bgp_nhg_path_nexthop_unlink(path, false); + bgp_nhg_path_unlink(path); + /* path should still be active */ + if (bgp_dest_get_bgp_table_info(path->net)->bgp) + bgp_zebra_announce( + path->net, p, path, + bgp_dest_get_bgp_table_info( + path->net) + ->bgp, + bgp_dest_get_bgp_table_info( + path->net) + ->afi, + bgp_dest_get_bgp_table_info( + path->net) + ->safi); + } + } + } + if (LIST_EMPTY(&(nhg->paths))) { + bgp_nhg_del_nhg(nhg); + return true; + } + return false; +} + +/* called whenever a nhg is resolved over default route: + * nexthop_active() detects resolution over default route by comparing to the matching routes + * but with nhg, we may lose the ability to detect a looped resolution + * this code replaced nhgs with regular route_add/delete to avoid this corner case + * for each path, we assume dependent paths are active, and are pushed again to zebra + */ +static bool bgp_nhg_detach_paths(struct bgp_nhg_cache *nhg) +{ + struct bgp_nhg_connected *rb_node_dep = NULL; + struct bgp_nhg_cache *dep_nhg; + struct bgp_path_info *path, *safe; + const struct prefix *p; + + frr_each_safe (bgp_nhg_connected_tree, &nhg->nhg_dependents, + rb_node_dep) { + dep_nhg = rb_node_dep->nhg; + LIST_FOREACH_SAFE (path, &(dep_nhg->paths), nhg_cache_thread, + safe) { + p = bgp_dest_get_prefix(path->net); + if (BGP_DEBUG(nexthop_group, NEXTHOP_GROUP)) + zlog_debug(" :%s: %pFX Resolved against default route", + __func__, p); + /* nhg = pi->nhg_nexthop is detached, + * nhg will not be suppressed when bgp_nhg_path_unlink() is called + */ + bgp_nhg_path_nexthop_unlink(path, false); + bgp_nhg_path_unlink(path); + /* path should still be active */ + if (bgp_dest_get_bgp_table_info(path->net)->bgp) + bgp_zebra_announce(path->net, p, path, + bgp_dest_get_bgp_table_info( + path->net) + ->bgp, + bgp_dest_get_bgp_table_info( + path->net) + ->afi, + bgp_dest_get_bgp_table_info( + path->net) + ->safi); + } + } + if (LIST_EMPTY(&(nhg->paths))) { + bgp_nhg_del_nhg(nhg); + return true; + } + return false; +} + void bgp_nhg_refresh_by_nexthop(struct bgp_nexthop_cache *bnc) { struct bgp_nhg_cache *nhg; @@ -803,6 +909,15 @@ void bgp_nhg_refresh_by_nexthop(struct bgp_nexthop_cache *bnc) bgp_nhg_detach_nexthop(nhg); continue; } + + if (is_default_prefix(&bnc->resolved_prefix) && + bgp_nhg_detach_paths(nhg)) + continue; + + if (bgp_nhg_try_detach_if_same_prefix(nhg, + &bnc->resolved_prefix)) + continue; + if (BGP_DEBUG(nexthop_group, NEXTHOP_GROUP)) zlog_debug("NHG %u, VRF %u : IGP change detected with NH %pFX SRTE %u", nhg->id, vrf_id, p, srte_color); diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 732458255a75..daa5fc62a18c 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -650,6 +650,8 @@ static void bgp_process_nexthop_update(struct bgp_nexthop_cache *bnc, } else if (nhr->nexthop_num) { struct peer *peer = bnc->nht_info; + prefix_copy(&bnc->resolved_prefix, &nhr->prefix); + /* notify bgp fsm if nbr ip goes from invalid->valid */ if (!bnc->nexthop_num) UNSET_FLAG(bnc->flags, BGP_NEXTHOP_PEER_NOTIFIED); @@ -755,6 +757,7 @@ static void bgp_process_nexthop_update(struct bgp_nexthop_cache *bnc, } } } else { + memset(&bnc->resolved_prefix, 0, sizeof(bnc->resolved_prefix)); bnc->flags &= ~BGP_NEXTHOP_EVPN_INCOMPLETE; bnc->flags &= ~BGP_NEXTHOP_VALID; bnc->flags &= ~BGP_NEXTHOP_LABELED_VALID; diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 6bee766d0243..53e62031e319 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1508,6 +1508,29 @@ void bgp_zebra_announce_parse_nexthop(struct bgp_path_info *info, * or blackhole routes */ for (i = 0; i < *valid_nh_count; i++) { + if (p_mpinfo[i] && p_mpinfo[i]->nexthop && + is_default_prefix(&p_mpinfo[i]->nexthop->resolved_prefix)) { + if (BGP_DEBUG(nexthop_group, NEXTHOP_GROUP)) + zlog_debug(" :%s: %pFX Resolved against default route", + __func__, p); + for (i = 0; i < *valid_nh_count; i++) + bgp_nhg_path_unlink(p_mpinfo[i]); + return; + } + if (p_mpinfo[i] && p_mpinfo[i]->nexthop && + prefix_same(p, &p_mpinfo[i]->nexthop->resolved_prefix)) { + if ((p->family == AF_INET && + (p->prefixlen != IPV4_MAX_BITLEN)) || + (p->family == AF_INET6 && + (p->prefixlen != IPV6_MAX_BITLEN))) { + if (BGP_DEBUG(nexthop_group, NEXTHOP_GROUP)) + zlog_debug(" %s: %pFX, Matched against ourself and prefix length is not max bit length", + __func__, p); + for (i = 0; i < *valid_nh_count; i++) + bgp_nhg_path_unlink(p_mpinfo[i]); + } + return; + } if (api->nexthops[i].type == NEXTHOP_TYPE_IFINDEX || api->nexthops[i].type == NEXTHOP_TYPE_BLACKHOLE) { for (i = 0; i < *valid_nh_count; i++)