]> git.puffer.fish Git - matthieu/frr.git/commitdiff
*: When matching against a nexthop send and process what it matched against
authorDonald Sharp <sharpd@nvidia.com>
Sat, 12 Mar 2022 15:47:16 +0000 (10:47 -0500)
committerDonald Sharp <sharpd@nvidia.com>
Sat, 12 Mar 2022 16:18:45 +0000 (11:18 -0500)
Currently the nexthop tracking code is only sending to the requestor
what it was requested to match against.  When the nexthop tracking
code was simplified to not need an import check and a nexthop check
in b8210849b8ac1abe2d5d9a5ab2459abfde65efa5 for bgpd.  It was not
noticed that a longer prefix could match but it would be seen
as a match because FRR was not sending up both the resolved
route prefix and the route FRR was asked to match against.

This code change causes the nexthop tracking code to pass
back up the matched requested route (so that the calling
protocol can figure out which one it is being told about )
as well as the actual prefix that was matched to.

Fixes: #10766
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
bgpd/bgp_nht.c
lib/zclient.c
lib/zclient.h
ospf6d/ospf6_zebra.c
pbrd/pbr_zebra.c
pimd/pim_nht.c
sharpd/sharp_zebra.c
staticd/static_zebra.c
zebra/zebra_rnh.c
zebra/zebra_srte.c

index 8313c12e613104da3441c0b000c896685a147655..a3330a6a5aaf6413960ad3276126b8265dee05db 100644 (file)
@@ -665,6 +665,7 @@ 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 *bgp;
+       struct prefix match;
        struct zapi_route nhr;
        afi_t afi;
 
@@ -677,16 +678,16 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
                return;
        }
 
