diff options
author | Donald Sharp <sharpd@nvidia.com> | 2022-03-12 16:47:16 +0100 |
---|---|---|
committer | Donald Sharp <sharpd@nvidia.com> | 2022-03-12 17:18:45 +0100 |
commit | 06e4e90132ad23815c6f288dd7e6be334f5ab233 (patch) | |
tree | d2e36926ed80e04a6b948baafc3a4bb065b7c02c | |
parent | zebra: Remove unused ZEBRA_NHT_EXACT_MATCH (diff) | |
download | frr-06e4e90132ad23815c6f288dd7e6be334f5ab233.tar.xz frr-06e4e90132ad23815c6f288dd7e6be334f5ab233.zip |
*: When matching against a nexthop send and process what it matched against
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>
-rw-r--r-- | bgpd/bgp_nht.c | 9 | ||||
-rw-r--r-- | lib/zclient.c | 19 | ||||
-rw-r--r-- | lib/zclient.h | 12 | ||||
-rw-r--r-- | ospf6d/ospf6_zebra.c | 7 | ||||
-rw-r--r-- | pbrd/pbr_zebra.c | 9 | ||||
-rw-r--r-- | pimd/pim_nht.c | 13 | ||||
-rw-r--r-- | sharpd/sharp_zebra.c | 9 | ||||
-rw-r--r-- | staticd/static_zebra.c | 15 | ||||
-rw-r--r-- | zebra/zebra_rnh.c | 23 | ||||
-rw-r--r-- | zebra/zebra_srte.c | 12 |
10 files changed, 99 insertions, 29 deletions
diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 8313c12e6..a3330a6a5 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -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( diff --git a/lib/zclient.c b/lib/zclient.c index 930adf6a7..f6c5a8af0 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -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) { diff --git a/lib/zclient.h b/lib/zclient.h index ca62b1afe..092754f60 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -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); diff --git a/ospf6d/ospf6_zebra.c b/ospf6d/ospf6_zebra.c index e279d0411..4b37c5bc2 100644 --- a/ospf6d/ospf6_zebra.c +++ b/ospf6d/ospf6_zebra.c @@ -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; diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c index b480d4072..f99258872 100644 --- a/pbrd/pbr_zebra.c +++ b/pbrd/pbr_zebra.c @@ -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; } diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 80d214b2f..4e7aad99f 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -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)); diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index 313febd9b..5304b17f0 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -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++; diff --git a/staticd/static_zebra.c b/staticd/static_zebra.c index f937492ec..bd293edeb 100644 --- a/staticd/static_zebra.c +++ b/staticd/static_zebra.c @@ -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?"); diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 8ca25359b..f90eb5bee 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -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); diff --git a/zebra/zebra_srte.c b/zebra/zebra_srte.c index 7933ef66b..c0f18dd09 100644 --- a/zebra/zebra_srte.c +++ b/zebra/zebra_srte.c @@ -117,16 +117,28 @@ 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); break; default: flog_warn(EC_LIB_DEVELOPMENT, |