]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd: LL peers need bnc's per peer
authorDonald Sharp <sharpd@nvidia.com>
Thu, 21 Jul 2022 19:42:51 +0000 (15:42 -0400)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Sat, 23 Jul 2022 10:17:19 +0000 (10:17 +0000)
FRR should create a bnc per peer.  Not have
one's that write over others.  Currently when
FRR has multiple Interface based peering, BGP wa
creating a single BNC.  This is insufficient in that
we were accidently overwriting the one LL with other
data.  This causes issues when there are multiple and
there is weird starting issues with those interfaces
that you are peering over.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 35aae5c9bcf91703b74464d2b0dca6504bd37e39)

bgpd/bgp_evpn.c
bgpd/bgp_nexthop.c
bgpd/bgp_nexthop.h
bgpd/bgp_nht.c

index bbbe538acc9bf6197411b30dcef2af56d7e23b88..395111e1d28d00e9e0bc5afb24ae69007f671ebd 100644 (file)
@@ -6428,7 +6428,7 @@ static void bgp_evpn_remote_ip_process_nexthops(struct bgpevpn *vpn,
                return;
 
        tree = &vpn->bgp_vrf->nexthop_cache_table[afi];
-       bnc = bnc_find(tree, &p, 0);
+       bnc = bnc_find(tree, &p, 0, 0);
 
        if (!bnc || !bnc->is_evpn_gwip_nexthop)
                return;
index c6043807dd9196f99718610ffc08fe15f4900bd1..e1fcc743eca3a158f8cd4a663bd3ecb788a017a3 100644 (file)
@@ -56,6 +56,11 @@ int bgp_nexthop_cache_compare(const struct bgp_nexthop_cache *a,
        if (a->srte_color > b->srte_color)
                return 1;
 
+       if (a->ifindex < b->ifindex)
+               return -1;
+       if (a->ifindex > b->ifindex)
+               return 1;
+
        return prefix_cmp(&a->prefix, &b->prefix);
 }
 
@@ -70,13 +75,15 @@ 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)
+                                 struct prefix *prefix, uint32_t srte_color,
+                                 ifindex_t ifindex)
 {
        struct bgp_nexthop_cache *bnc;
 
        bnc = XCALLOC(MTYPE_BGP_NEXTHOP_CACHE,
                      sizeof(struct bgp_nexthop_cache));
        bnc->prefix = *prefix;
+       bnc->ifindex = ifindex;
        bnc->srte_color = srte_color;
        bnc->tree = tree;
        LIST_INIT(&(bnc->paths));
@@ -106,7 +113,8 @@ void bnc_free(struct bgp_nexthop_cache *bnc)
 }
 
 struct bgp_nexthop_cache *bnc_find(struct bgp_nexthop_cache_head *tree,
-                                  struct prefix *prefix, uint32_t srte_color)
+                                  struct prefix *prefix, uint32_t srte_color,
+                                  ifindex_t ifindex)
 {
        struct bgp_nexthop_cache bnc = {};
 
@@ -115,6 +123,7 @@ struct bgp_nexthop_cache *bnc_find(struct bgp_nexthop_cache_head *tree,
 
        bnc.prefix = *prefix;
        bnc.srte_color = srte_color;
+       bnc.ifindex = ifindex;
        return bgp_nexthop_cache_find(tree, &bnc);
 }
 
@@ -915,7 +924,7 @@ static int show_ip_bgp_nexthop_table(struct vty *vty, const char *name,
                }
                tree = import_table ? &bgp->import_check_table
                                    : &bgp->nexthop_cache_table;
-               bnc = bnc_find(tree[family2afi(nhop.family)], &nhop, 0);
+               bnc = bnc_find(tree[family2afi(nhop.family)], &nhop, 0, 0);
                if (!bnc) {
                        vty_out(vty, "specified nexthop does not have entry\n");
                        return CMD_SUCCESS;
index 16c2b6c65a8ad36c5f4a5d04d1d9f5dc15f2fa64..9d653ef4dc2f497b1a6795be2c18c366086ca819 100644 (file)
@@ -152,12 +152,14 @@ 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);
+                                        uint32_t srte_color,
+                                        ifindex_t ifindex);
 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,
                                          struct prefix *prefix,
