From 052aea624e8be80c7a20bb69cb0e1b79a8a28a88 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 12 Mar 2025 09:01:52 -0400 Subject: [PATCH] Revert "bgpd: upon if event, evaluate bnc with matching nexthop" This reverts commit 58592be57783a3b24e7351af2a5afc61299768df. This commit is being reverted because of several issues: a) tcpdump -i This command causes bgp to dump it's entire table to all of it's peers again. This is a huge problem in any type of scaled environment *and* it is not unusual to have an operator do this. b) This commit appears to be attempting to solve the problem with route leaking across vrf's using labels( or somesuch ). Unfortunately we have absolutely no topotests that show the behavior. I am also unable to get any type of how to reproduce the problem being solved by the commit. I do know, though, that the problem really stems from the fact that bgp has decided to cheat and not create bnc's for route leaking. Thus when a nexthop changes, bgp is not being notified. This commit was being used as a hammer to solve the problem. While I do agree backing out a bug fix for some operator is less then ideal, I believe that since I cannot get the operator to tell me the problem it solved and the fact that sending large amounts of updates with just a simple tcpdump command ( actually 2 one for tcpdump start and one for finishing ) is more detrimental in my eyes at this point in time. Additionally the solution used is the wrong one for the problem. Signed-off-by: Donald Sharp --- bgpd/bgp_nht.c | 40 +--------------------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 3be6ee48b8..268d804699 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -763,10 +763,6 @@ static void bgp_nht_ifp_table_handle(struct bgp *bgp, struct interface *ifp, bool up) { struct bgp_nexthop_cache *bnc; - struct nexthop *nhop; - uint16_t other_nh_count; - bool nhop_ll_found = false; - bool nhop_found = false; if (ifp->ifindex == IFINDEX_INTERNAL) { zlog_warn("%s: The interface %s ignored", __func__, ifp->name); @@ -774,42 +770,9 @@ static void bgp_nht_ifp_table_handle(struct bgp *bgp, } frr_each (bgp_nexthop_cache, table, bnc) { - other_nh_count = 0; - nhop_ll_found = bnc->ifindex_ipv6_ll == ifp->ifindex; - for (nhop = bnc->nexthop; nhop; nhop = nhop->next) { - if (nhop->ifindex == bnc->ifindex_ipv6_ll) - continue; - - if (nhop->ifindex != ifp->ifindex) { - other_nh_count++; - continue; - } - if (nhop->vrf_id != ifp->vrf->vrf_id) { - other_nh_count++; - continue; - } - nhop_found = true; - } - - if (!nhop_found && !nhop_ll_found) - /* The event interface does not match the nexthop cache - * entry */ + if (bnc->ifindex_ipv6_ll != ifp->ifindex) continue; - if (!up && other_nh_count > 0) - /* Down event ignored in case of multiple next-hop - * interfaces. The other might interfaces might be still - * up. The cases where all interfaces are down or a bnc - * is invalid are processed by a separate zebra rnh - * messages. - */ - continue; - - if (!nhop_ll_found) { - evaluate_paths(bnc); - continue; - } - bnc->last_update = monotime(NULL); bnc->change_flags = 0; @@ -822,7 +785,6 @@ static void bgp_nht_ifp_table_handle(struct bgp *bgp, if (up) { SET_FLAG(bnc->flags, BGP_NEXTHOP_VALID); SET_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED); - /* change nexthop number only for ll */ bnc->nexthop_num = 1; } else { UNSET_FLAG(bnc->flags, BGP_NEXTHOP_PEER_NOTIFIED);