From 8761cd6ddb5437767625f58c8e9cc3ccda7887ab Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 17 Dec 2020 09:46:30 -0500 Subject: [PATCH] bgpd: Switch LL nexthop tracking to be interface based bgp is currently registering v6 LL as nexthops to be tracked from zebra. This presents several problems. a) zebra does not properly track multiple prefixes that match the same route properly at this point in time. b) BGP was receiving nexthops that were just incorrect because of (a). c) When a nexthop changed that really didn't affect the v6 LL we were responding incorrectly because of this Modify the code such that bgp nexthop tracking notices that we are trying to register a v6 LL. When we do so, shortcut and watch interface up/down events for this v6 LL and do the work when an interface goes up / down for this type of tracking. Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 6 +++ bgpd/bgp_nexthop.c | 12 +++-- bgpd/bgp_nexthop.h | 3 ++ bgpd/bgp_nht.c | 128 ++++++++++++++++++++++++++++++++++++++++++++- bgpd/bgp_nht.h | 4 ++ bgpd/bgp_zebra.c | 4 ++ 6 files changed, 152 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index b69e2d71b6..30e2c3d489 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -1634,6 +1634,12 @@ static int bgp_connect_fail(struct peer *peer) return -1; } + /* + * If we are doing nht for a peer that ls v6 LL based + * massage the event system to make things happy + */ + bgp_nht_interface_events(peer); + return (bgp_stop(peer)); } diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index b7f62ec0a1..46942a0bea 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -783,7 +783,9 @@ static void bgp_show_nexthops_detail(struct vty *vty, struct bgp *bgp, vty_out(vty, " gate %s, if %s\n", inet_ntop(AF_INET6, &nexthop->gate.ipv6, buf, sizeof(buf)), - ifindex2ifname(nexthop->ifindex, bgp->vrf_id)); + ifindex2ifname(bnc->ifindex ? bnc->ifindex + : nexthop->ifindex, + bgp->vrf_id)); break; case NEXTHOP_TYPE_IPV4: vty_out(vty, " gate %s\n", @@ -792,13 +794,17 @@ static void bgp_show_nexthops_detail(struct vty *vty, struct bgp *bgp, break; case NEXTHOP_TYPE_IFINDEX: vty_out(vty, " if %s\n", - ifindex2ifname(nexthop->ifindex, bgp->vrf_id)); + ifindex2ifname(bnc->ifindex ? bnc->ifindex + : nexthop->ifindex, + bgp->vrf_id)); break; case NEXTHOP_TYPE_IPV4_IFINDEX: vty_out(vty, " gate %s, if %s\n", inet_ntop(AF_INET, &nexthop->gate.ipv4, buf, sizeof(buf)), - ifindex2ifname(nexthop->ifindex, bgp->vrf_id)); + ifindex2ifname(bnc->ifindex ? bnc->ifindex + : nexthop->ifindex, + bgp->vrf_id)); break; case NEXTHOP_TYPE_BLACKHOLE: vty_out(vty, " blackhole\n"); diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h index a223ff4133..fe0a9646a0 100644 --- a/bgpd/bgp_nexthop.h +++ b/bgpd/bgp_nexthop.h @@ -41,6 +41,9 @@ PREDECL_RBTREE_UNIQ(bgp_nexthop_cache); /* BGP nexthop cache value structure. */ struct bgp_nexthop_cache { + /* The ifindex of the outgoing interface *if* it's a v6 LL */ + ifindex_t ifindex; + /* RB-tree entry. */ struct bgp_nexthop_cache_item entry; diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index bc5da0ee21..bd26408687 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -55,6 +55,7 @@ static void unregister_zebra_rnh(struct bgp_nexthop_cache *bnc, int is_bgp_static_route); static void evaluate_paths(struct bgp_nexthop_cache *bnc); static int make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p); +static int bgp_nht_ifp_initial(struct thread *thread); static int bgp_isvalid_nexthop(struct bgp_nexthop_cache *bnc) { @@ -129,6 +130,7 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, struct prefix p; uint32_t srte_color = 0; int is_bgp_static_route = 0; + ifindex_t ifindex = 0; if (pi) { is_bgp_static_route = ((pi->type == ZEBRA_ROUTE_BGP) @@ -155,6 +157,14 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, srte_color = pi->attr->srte_color; } else if (peer) { + /* + * Gather the ifindex for if up/down events to be + * tagged into this fun + */ + if (afi == AFI_IP6 + && IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr)) + ifindex = peer->su.sin6.sin6_scope_id; + if (!sockunion2hostprefix(&peer->su, &p)) { if (BGP_DEBUG(nht, NHT)) { zlog_debug( @@ -175,6 +185,7 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, if (!bnc) { bnc = bnc_new(tree, &p, srte_color); bnc->bgp = bgp_nexthop; + bnc->ifindex = ifindex; if (BGP_DEBUG(nht, NHT)) { char buf[PREFIX2STR_BUFFER]; @@ -430,6 +441,107 @@ static void bgp_process_nexthop_update(struct bgp_nexthop_cache *bnc, evaluate_paths(bnc); } +static void bgp_nht_ifp_table_handle(struct bgp *bgp, + struct bgp_nexthop_cache_head *table, + struct interface *ifp, bool up) +{ + struct bgp_nexthop_cache *bnc; + + frr_each (bgp_nexthop_cache, table, bnc) { + if (bnc->ifindex != ifp->ifindex) + continue; + + bnc->last_update = bgp_clock(); + bnc->change_flags = 0; + + if (up) { + SET_FLAG(bnc->flags, BGP_NEXTHOP_VALID); + SET_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED); + bnc->metric = 1; + bnc->nexthop_num = 1; + } else { + UNSET_FLAG(bnc->flags, BGP_NEXTHOP_PEER_NOTIFIED); + UNSET_FLAG(bnc->flags, BGP_NEXTHOP_VALID); + SET_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED); + bnc->nexthop_num = 0; + bnc->metric = 0; + } + + evaluate_paths(bnc); + } +} +static void bgp_nht_ifp_handle(struct interface *ifp, bool up) +{ + struct bgp *bgp; + + bgp = bgp_lookup_by_vrf_id(ifp->vrf_id); + if (!bgp) + return; + + 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) +{ + bgp_nht_ifp_handle(ifp, true); +} + +void bgp_nht_ifp_down(struct interface *ifp) +{ + bgp_nht_ifp_handle(ifp, false); +} + +static int bgp_nht_ifp_initial(struct thread *thread) +{ + ifindex_t ifindex = THREAD_VAL(thread); + struct interface *ifp = if_lookup_by_index_all_vrf(ifindex); + + if (!ifp) + return 0; + + if (if_is_up(ifp)) + bgp_nht_ifp_up(ifp); + else + bgp_nht_ifp_down(ifp); + + return 0; +} + +/* + * So the bnc code has the ability to handle interface up/down + * events to properly handle v6 LL peering. + * What is happening here: + * The event system for peering expects the nht code to + * report on the tracking events after we move to active + * So let's give the system a chance to report on that event + * in a manner that is expected. + */ +void bgp_nht_interface_events(struct peer *peer) +{ + struct bgp *bgp = peer->bgp; + struct bgp_nexthop_cache_head *table; + struct bgp_nexthop_cache *bnc; + struct prefix p; + + if (!IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr)) + return; + + if (!sockunion2hostprefix(&peer->su, &p)) + return; + + table = &bgp->nexthop_cache_table[AFI_IP6]; + bnc = bnc_find(table, &p, 0); + if (!bnc) + return; + + if (bnc->ifindex) + thread_add_event(bm->master, bgp_nht_ifp_initial, NULL, + bnc->ifindex, NULL); +} + void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id) { struct bgp_nexthop_cache_head *tree = NULL; @@ -661,6 +773,12 @@ static void register_zebra_rnh(struct bgp_nexthop_cache *bnc, /* Check if we have already registered */ if (bnc->flags & BGP_NEXTHOP_REGISTERED) return; + + if (bnc->ifindex) { + SET_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED); + return; + } + if (is_bgp_import_route) sendmsg_zebra_rnh(bnc, ZEBRA_IMPORT_ROUTE_REGISTER); else @@ -681,6 +799,11 @@ static void unregister_zebra_rnh(struct bgp_nexthop_cache *bnc, if (!CHECK_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED)) return; + if (bnc->ifindex) { + UNSET_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED); + return; + } + if (is_bgp_import_route) sendmsg_zebra_rnh(bnc, ZEBRA_IMPORT_ROUTE_UNREGISTER); else @@ -851,9 +974,10 @@ static void evaluate_paths(struct bgp_nexthop_cache *bnc) if (!CHECK_FLAG(bnc->flags, BGP_NEXTHOP_PEER_NOTIFIED)) { if (BGP_DEBUG(nht, NHT)) zlog_debug( - "%s: Updating peer (%s(%s)) status with NHT", + "%s: Updating peer (%s(%s)) status with NHT nexthops %d", __func__, peer->host, - peer->bgp->name_pretty); + peer->bgp->name_pretty, + !!valid_nexthops); bgp_fsm_nht_update(peer, !!valid_nexthops); SET_FLAG(bnc->flags, BGP_NEXTHOP_PEER_NOTIFIED); } diff --git a/bgpd/bgp_nht.h b/bgpd/bgp_nht.h index f374e8dfa5..a1683e1511 100644 --- a/bgpd/bgp_nht.h +++ b/bgpd/bgp_nht.h @@ -97,4 +97,8 @@ extern void bgp_l3nhg_id_free(uint32_t nhg_id); extern void bgp_l3nhg_init(void); void bgp_l3nhg_finish(void); +extern void bgp_nht_ifp_up(struct interface *ifp); +extern void bgp_nht_ifp_down(struct interface *ifp); + +extern void bgp_nht_interface_events(struct peer *peer); #endif /* _BGP_NHT_H */ diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 8d03079fd7..d397a5241a 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -250,6 +250,8 @@ static int bgp_ifp_up(struct interface *ifp) bgp_nbr_connected_add(bgp, nc); hook_call(bgp_vrf_status_changed, bgp, ifp); + bgp_nht_ifp_up(ifp); + return 0; } @@ -305,6 +307,8 @@ static int bgp_ifp_down(struct interface *ifp) } hook_call(bgp_vrf_status_changed, bgp, ifp); + bgp_nht_ifp_down(ifp); + return 0; }