-       if (!zapi_nexthop_update_decode(zclient->ibuf, &nhr)) {
+       if (!zapi_nexthop_update_decode(zclient->ibuf, &match, &nhr)) {
                zlog_err("%s[%s]: Failure to decode nexthop update", __func__,
                         bgp->name_pretty);
                return;
        }
 
-       afi = family2afi(nhr.prefix.family);
+       afi = family2afi(match.family);
        tree = &bgp->nexthop_cache_table[afi];
 
-       bnc_nhc = bnc_find(tree, &nhr.prefix, nhr.srte_color);
+       bnc_nhc = bnc_find(tree, &match, nhr.srte_color);
        if (!bnc_nhc) {
                if (BGP_DEBUG(nht, NHT))
                        zlog_debug(
@@ -697,7 +698,7 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 
        tree = &bgp->import_check_table[afi];
 
-       bnc_import = bnc_find(tree, &nhr.prefix, nhr.srte_color);
+       bnc_import = bnc_find(tree, &match, nhr.srte_color);
        if (!bnc_import) {
                if (BGP_DEBUG(nht, NHT))
                        zlog_debug(
index 930adf6a7af20d95cf7a1ca369b4ca209658e887..f6c5a8af08fbd6f9da01a0698d97d475dc2d7eb9 100644 (file)
@@ -1924,7 +1924,8 @@ const char *zapi_nexthop2str(const struct zapi_nexthop *znh, char *buf,
 /*
  * Decode the nexthop-tracking update message
  */
-bool zapi_nexthop_update_decode(struct stream *s, struct zapi_route *nhr)
+bool zapi_nexthop_update_decode(struct stream *s, struct prefix *match,
+                               struct zapi_route *nhr)
 {
        uint32_t i;
 
@@ -1932,6 +1933,22 @@ bool zapi_nexthop_update_decode(struct stream *s, struct zapi_route *nhr)
 
        STREAM_GETL(s, nhr->message);
        STREAM_GETW(s, nhr->safi);
+       STREAM_GETW(s, match->family);
+       STREAM_GETC(s, match->prefixlen);
+       /*
+        * What we got told to match against
+        */
+       switch (match->family) {
+       case AF_INET:
+               STREAM_GET(&match->u.prefix4.s_addr, s, IPV4_MAX_BYTELEN);
+               break;
+       case AF_INET6:
+               STREAM_GET(&match->u.prefix6, s, IPV6_MAX_BYTELEN);
+               break;
+       }
+       /*
+        * What we matched against
+        */
        STREAM_GETW(s, nhr->prefix.family);
        STREAM_GETC(s, nhr->prefix.prefixlen);
        switch (nhr->prefix.family) {
index ca62b1afeb8f03af99ab2479ea9c3496c36a4148..092754f6026255b429ff9ab5df92a486c81de12c 100644 (file)
@@ -1111,7 +1111,17 @@ int zapi_nexthop_from_nexthop(struct zapi_nexthop *znh,
                              const struct nexthop *nh);
 int zapi_backup_nexthop_from_nexthop(struct zapi_nexthop *znh,
                                     const struct nexthop *nh);
-extern bool zapi_nexthop_update_decode(struct stream *s,
+/*
+ * match -> is the prefix that the calling daemon asked to be matched
+ * against.
+ * nhr->prefix -> is the actual prefix that was matched against in the
+ * rib itself.
+ *
+ * This distinction is made because a LPM can be made if there is a
+ * covering route.  This way the upper level protocol can make a decision
+ * point about whether or not it wants to use the match or not.
+ */
+extern bool zapi_nexthop_update_decode(struct stream *s, struct prefix *match,
                                       struct zapi_route *nhr);
 const char *zapi_nexthop2str(const struct zapi_nexthop *znh, char *buf,
                             int bufsize);
index e279d0411ba14c98f6f8fd45dbea9e524bde33af..4b37c5bc208bf6aa408bf4b262e3ee06ba9214a5 100644 (file)
@@ -166,19 +166,20 @@ static int ospf6_zebra_import_check_update(ZAPI_CALLBACK_ARGS)
 {
        struct ospf6 *ospf6;
        struct zapi_route nhr;
+       struct prefix matched;
 
        ospf6 = ospf6_lookup_by_vrf_id(vrf_id);
        if (ospf6 == NULL || !IS_OSPF6_ASBR(ospf6))
                return 0;
 
-       if (!zapi_nexthop_update_decode(zclient->ibuf, &nhr)) {
+       if (!zapi_nexthop_update_decode(zclient->ibuf, &matched, &nhr)) {
                zlog_err("%s[%u]: Failure to decode route", __func__,
                         ospf6->vrf_id);
                return -1;
        }
 
-       if (nhr.prefix.family != AF_INET6 || nhr.prefix.prefixlen != 0
-           || nhr.type == ZEBRA_ROUTE_OSPF6)
+       if (matched.family != AF_INET6 || matched.prefixlen != 0 ||
+           nhr.type == ZEBRA_ROUTE_OSPF6)
                return 0;
 
        ospf6->nssa_default_import_check.status = !!nhr.nexthop_num;
index b480d4072e72edb00501e0ebe3a73e8bd1134e2c..f992588729c33888ad98088e8a2ada36a96dbc59 100644 (file)
@@ -399,17 +399,19 @@ void route_delete(struct pbr_nexthop_group_cache *pnhgc, afi_t afi)
 static int pbr_zebra_nexthop_update(ZAPI_CALLBACK_ARGS)
 {
        struct zapi_route nhr;
+       struct prefix matched;
        uint32_t i;
 
-       if (!zapi_nexthop_update_decode(zclient->ibuf, &nhr)) {
+       if (!zapi_nexthop_update_decode(zclient->ibuf, &matched, &nhr)) {
                zlog_err("Failure to decode Nexthop update message");
                return 0;
        }
 
        if (DEBUG_MODE_CHECK(&pbr_dbg_zebra, DEBUG_MODE_ALL)) {
 
-               DEBUGD(&pbr_dbg_zebra, "%s: Received Nexthop update: %pFX",
-                      __func__, &nhr.prefix);
+               DEBUGD(&pbr_dbg_zebra,
+                      "%s: Received Nexthop update: %pFX against %pFX",
+                      __func__, &matched, &nhr.prefix);
 
                DEBUGD(&pbr_dbg_zebra, "%s:   (Nexthops(%u)", __func__,
                       nhr.nexthop_num);
@@ -423,6 +425,7 @@ static int pbr_zebra_nexthop_update(ZAPI_CALLBACK_ARGS)
                }
        }
 
+       nhr.prefix = matched;
        pbr_nht_nexthop_update(&nhr);
        return 1;
 }
index 80d214b2f71de3063ab70e1648d389e625b1835b..4e7aad99f1f351db0306b444b2e9310768a3c85b 100644 (file)
@@ -702,19 +702,20 @@ int pim_parse_nexthop_update(ZAPI_CALLBACK_ARGS)
        struct vrf *vrf = vrf_lookup_by_id(vrf_id);
        struct pim_instance *pim;
        struct zapi_route nhr;
+       struct prefix match;
 
        if (!vrf)
                return 0;
        pim = vrf->info;
 
-       if (!zapi_nexthop_update_decode(zclient->ibuf, &nhr)) {
+       if (!zapi_nexthop_update_decode(zclient->ibuf, &match, &nhr)) {
                zlog_err("%s: Decode of nexthop update from zebra failed",
                         __func__);
                return 0;
        }
 
        if (cmd == ZEBRA_NEXTHOP_UPDATE) {
-               prefix_copy(&rpf.rpf_addr, &nhr.prefix);
+               prefix_copy(&rpf.rpf_addr, &match);
                pnc = pim_nexthop_cache_find(pim, &rpf);
                if (!pnc) {
                        if (PIM_DEBUG_PIM_NHT)
@@ -806,9 +807,9 @@ int pim_parse_nexthop_update(ZAPI_CALLBACK_ARGS)
                        if (PIM_DEBUG_PIM_NHT)
                                zlog_debug(
                                        "%s: NHT addr %pFX(%s) %d-nhop via %pI4(%s) type %d distance:%u metric:%u ",
-                                       __func__, &nhr.prefix, pim->vrf->name,
-                                       i + 1, &nexthop->gate.ipv4,
-                                       ifp->name, nexthop->type, nhr.distance,
+                                       __func__, &match, pim->vrf->name, i + 1,
+                                       &nexthop->gate.ipv4, ifp->name,
+                                       nexthop->type, nhr.distance,
                                        nhr.metric);
 
                        if (!ifp->info) {
@@ -862,7 +863,7 @@ int pim_parse_nexthop_update(ZAPI_CALLBACK_ARGS)
        if (PIM_DEBUG_PIM_NHT)
                zlog_debug(
                        "%s: NHT Update for %pFX(%s) num_nh %d num_pim_nh %d vrf:%u up %ld rp %d",
-                       __func__, &nhr.prefix, pim->vrf->name, nhr.nexthop_num,
+                       __func__, &match, pim->vrf->name, nhr.nexthop_num,
                        pnc->nexthop_num, vrf_id, pnc->upstream_hash->count,
                        listcount(pnc->rp_list));
 
index 313febd9bb6694ef9f27dece11254e5d2d63de84..5304b17f06fdcd4bcff6ab1630e383f4fc158e32 100644 (file)
@@ -680,16 +680,17 @@ static int sharp_nexthop_update(ZAPI_CALLBACK_ARGS)
 {
        struct sharp_nh_tracker *nht;
        struct zapi_route nhr;
+       struct prefix matched;
 
-       if (!zapi_nexthop_update_decode(zclient->ibuf, &nhr)) {
+       if (!zapi_nexthop_update_decode(zclient->ibuf, &matched, &nhr)) {
                zlog_err("%s: Decode of update failed", __func__);
                return 0;
        }
 
-       zlog_debug("Received update for %pFX metric: %u", &nhr.prefix,
-                  nhr.metric);
+       zlog_debug("Received update for %pFX actual match: %pFX metric: %u",
+                  &matched, &nhr.prefix, nhr.metric);
 
-       nht = sharp_nh_tracker_get(&nhr.prefix);
+       nht = sharp_nh_tracker_get(&matched);
        nht->nhop_num = nhr.nexthop_num;
        nht->updates++;
 
index f937492ec21796be01e3685a77ade0c4c99c4ffe..bd293edebc15ae0cdb0af7d0828f2154588fc2f8 100644 (file)
@@ -169,24 +169,25 @@ static int static_zebra_nexthop_update(ZAPI_CALLBACK_ARGS)
 {
        struct static_nht_data *nhtd, lookup;
        struct zapi_route nhr;
+       struct prefix matched;
        afi_t afi = AFI_IP;
 
-       if (!zapi_nexthop_update_decode(zclient->ibuf, &nhr)) {
+       if (!zapi_nexthop_update_decode(zclient->ibuf, &matched, &nhr)) {
                zlog_err("Failure to decode nexthop update message");
                return 1;
        }
 
-       if (nhr.prefix.family == AF_INET6)
+       if (matched.family == AF_INET6)
                afi = AFI_IP6;
 
        if (nhr.type == ZEBRA_ROUTE_CONNECT) {
-               if (static_nexthop_is_local(vrf_id, &nhr.prefix,
-                                       nhr.prefix.family))
+               if (static_nexthop_is_local(vrf_id, &matched,
+                                           nhr.prefix.family))
                        nhr.nexthop_num = 0;
        }
 
        memset(&lookup, 0, sizeof(lookup));
-       lookup.nh = &nhr.prefix;
+       lookup.nh = &matched;
        lookup.nh_vrf_id = vrf_id;
 
        nhtd = hash_lookup(static_nht_hash, &lookup);
@@ -194,8 +195,8 @@ static int static_zebra_nexthop_update(ZAPI_CALLBACK_ARGS)
        if (nhtd) {
                nhtd->nh_num = nhr.nexthop_num;
 
-               static_nht_reset_start(&nhr.prefix, afi, nhtd->nh_vrf_id);
-               static_nht_update(NULL, &nhr.prefix, nhr.nexthop_num, afi,
+               static_nht_reset_start(&matched, afi, nhtd->nh_vrf_id);
+               static_nht_update(NULL, &matched, nhr.nexthop_num, afi,
                                  nhtd->nh_vrf_id);
        } else
                zlog_err("No nhtd?");
index 8ca25359beb565f209c7d077df240f167f710fdb..f90eb5bee1a5021580aa20e58db745c52c7a76fb 100644 (file)
@@ -1169,6 +1169,9 @@ int zebra_send_rnh_update(struct rnh *rnh, struct zserv *client,
                SET_FLAG(message, ZAPI_MESSAGE_SRTE);
        stream_putl(s, message);
 
+       /*
+        * Put what we were told to match against
+        */
        stream_putw(s, rnh->safi);
        stream_putw(s, rn->p.family);
        switch (rn->p.family) {
@@ -1186,6 +1189,26 @@ int zebra_send_rnh_update(struct rnh *rnh, struct zserv *client,
                         __func__, rn->p.family);
                goto failure;
        }
+
+       /*
+        * What we matched against
+        */
+       stream_putw(s, rnh->resolved_route.family);
+       stream_putc(s, rnh->resolved_route.prefixlen);
+       switch (rnh->resolved_route.family) {
+       case AF_INET:
+               stream_put_in_addr(s, &rnh->resolved_route.u.prefix4);
+               break;
+       case AF_INET6:
+               stream_put(s, &rnh->resolved_route.u.prefix6, IPV6_MAX_BYTELEN);
+               break;
+       default:
+               flog_err(EC_ZEBRA_RNH_UNKNOWN_FAMILY,
+                        "%s: Unknown family (%d) notification attempted",
+                        __func__, rn->p.family);
+               goto failure;
+       }
+
        if (srte_color)
                stream_putl(s, srte_color);
 
index 7933ef66b147d02834267e104210a10fe5a1a4b0..c0f18dd09160085ddbeced7a34a8e17d46d14c5c 100644 (file)
@@ -117,13 +117,25 @@ static int zebra_sr_policy_notify_update_client(struct zebra_sr_policy *policy,
        stream_putl(s, message);
 
        stream_putw(s, SAFI_UNICAST);
+       /*
+        * The prefix is copied twice because the ZEBRA_NEXTHOP_UPDATE
+        * code was modified to send back both the matched against
+        * as well as the actual matched.  There does not appear to
+        * be an equivalent here so just send the same thing twice.
+        */
        switch (policy->endpoint.ipa_type) {
        case IPADDR_V4:
+               stream_putw(s, AF_INET);
+               stream_putc(s, IPV4_MAX_BITLEN);
+               stream_put_in_addr(s, &policy->endpoint.ipaddr_v4);
                stream_putw(s, AF_INET);
                stream_putc(s, IPV4_MAX_BITLEN);
                stream_put_in_addr(s, &policy->endpoint.ipaddr_v4);
                break;
        case IPADDR_V6:
+               stream_putw(s, AF_INET6);
+               stream_putc(s, IPV6_MAX_BITLEN);
+               stream_put(s, &policy->endpoint.ipaddr_v6, IPV6_MAX_BYTELEN);
                stream_putw(s, AF_INET6);
                stream_putc(s, IPV6_MAX_BITLEN);
                stream_put(s, &policy->endpoint.ipaddr_v6, IPV6_MAX_BYTELEN);