*: 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 <mjs@voltanet.io>

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

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) {

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));
+				prefix2str(&api->prefix, buf, sizeof(buf));
+
 				flog_err(EC_LIB_ZAPI_ENCODE,
-						 "%s: prefix %s: can't encode "
+					 "%s: prefix %s: can't encode %u labels (maximum is %u)",
-						 "%u labels (maximum is %u)",
 					 __func__, buf,
 					 api_nh->label_num,
 					 MPLS_MAX_LABELS);
 				return -1;
 			}
 
-				stream_putc(s, api_nh->label_num);
+			if (zapi_nexthop_encode(s, api_nh, api->flags) != 0)
-				stream_put(s, &api_nh->labels[0],
+				return -1;
-					   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));
 		}
 	}
 
@@ -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,56 +1188,9 @@ 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);
+			if (zapi_nexthop_decode(s, api_nh, api->flags) != 0)
-			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));
-		}
 	}
 
 	/* Attributes. */
@@ -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);
+		if (zapi_nexthop_decode(s, &(nhr->nexthops[i]), 0) != 0)
-		STREAM_GETC(s, nhr->nexthops[i].type);
+			return -1;
-		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));
 	}
 
 	return true;

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);
 

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];

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",
+		zlog_debug("%s: p=%s, flags=0x%x",
-			   __func__, buf_prefix,
+			   __func__, buf_prefix, api.flags);
-			   (CHECK_FLAG(api.message, ZAPI_MESSAGE_LABEL) ? ""
-									: "un"),
-			   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;

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);
+				zapi_nexthop_from_nexthop(&znh, nh);
-				stream_putc(s, nh->type);
+				zapi_nexthop_encode(s, &znh, 0 /* flags */);
-				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);
 				num++;
 			}
 		stream_putc_at(s, nump, num);