From 68a02e06e5f103048d947262c08c569056f74d1c Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Wed, 13 Nov 2019 16:06:06 -0500 Subject: [PATCH] *: revise zapi nexthop encoding Use a per-nexthop flag to indicate the presence of labels; add some utility zapi encode/decode apis for nexthops; use the zapi apis more consistently. Signed-off-by: Mark Stapp --- bgpd/bgp_zebra.c | 11 +- isisd/isis_zebra.c | 2 +- lib/zclient.c | 314 ++++++++++++++++++++++++----------------- lib/zclient.h | 13 +- staticd/static_zebra.c | 5 +- zebra/zapi_msg.c | 11 +- zebra/zebra_rnh.c | 35 +---- 7 files changed, 210 insertions(+), 181 deletions(-) diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index e886733ce..7ee8612e2 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1072,7 +1072,7 @@ static int update_ipv4nh_for_route_install(int nh_othervrf, */ if (is_evpn) { api_nh->type = NEXTHOP_TYPE_IPV4_IFINDEX; - api_nh->onlink = true; + SET_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_ONLINK); api_nh->ifindex = nh_bgp->l3vni_svi_ifindex; } else if (nh_othervrf && api_nh->gate.ipv4.s_addr == INADDR_ANY) { @@ -1098,7 +1098,7 @@ update_ipv6nh_for_route_install(int nh_othervrf, struct bgp *nh_bgp, if (is_evpn) { api_nh->type = NEXTHOP_TYPE_IPV6_IFINDEX; - api_nh->onlink = true; + SET_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_ONLINK); api_nh->ifindex = nh_bgp->l3vni_svi_ifindex; } else if (nh_othervrf) { if (IN6_IS_ADDR_UNSPECIFIED(nexthop)) { @@ -1347,6 +1347,8 @@ void bgp_zebra_announce(struct bgp_node *rn, struct prefix *p, has_valid_label = 1; label = label_pton(&mpinfo->extra->label[0]); + SET_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_LABEL); + api_nh->label_num = 1; api_nh->labels[0] = label; } @@ -1355,11 +1357,6 @@ void bgp_zebra_announce(struct bgp_node *rn, struct prefix *p, valid_nh_count++; } - - /* if this is a evpn route we don't have to include the label */ - if (has_valid_label && !(CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE))) - SET_FLAG(api.message, ZAPI_MESSAGE_LABEL); - /* * When we create an aggregate route we must also * install a Null0 route in the RIB, so overwrite diff --git a/isisd/isis_zebra.c b/isisd/isis_zebra.c index b4c699ccb..630264768 100644 --- a/isisd/isis_zebra.c +++ b/isisd/isis_zebra.c @@ -184,7 +184,7 @@ void isis_zebra_route_add_route(struct prefix *prefix, break; api_nh = &api.nexthops[count]; if (fabricd) - api_nh->onlink = true; + SET_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_ONLINK); api_nh->vrf_id = VRF_DEFAULT; switch (nexthop->family) { diff --git a/lib/zclient.c b/lib/zclient.c index 7a62e408e..5ce1150b0 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -859,6 +859,76 @@ static void zapi_nexthop_group_sort(struct zapi_nexthop *nh_grp, &zapi_nexthop_cmp); } +/* + * Encode a single zapi nexthop + */ +int zapi_nexthop_encode(struct stream *s, const struct zapi_nexthop *api_nh, + uint32_t api_flags) +{ + int ret = 0; + int nh_flags = api_nh->flags; + + stream_putl(s, api_nh->vrf_id); + stream_putc(s, api_nh->type); + + /* If needed, set 'labelled nexthop' flag */ + if (api_nh->label_num > 0) { + SET_FLAG(nh_flags, ZAPI_NEXTHOP_FLAG_LABEL); + + /* Validate label count */ + if (api_nh->label_num > MPLS_MAX_LABELS) { + ret = -1; + goto done; + } + } + + /* Note that we're only encoding a single octet */ + stream_putc(s, nh_flags); + + switch (api_nh->type) { + case NEXTHOP_TYPE_BLACKHOLE: + stream_putc(s, api_nh->bh_type); + break; + case NEXTHOP_TYPE_IPV4: + stream_put_in_addr(s, &api_nh->gate.ipv4); + break; + case NEXTHOP_TYPE_IPV4_IFINDEX: + stream_put_in_addr(s, &api_nh->gate.ipv4); + stream_putl(s, api_nh->ifindex); + break; + case NEXTHOP_TYPE_IFINDEX: + stream_putl(s, api_nh->ifindex); + break; + case NEXTHOP_TYPE_IPV6: + stream_write(s, (uint8_t *)&api_nh->gate.ipv6, + 16); + break; + case NEXTHOP_TYPE_IPV6_IFINDEX: + stream_write(s, (uint8_t *)&api_nh->gate.ipv6, + 16); + stream_putl(s, api_nh->ifindex); + break; + } + + /* We only encode labels if we have >0 - we use + * the per-nexthop flag above to signal that the count + * is present in the payload. + */ + if (api_nh->label_num > 0) { + stream_putc(s, api_nh->label_num); + stream_put(s, &api_nh->labels[0], + api_nh->label_num * sizeof(mpls_label_t)); + } + + /* Router MAC for EVPN routes. */ + if (CHECK_FLAG(api_flags, ZEBRA_FLAG_EVPN_ROUTE)) + stream_put(s, &(api_nh->rmac), + sizeof(struct ethaddr)); + +done: + return ret; +} + int zapi_route_encode(uint8_t cmd, struct stream *s, struct zapi_route *api) { struct zapi_nexthop *api_nh; @@ -921,59 +991,22 @@ int zapi_route_encode(uint8_t cmd, struct stream *s, struct zapi_route *api) for (i = 0; i < api->nexthop_num; i++) { api_nh = &api->nexthops[i]; - stream_putl(s, api_nh->vrf_id); - stream_putc(s, api_nh->type); - stream_putc(s, api_nh->onlink); - switch (api_nh->type) { - case NEXTHOP_TYPE_BLACKHOLE: - stream_putc(s, api_nh->bh_type); - break; - case NEXTHOP_TYPE_IPV4: - stream_put_in_addr(s, &api_nh->gate.ipv4); - break; - case NEXTHOP_TYPE_IPV4_IFINDEX: - stream_put_in_addr(s, &api_nh->gate.ipv4); - stream_putl(s, api_nh->ifindex); - break; - case NEXTHOP_TYPE_IFINDEX: - stream_putl(s, api_nh->ifindex); - break; - case NEXTHOP_TYPE_IPV6: - stream_write(s, (uint8_t *)&api_nh->gate.ipv6, - 16); - break; - case NEXTHOP_TYPE_IPV6_IFINDEX: - stream_write(s, (uint8_t *)&api_nh->gate.ipv6, - 16); - stream_putl(s, api_nh->ifindex); - break; - } - /* MPLS labels for BGP-LU or Segment Routing */ - if (CHECK_FLAG(api->message, ZAPI_MESSAGE_LABEL)) { - if (api_nh->label_num > MPLS_MAX_LABELS) { - char buf[PREFIX2STR_BUFFER]; - prefix2str(&api->prefix, buf, - sizeof(buf)); - flog_err(EC_LIB_ZAPI_ENCODE, - "%s: prefix %s: can't encode " - "%u labels (maximum is %u)", - __func__, buf, - api_nh->label_num, - MPLS_MAX_LABELS); - return -1; - } - - stream_putc(s, api_nh->label_num); - stream_put(s, &api_nh->labels[0], - api_nh->label_num - * sizeof(mpls_label_t)); + if (api_nh->label_num > MPLS_MAX_LABELS) { + char buf[PREFIX2STR_BUFFER]; + + prefix2str(&api->prefix, buf, sizeof(buf)); + + flog_err(EC_LIB_ZAPI_ENCODE, + "%s: prefix %s: can't encode %u labels (maximum is %u)", + __func__, buf, + api_nh->label_num, + MPLS_MAX_LABELS); + return -1; } - /* Router MAC for EVPN routes. */ - if (CHECK_FLAG(api->flags, ZEBRA_FLAG_EVPN_ROUTE)) - stream_put(s, &(api_nh->rmac), - sizeof(struct ethaddr)); + if (zapi_nexthop_encode(s, api_nh, api->flags) != 0) + return -1; } } @@ -995,6 +1028,73 @@ int zapi_route_encode(uint8_t cmd, struct stream *s, struct zapi_route *api) return 0; } +/* + * Decode a single zapi nexthop object + */ +static int zapi_nexthop_decode(struct stream *s, struct zapi_nexthop *api_nh, + uint32_t api_flags) +{ + int ret = -1; + + STREAM_GETL(s, api_nh->vrf_id); + STREAM_GETC(s, api_nh->type); + + /* Note that we're only using a single octet of flags */ + STREAM_GETC(s, api_nh->flags); + + switch (api_nh->type) { + case NEXTHOP_TYPE_BLACKHOLE: + STREAM_GETC(s, api_nh->bh_type); + break; + case NEXTHOP_TYPE_IPV4: + STREAM_GET(&api_nh->gate.ipv4.s_addr, s, + IPV4_MAX_BYTELEN); + break; + case NEXTHOP_TYPE_IPV4_IFINDEX: + STREAM_GET(&api_nh->gate.ipv4.s_addr, s, + IPV4_MAX_BYTELEN); + STREAM_GETL(s, api_nh->ifindex); + break; + case NEXTHOP_TYPE_IFINDEX: + STREAM_GETL(s, api_nh->ifindex); + break; + case NEXTHOP_TYPE_IPV6: + STREAM_GET(&api_nh->gate.ipv6, s, 16); + break; + case NEXTHOP_TYPE_IPV6_IFINDEX: + STREAM_GET(&api_nh->gate.ipv6, s, 16); + STREAM_GETL(s, api_nh->ifindex); + break; + } + + /* MPLS labels for BGP-LU or Segment Routing */ + if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_LABEL)) { + STREAM_GETC(s, api_nh->label_num); + if (api_nh->label_num > MPLS_MAX_LABELS) { + flog_err( + EC_LIB_ZAPI_ENCODE, + "%s: invalid number of MPLS labels (%u)", + __func__, api_nh->label_num); + return -1; + } + + STREAM_GET(&api_nh->labels[0], s, + api_nh->label_num * sizeof(mpls_label_t)); + } + + /* Router MAC for EVPN routes. */ + if (CHECK_FLAG(api_flags, ZEBRA_FLAG_EVPN_ROUTE)) + STREAM_GET(&(api_nh->rmac), s, + sizeof(struct ethaddr)); + + /* Success */ + ret = 0; + +stream_failure: + + return ret; +} + int zapi_route_decode(struct stream *s, struct zapi_route *api) { struct zapi_nexthop *api_nh; @@ -1088,55 +1188,8 @@ int zapi_route_decode(struct stream *s, struct zapi_route *api) for (i = 0; i < api->nexthop_num; i++) { api_nh = &api->nexthops[i]; - STREAM_GETL(s, api_nh->vrf_id); - STREAM_GETC(s, api_nh->type); - STREAM_GETC(s, api_nh->onlink); - switch (api_nh->type) { - case NEXTHOP_TYPE_BLACKHOLE: - STREAM_GETC(s, api_nh->bh_type); - break; - case NEXTHOP_TYPE_IPV4: - STREAM_GET(&api_nh->gate.ipv4.s_addr, s, - IPV4_MAX_BYTELEN); - break; - case NEXTHOP_TYPE_IPV4_IFINDEX: - STREAM_GET(&api_nh->gate.ipv4.s_addr, s, - IPV4_MAX_BYTELEN); - STREAM_GETL(s, api_nh->ifindex); - break; - case NEXTHOP_TYPE_IFINDEX: - STREAM_GETL(s, api_nh->ifindex); - break; - case NEXTHOP_TYPE_IPV6: - STREAM_GET(&api_nh->gate.ipv6, s, 16); - break; - case NEXTHOP_TYPE_IPV6_IFINDEX: - STREAM_GET(&api_nh->gate.ipv6, s, 16); - STREAM_GETL(s, api_nh->ifindex); - break; - } - - /* MPLS labels for BGP-LU or Segment Routing */ - if (CHECK_FLAG(api->message, ZAPI_MESSAGE_LABEL)) { - STREAM_GETC(s, api_nh->label_num); - - if (api_nh->label_num > MPLS_MAX_LABELS) { - flog_err( - EC_LIB_ZAPI_ENCODE, - "%s: invalid number of MPLS labels (%u)", - __func__, api_nh->label_num); - return -1; - } - - STREAM_GET(&api_nh->labels[0], s, - api_nh->label_num - * sizeof(mpls_label_t)); - } - - /* Router MAC for EVPN routes. */ - if (CHECK_FLAG(api->flags, ZEBRA_FLAG_EVPN_ROUTE)) - stream_get(&(api_nh->rmac), s, - sizeof(struct ethaddr)); + if (zapi_nexthop_decode(s, api_nh, api->flags) != 0) + return -1; } } @@ -1335,6 +1388,35 @@ struct nexthop *nexthop_from_zapi_nexthop(struct zapi_nexthop *znh) return n; } +/* + * Convert nexthop to zapi nexthop + */ +int zapi_nexthop_from_nexthop(struct zapi_nexthop *znh, + const struct nexthop *nh) +{ + int i; + + memset(znh, 0, sizeof(*znh)); + + znh->type = nh->type; + znh->vrf_id = nh->vrf_id; + znh->ifindex = nh->ifindex; + znh->gate = nh->gate; + + if (nh->nh_label && (nh->nh_label->num_labels > 0)) { + for (i = 0; i < nh->nh_label->num_labels; i++) + znh->labels[i] = nh->nh_label->label[i]; + + znh->label_num = i; + SET_FLAG(znh->flags, ZAPI_NEXTHOP_FLAG_LABEL); + } + + return 0; +} + +/* + * Decode the nexthop-tracking update message + */ bool zapi_nexthop_update_decode(struct stream *s, struct zapi_route *nhr) { uint32_t i; @@ -1361,38 +1443,8 @@ bool zapi_nexthop_update_decode(struct stream *s, struct zapi_route *nhr) STREAM_GETC(s, nhr->nexthop_num); for (i = 0; i < nhr->nexthop_num; i++) { - STREAM_GETL(s, nhr->nexthops[i].vrf_id); - STREAM_GETC(s, nhr->nexthops[i].type); - switch (nhr->nexthops[i].type) { - case NEXTHOP_TYPE_IPV4: - case NEXTHOP_TYPE_IPV4_IFINDEX: - STREAM_GET(&nhr->nexthops[i].gate.ipv4.s_addr, s, - IPV4_MAX_BYTELEN); - STREAM_GETL(s, nhr->nexthops[i].ifindex); - break; - case NEXTHOP_TYPE_IFINDEX: - STREAM_GETL(s, nhr->nexthops[i].ifindex); - break; - case NEXTHOP_TYPE_IPV6: - case NEXTHOP_TYPE_IPV6_IFINDEX: - STREAM_GET(&nhr->nexthops[i].gate.ipv6, s, - IPV6_MAX_BYTELEN); - STREAM_GETL(s, nhr->nexthops[i].ifindex); - break; - case NEXTHOP_TYPE_BLACKHOLE: - break; - } - STREAM_GETC(s, nhr->nexthops[i].label_num); - if (nhr->nexthops[i].label_num > MPLS_MAX_LABELS) { - flog_err(EC_LIB_ZAPI_ENCODE, - "%s: invalid number of MPLS labels (%u)", - __func__, nhr->nexthops[i].label_num); - return false; - } - if (nhr->nexthops[i].label_num) - STREAM_GET(&nhr->nexthops[i].labels[0], s, - nhr->nexthops[i].label_num - * sizeof(mpls_label_t)); + if (zapi_nexthop_decode(s, &(nhr->nexthops[i]), 0) != 0) + return -1; } return true; diff --git a/lib/zclient.h b/lib/zclient.h index 7adb294a3..ba5b0c615 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -289,7 +289,6 @@ struct zclient { #define ZAPI_MESSAGE_TAG 0x08 #define ZAPI_MESSAGE_MTU 0x10 #define ZAPI_MESSAGE_SRCPFX 0x20 -#define ZAPI_MESSAGE_LABEL 0x40 /* * This should only be used by a DAEMON that needs to communicate * the table being used is not in the VRF. You must pass the @@ -313,7 +312,7 @@ struct zapi_nexthop { enum nexthop_types_t type; vrf_id_t vrf_id; ifindex_t ifindex; - bool onlink; + uint8_t flags; union { union g_addr gate; enum blackhole_type bh_type; @@ -326,6 +325,12 @@ struct zapi_nexthop { struct ethaddr rmac; }; +/* + * ZAPI nexthop flags values + */ +#define ZAPI_NEXTHOP_FLAG_ONLINK 0x01 +#define ZAPI_NEXTHOP_FLAG_LABEL 0x02 + /* * Some of these data structures do not map easily to * a actual data structure size giving different compilers @@ -666,6 +671,8 @@ extern int zclient_route_send(uint8_t, struct zclient *, struct zapi_route *); extern int zclient_send_rnh(struct zclient *zclient, int command, struct prefix *p, bool exact_match, vrf_id_t vrf_id); +int zapi_nexthop_encode(struct stream *s, const struct zapi_nexthop *api_nh, + uint32_t api_flags); extern int zapi_route_encode(uint8_t, struct stream *, struct zapi_route *); extern int zapi_route_decode(struct stream *, struct zapi_route *); bool zapi_route_notify_decode(struct stream *s, struct prefix *p, @@ -690,6 +697,8 @@ bool zapi_iptable_notify_decode(struct stream *s, enum zapi_iptable_notify_owner *note); extern struct nexthop *nexthop_from_zapi_nexthop(struct zapi_nexthop *znh); +int zapi_nexthop_from_nexthop(struct zapi_nexthop *znh, + const struct nexthop *nh); extern bool zapi_nexthop_update_decode(struct stream *s, struct zapi_route *nhr); diff --git a/staticd/static_zebra.c b/staticd/static_zebra.c index a474613b4..42646d15b 100644 --- a/staticd/static_zebra.c +++ b/staticd/static_zebra.c @@ -388,7 +388,8 @@ extern void static_zebra_route_add(struct route_node *rn, continue; api_nh->vrf_id = si->nh_vrf_id; - api_nh->onlink = si->onlink; + if (si->onlink) + SET_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_ONLINK); si->state = STATIC_SENT_TO_ZEBRA; @@ -441,7 +442,7 @@ extern void static_zebra_route_add(struct route_node *rn, if (si->snh_label.num_labels) { int i; - SET_FLAG(api.message, ZAPI_MESSAGE_LABEL); + SET_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_LABEL); api_nh->label_num = si->snh_label.num_labels; for (i = 0; i < api_nh->label_num; i++) api_nh->labels[i] = si->snh_label.label[i]; diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index bf29bda77..e1654a1a3 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1411,11 +1411,8 @@ static void zread_route_add(ZAPI_HANDLER_ARGS) char buf_prefix[PREFIX_STRLEN]; prefix2str(&api.prefix, buf_prefix, sizeof(buf_prefix)); - zlog_debug("%s: p=%s, ZAPI_MESSAGE_LABEL: %sset, flags=0x%x", - __func__, buf_prefix, - (CHECK_FLAG(api.message, ZAPI_MESSAGE_LABEL) ? "" - : "un"), - api.flags); + zlog_debug("%s: p=%s, flags=0x%x", + __func__, buf_prefix, api.flags); } /* Allocate new route. */ @@ -1544,11 +1541,11 @@ static void zread_route_add(ZAPI_HANDLER_ARGS) return; } - if (api_nh->onlink) + if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_ONLINK)) SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK); /* MPLS labels for BGP-LU or Segment Routing */ - if (CHECK_FLAG(api.message, ZAPI_MESSAGE_LABEL) + if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_LABEL) && api_nh->type != NEXTHOP_TYPE_IFINDEX && api_nh->type != NEXTHOP_TYPE_BLACKHOLE) { enum lsp_types_t label_type; diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 9a6631a59..2d9c83bec 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -1034,6 +1034,8 @@ static int send_client(struct rnh *rnh, struct zserv *client, rnh_type_t type, break; } if (re) { + struct zapi_nexthop znh; + stream_putc(s, re->type); stream_putw(s, re->instance); stream_putc(s, re->distance); @@ -1043,37 +1045,8 @@ static int send_client(struct rnh *rnh, struct zserv *client, rnh_type_t type, stream_putc(s, 0); for (ALL_NEXTHOPS_PTR(re->nhe->nhg, nh)) if (rnh_nexthop_valid(re, nh)) { - stream_putl(s, nh->vrf_id); - stream_putc(s, nh->type); - switch (nh->type) { - case NEXTHOP_TYPE_IPV4: - case NEXTHOP_TYPE_IPV4_IFINDEX: - stream_put_in_addr(s, &nh->gate.ipv4); - stream_putl(s, nh->ifindex); - break; - case NEXTHOP_TYPE_IFINDEX: - stream_putl(s, nh->ifindex); - break; - case NEXTHOP_TYPE_IPV6: - case NEXTHOP_TYPE_IPV6_IFINDEX: - stream_put(s, &nh->gate.ipv6, 16); - stream_putl(s, nh->ifindex); - break; - default: - /* do nothing */ - break; - } - if (nh->nh_label) { - stream_putc(s, - nh->nh_label->num_labels); - if (nh->nh_label->num_labels) - stream_put( - s, - &nh->nh_label->label[0], - nh->nh_label->num_labels - * sizeof(mpls_label_t)); - } else - stream_putc(s, 0); + zapi_nexthop_from_nexthop(&znh, nh); + zapi_nexthop_encode(s, &znh, 0 /* flags */); num++; } stream_putc_at(s, nump, num); -- 2.39.5