-                                         uint32_t srte_color);
+                                         uint32_t srte_color,
+                                         ifindex_t ifindex);
 extern void bnc_nexthop_free(struct bgp_nexthop_cache *bnc);
 extern const char *bnc_str(struct bgp_nexthop_cache *bnc, char *buf, int size);
 extern void bgp_scan_init(struct bgp *bgp);
index 3ab67a618152bf0447f00a094c6a4fe30043386f..e03e83db2c3363a8b6ff943147af931664e3d83d 100644 (file)
@@ -84,9 +84,10 @@ static void bgp_unlink_nexthop_check(struct bgp_nexthop_cache *bnc)
        if (LIST_EMPTY(&(bnc->paths)) && !bnc->nht_info) {
                if (BGP_DEBUG(nht, NHT)) {
                        char buf[PREFIX2STR_BUFFER];
-                       zlog_debug("%s: freeing bnc %s(%u)(%s)", __func__,
+                       zlog_debug("%s: freeing bnc %s(%d)(%u)(%s)", __func__,
                                   bnc_str(bnc, buf, PREFIX2STR_BUFFER),
-                                  bnc->srte_color, bnc->bgp->name_pretty);
+                                  bnc->ifindex, bnc->srte_color,
+                                  bnc->bgp->name_pretty);
                }
                /* only unregister if this is the last nh for this prefix*/
                if (!bnc_existing_for_prefix(bnc))
@@ -113,17 +114,32 @@ void bgp_replace_nexthop_by_peer(struct peer *from, struct peer *to)
        struct prefix pt;
        struct bgp_nexthop_cache *bncp, *bnct;
        afi_t afi;
+       ifindex_t ifindex = 0;
 
        if (!sockunion2hostprefix(&from->su, &pp))
                return;
 
+       /*
+        * Gather the ifindex for if up/down events to be
+        * tagged into this fun
+        */
+       if (from->conf_if && IN6_IS_ADDR_LINKLOCAL(&from->su.sin6.sin6_addr))
+               ifindex = from->su.sin6.sin6_scope_id;
+
        afi = family2afi(pp.family);
-       bncp = bnc_find(&from->bgp->nexthop_cache_table[afi], &pp, 0);
+       bncp = bnc_find(&from->bgp->nexthop_cache_table[afi], &pp, 0, ifindex);
 
        if (!sockunion2hostprefix(&to->su, &pt))
                return;
 
-       bnct = bnc_find(&to->bgp->nexthop_cache_table[afi], &pt, 0);
+       /*
+        * Gather the ifindex for if up/down events to be
+        * tagged into this fun
+        */
+       ifindex = 0;
+       if (to->conf_if && IN6_IS_ADDR_LINKLOCAL(&to->su.sin6.sin6_addr))
+               ifindex = to->su.sin6.sin6_scope_id;
+       bnct = bnc_find(&to->bgp->nexthop_cache_table[afi], &pt, 0, ifindex);
 
        if (bnct != bncp)
                return;
@@ -137,11 +153,17 @@ void bgp_unlink_nexthop_by_peer(struct peer *peer)
        struct prefix p;
        struct bgp_nexthop_cache *bnc;
        afi_t afi = family2afi(peer->su.sa.sa_family);
+       ifindex_t ifindex = 0;
 
        if (!sockunion2hostprefix(&peer->su, &p))
                return;
-
-       bnc = bnc_find(&peer->bgp->nexthop_cache_table[afi], &p, 0);
+       /*
+        * 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;
+       bnc = bnc_find(&peer->bgp->nexthop_cache_table[afi], &p, 0, ifindex);
        if (!bnc)
                return;
 
@@ -206,9 +228,18 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
                 * 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))
+               if (afi == AFI_IP6 &&
+                   IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr)) {
                        ifindex = peer->su.sin6.sin6_scope_id;
+                       if (ifindex == 0) {
+                               if (BGP_DEBUG(nht, NHT)) {
+                                       zlog_debug(
+                                               "%s: Unable to locate ifindex, waiting till we have one",
+                                               peer->conf_if);
+                               }
+                               return 0;
+                       }
+               }
 
                if (!sockunion2hostprefix(&peer->su, &p)) {
                        if (BGP_DEBUG(nht, NHT)) {
@@ -226,28 +257,27 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
        else
                tree = &bgp_nexthop->nexthop_cache_table[afi];
 
-       bnc = bnc_find(tree, &p, srte_color);
+       bnc = bnc_find(tree, &p, srte_color, ifindex);
        if (!bnc) {
-               bnc = bnc_new(tree, &p, srte_color);
+               bnc = bnc_new(tree, &p, srte_color, ifindex);
                bnc->bgp = bgp_nexthop;
-               bnc->ifindex = ifindex;
                if (BGP_DEBUG(nht, NHT)) {
                        char buf[PREFIX2STR_BUFFER];
 
-                       zlog_debug("Allocated bnc %s(%u)(%s) peer %p",
+                       zlog_debug("Allocated bnc %s(%d)(%u)(%s) peer %p",
                                   bnc_str(bnc, buf, PREFIX2STR_BUFFER),
-                                  bnc->srte_color, bnc->bgp->name_pretty,
-                                  peer);
+                                  bnc->ifindex, bnc->srte_color,
+                                  bnc->bgp->name_pretty, peer);
                }
        } else {
                if (BGP_DEBUG(nht, NHT)) {
                        char buf[PREFIX2STR_BUFFER];
 
                        zlog_debug(
-                               "Found existing bnc %s(%s) flags 0x%x ifindex %d #paths %d peer %p",
+                               "Found existing bnc %s(%d)(%s) flags 0x%x ifindex %d #paths %d peer %p",
                                bnc_str(bnc, buf, PREFIX2STR_BUFFER),
-                               bnc->bgp->name_pretty, bnc->flags, bnc->ifindex,
-                               bnc->path_count, bnc->nht_info);
+                               bnc->ifindex, bnc->bgp->name_pretty, bnc->flags,
+                               bnc->ifindex, bnc->path_count, bnc->nht_info);
                }
        }
 
@@ -351,15 +381,21 @@ void bgp_delete_connected_nexthop(afi_t afi, struct peer *peer)
 {
        struct bgp_nexthop_cache *bnc;
        struct prefix p;
+       ifindex_t ifindex = 0;
 
        if (!peer)
                return;
 
        if (!sockunion2hostprefix(&peer->su, &p))
                return;
-
+       /*
+        * 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;
        bnc = bnc_find(&peer->bgp->nexthop_cache_table[family2afi(p.family)],
-                      &p, 0);
+                      &p, 0, ifindex);
        if (!bnc) {
                if (BGP_DEBUG(nht, NHT))
                        zlog_debug(
@@ -408,9 +444,9 @@ static void bgp_process_nexthop_update(struct bgp_nexthop_cache *bnc,
                char bnc_buf[BNC_FLAG_DUMP_SIZE];
 
                zlog_debug(
-                       "%s(%u): Rcvd NH update %pFX(%u) - metric %d/%d #nhops %d/%d flags %s",
+                       "%s(%u): Rcvd NH update %pFX(%u)%u) - metric %d/%d #nhops %d/%d flags %s",
                        bnc->bgp->name_pretty, bnc->bgp->vrf_id, &nhr->prefix,
-                       bnc->srte_color, nhr->metric, bnc->metric,
+                       bnc->ifindex, bnc->srte_color, nhr->metric, bnc->metric,
                        nhr->nexthop_num, bnc->nexthop_num,
                        bgp_nexthop_dump_bnc_flags(bnc, bnc_buf,
                                                   sizeof(bnc_buf)));
@@ -659,15 +695,22 @@ void bgp_nht_interface_events(struct peer *peer)
        struct bgp_nexthop_cache_head *table;
        struct bgp_nexthop_cache *bnc;
        struct prefix p;
+       ifindex_t ifindex = 0;
 
        if (!IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr))
                return;
 
        if (!sockunion2hostprefix(&peer->su, &p))
                return;
+       /*
+        * Gather the ifindex for if up/down events to be
+        * tagged into this fun
+        */
+       if (peer->conf_if && IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr))
+               ifindex = peer->su.sin6.sin6_scope_id;
 
        table = &bgp->nexthop_cache_table[AFI_IP6];
-       bnc = bnc_find(table, &p, 0);
+       bnc = bnc_find(table, &p, 0, ifindex);
        if (!bnc)
                return;
 
@@ -703,7 +746,7 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
        afi = family2afi(match.family);
        tree = &bgp->nexthop_cache_table[afi];
 
-       bnc_nhc = bnc_find(tree, &match, nhr.srte_color);
+       bnc_nhc = bnc_find(tree, &match, nhr.srte_color, 0);
        if (!bnc_nhc) {
                if (BGP_DEBUG(nht, NHT))
                        zlog_debug(
@@ -714,7 +757,7 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 
        tree = &bgp->import_check_table[afi];
 
-       bnc_import = bnc_find(tree, &match, nhr.srte_color);
+       bnc_import = bnc_find(tree, &match, nhr.srte_color, 0);
        if (!bnc_import) {
                if (BGP_DEBUG(nht, NHT))
                        zlog_debug(
@@ -982,8 +1025,9 @@ void evaluate_paths(struct bgp_nexthop_cache *bnc)
 
                bnc_str(bnc, buf, PREFIX2STR_BUFFER);
                zlog_debug(
-                       "NH update for %s(%u)(%s) - flags %s chgflags %s- evaluate paths",
-                       buf, bnc->srte_color, bnc->bgp->name_pretty,
+                       "NH update for %s(%d)(%u)(%s) - flags %s chgflags %s- evaluate paths",
+                       buf, bnc->ifindex, bnc->srte_color,
+                       bnc->bgp->name_pretty,
                        bgp_nexthop_dump_bnc_flags(bnc, bnc_buf,
                                                   sizeof(bnc_buf)),
                        bgp_nexthop_dump_bnc_change_flags(bnc, chg_buf,
@@ -1190,6 +1234,7 @@ void bgp_nht_reg_enhe_cap_intfs(struct peer *peer)
        struct nexthop *nhop;
        struct interface *ifp;
        struct prefix p;
+       ifindex_t ifindex = 0;
 
        if (peer->ifp)
                return;
@@ -1203,8 +1248,14 @@ void bgp_nht_reg_enhe_cap_intfs(struct peer *peer)
 
        if (p.family != AF_INET6)
                return;
+       /*
+        * Gather the ifindex for if up/down events to be
+        * tagged into this fun
+        */
+       if (peer->conf_if && IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr))
+               ifindex = peer->su.sin6.sin6_scope_id;
 
-       bnc = bnc_find(&bgp->nexthop_cache_table[AFI_IP6], &p, 0);
+       bnc = bnc_find(&bgp->nexthop_cache_table[AFI_IP6], &p, 0, ifindex);
        if (!bnc)
                return;
 
@@ -1231,6 +1282,7 @@ void bgp_nht_dereg_enhe_cap_intfs(struct peer *peer)
        struct nexthop *nhop;
        struct interface *ifp;
        struct prefix p;
+       ifindex_t ifindex = 0;
 
        if (peer->ifp)
                return;
@@ -1245,8 +1297,14 @@ void bgp_nht_dereg_enhe_cap_intfs(struct peer *peer)
 
        if (p.family != AF_INET6)
                return;
+       /*
+        * Gather the ifindex for if up/down events to be
+        * tagged into this fun
+        */
+       if (peer->conf_if && IN6_IS_ADDR_LINKLOCAL(&peer->su.sin6.sin6_addr))
+               ifindex = peer->su.sin6.sin6_scope_id;
 
-       bnc = bnc_find(&bgp->nexthop_cache_table[AFI_IP6], &p, 0);
+       bnc = bnc_find(&bgp->nexthop_cache_table[AFI_IP6], &p, 0, ifindex);
        if (!bnc)
                return;