zebra: Refactor nexthop reading from zapi messages

Take the zebra code that reads nexthops and combine it
into one function so that when we add zapi messages
to send/receive nexthops we can take advantage of this function.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

1 files changed, 127 insertions, 152 deletions

diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c
index 6d7e84235..1301a577f 100644
--- a/zebra/zapi_msg.c
+++ b/zebra/zapi_msg.c
@@ -1578,99 +1578,72 @@ done:
 	return nexthop;
 }
 
-static void zread_route_add(ZAPI_HANDLER_ARGS)
+static bool zapi_read_nexthops(struct zserv *client, struct zapi_route *api,
+			       struct route_entry *re,
+			       struct nexthop_group **png,
+			       struct nhg_backup_info **pbnhg)
 {
-	struct stream *s;
-	struct zapi_route api;
-	struct zapi_nexthop *api_nh;
-	afi_t afi;
-	struct prefix_ipv6 *src_p = NULL;
-	struct route_entry *re;
-	struct nexthop *nexthop = NULL, *last_nh;
 	struct nexthop_group *ng = NULL;
 	struct nhg_backup_info *bnhg = NULL;
-	int i, ret;
-	vrf_id_t vrf_id;
-	struct nhg_hash_entry nhe;
-	enum lsp_types_t label_type;
-	char nhbuf[NEXTHOP_STRLEN];
-	char labelbuf[MPLS_LABEL_STRLEN];
-
-	s = msg;
-	if (zapi_route_decode(s, &api) < 0) {
-		if (IS_ZEBRA_DEBUG_RECV)
-			zlog_debug("%s: Unable to decode zapi_route sent",
-				   __func__);
-		return;
-	}
+	uint16_t nexthop_num, i;;
+	struct zapi_nexthop *nhops;
+	struct nexthop *last_nh = NULL;
 
-	vrf_id = zvrf_id(zvrf);
+	assert(!(png && pbnhg));
 
-	if (IS_ZEBRA_DEBUG_RECV) {
-		char buf_prefix[PREFIX_STRLEN];
-
-		prefix2str(&api.prefix, buf_prefix, sizeof(buf_prefix));
-		zlog_debug("%s: p=(%u:%u)%s, msg flags=0x%x, flags=0x%x",
-			   __func__, vrf_id, api.tableid, buf_prefix, (int)api.message, api.flags);
+	if (png) {
+		*png = ng = nexthop_group_new();
+		nexthop_num = api->nexthop_num;
+		nhops = api->nexthops;
 	}
 
-	/* Allocate new route. */
-	re = XCALLOC(MTYPE_RE, sizeof(struct route_entry));
-	re->type = api.type;
-	re->instance = api.instance;
-	re->flags = api.flags;
-	re->uptime = monotime(NULL);
-	re->vrf_id = vrf_id;
-
-	if (api.tableid)
-		re->table = api.tableid;
-	else
-		re->table = zvrf->table_id;
-
-	if (!CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP)
-	    || api.nexthop_num == 0) {
-		flog_warn(EC_ZEBRA_RX_ROUTE_NO_NEXTHOPS,
-			  "%s: received a route without nexthops for prefix %pFX from client %s",
-			  __func__, &api.prefix,
-			  zebra_route_string(client->proto));
-
-		XFREE(MTYPE_RE, re);
-		return;
-	}
+	if (pbnhg && api->backup_nexthop_num > 0) {
+		if (IS_ZEBRA_DEBUG_RECV)
+			zlog_debug("%s: adding %d backup nexthops",
+				   __func__, api->backup_nexthop_num);
 
-	/* Report misuse of the backup flag */
-	if (CHECK_FLAG(api.message, ZAPI_MESSAGE_BACKUP_NEXTHOPS) &&
-	    api.backup_nexthop_num == 0) {
-		if (IS_ZEBRA_DEBUG_RECV || IS_ZEBRA_DEBUG_EVENT)
-			zlog_debug("%s: client %s: BACKUP flag set but no backup nexthops, prefix %pFX",
-				__func__,
-				zebra_route_string(client->proto), &api.prefix);
+		nexthop_num = api->backup_nexthop_num;
+		*pbnhg = bnhg = zebra_nhg_backup_alloc();
+		nhops = api->backup_nexthops;
 	}
 
-	/* Use temporary list of nexthops */
-	ng = nexthop_group_new();
-
 	/*
 	 * TBD should _all_ of the nexthop add operations use
 	 * api_nh->vrf_id instead of re->vrf_id ? I only changed
 	 * for cases NEXTHOP_TYPE_IPV4 and NEXTHOP_TYPE_IPV6.
 	 */
-	for (i = 0; i < api.nexthop_num; i++) {
-		api_nh = &api.nexthops[i];
+	for (i = 0; i < nexthop_num; i++) {
+		struct nexthop *nexthop;
+		enum lsp_types_t label_type;
+		char nhbuf[NEXTHOP_STRLEN];
+		char labelbuf[MPLS_LABEL_STRLEN];
+		struct zapi_nexthop *api_nh = &nhops[i];
 
 		/* Convert zapi nexthop */
-		nexthop = nexthop_from_zapi(re, api_nh, &api);
+		nexthop = nexthop_from_zapi(re, api_nh, api);
 		if (!nexthop) {
 			flog_warn(
 				EC_ZEBRA_NEXTHOP_CREATION_FAILED,
-				"%s: Nexthops Specified: %d but we failed to properly create one",
-				__func__, api.nexthop_num);
-			nexthop_group_delete(&ng);
-			XFREE(MTYPE_RE, re);
-			return;
+				"%s: Nexthops Specified: %u(%u) but we failed to properly create one",
+				__func__, nexthop_num, i);
+			if (ng)
+				nexthop_group_delete(&ng);
+			if (bnhg)
+				zebra_nhg_backup_free(&bnhg);
+			return false;
 		}
 
-		if (CHECK_FLAG(api.message, ZAPI_MESSAGE_SRTE)) {
+		if (bnhg && CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) {
+			if (IS_ZEBRA_DEBUG_RECV) {
+				nexthop2str(nexthop, nhbuf, sizeof(nhbuf));
+				zlog_debug("%s: backup nh %s with BACKUP flag!",
+					   __func__, nhbuf);
+			}
+			UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP);
+			nexthop->backup_num = 0;
+		}
+
+		if (CHECK_FLAG(api->message, ZAPI_MESSAGE_SRTE)) {
 			SET_FLAG(nexthop->flags, NEXTHOP_FLAG_SRTE);
 			nexthop->srte_color = api_nh->srte_color;
 		}
@@ -1705,100 +1678,102 @@ static void zread_route_add(ZAPI_HANDLER_ARGS)
 				   __func__, nhbuf, api_nh->vrf_id, labelbuf);
 		}
 
-		/* Add new nexthop to temporary list. This list is
-		 * canonicalized - sorted - so that it can be hashed later
-		 * in route processing. We expect that the sender has sent
-		 * the list sorted, and the zapi client api attempts to enforce
-		 * that, so this should be inexpensive - but it is necessary
-		 * to support shared nexthop-groups.
-		 */
-		nexthop_group_add_sorted(ng, nexthop);
-	}
-
-	/* Allocate temporary list of backup nexthops, if necessary */
-	if (api.backup_nexthop_num > 0) {
-		if (IS_ZEBRA_DEBUG_RECV)
-			zlog_debug("%s: adding %d backup nexthops",
-				   __func__, api.backup_nexthop_num);
-
-		bnhg = zebra_nhg_backup_alloc();
-		nexthop = NULL;
-		last_nh = NULL;
+		if (png) {
+			/* Add new nexthop to temporary list. This list is
+			 * canonicalized - sorted - so that it can be hashed later
+			 * in route processing. We expect that the sender has sent
+			 * the list sorted, and the zapi client api attempts to enforce
+			 * that, so this should be inexpensive - but it is necessary
+			 * to support shared nexthop-groups.
+			 */
+			nexthop_group_add_sorted(ng, nexthop);
+		}
+		if (bnhg) {
+			/* Note that the order of the backup nexthops is significant,
+			 * so we don't sort this list as we do the primary nexthops,
+			 * we just append.
+			 */
+			if (last_nh)
+				NEXTHOP_APPEND(last_nh, nexthop);
+			else
+				bnhg->nhe->nhg.nexthop = nexthop;
+
+			last_nh = nexthop;
+		}
 	}
 
-	/* Copy backup nexthops also, if present */
-	for (i = 0; i < api.backup_nexthop_num; i++) {
-		api_nh = &api.backup_nexthops[i];
+	return true;
+}
 
-		/* Convert zapi backup nexthop */
-		nexthop = nexthop_from_zapi(re, api_nh, &api);
-		if (!nexthop) {
-			flog_warn(
-				EC_ZEBRA_NEXTHOP_CREATION_FAILED,
-				"%s: Backup Nexthops Specified: %d but we failed to properly create one",
-				__func__, api.backup_nexthop_num);
-			nexthop_group_delete(&ng);
-			zebra_nhg_backup_free(&bnhg);
-			XFREE(MTYPE_RE, re);
-			return;
-		}
+static void zread_route_add(ZAPI_HANDLER_ARGS)
+{
+	struct stream *s;
+	struct zapi_route api;
+	afi_t afi;
+	struct prefix_ipv6 *src_p = NULL;
+	struct route_entry *re;
+	struct nexthop_group *ng = NULL;
+	struct nhg_backup_info *bnhg = NULL;
+	int ret;
+	vrf_id_t vrf_id;
+	struct nhg_hash_entry nhe;
 
-		/* Backup nexthops can't have backups; that's not valid. */
-		if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) {
-			if (IS_ZEBRA_DEBUG_RECV) {
-				nexthop2str(nexthop, nhbuf, sizeof(nhbuf));
-				zlog_debug("%s: backup nh %s with BACKUP flag!",
-					   __func__, nhbuf);
-			}
-			UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP);
-			nexthop->backup_num = 0;
-		}
+	s = msg;
+	if (zapi_route_decode(s, &api) < 0) {
+		if (IS_ZEBRA_DEBUG_RECV)
+			zlog_debug("%s: Unable to decode zapi_route sent",
+				   __func__);
+		return;
+	}
 
-		if (CHECK_FLAG(api.message, ZAPI_MESSAGE_SRTE)) {
-			SET_FLAG(nexthop->flags, NEXTHOP_FLAG_SRTE);
-			nexthop->srte_color = api_nh->srte_color;
-		}
+	vrf_id = zvrf_id(zvrf);
 
-		/* MPLS labels for BGP-LU or Segment Routing */
-		if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_LABEL)
-		    && api_nh->type != NEXTHOP_TYPE_IFINDEX
-		    && api_nh->type != NEXTHOP_TYPE_BLACKHOLE
-		    && api_nh->label_num > 0) {
+	if (IS_ZEBRA_DEBUG_RECV) {
+		char buf_prefix[PREFIX_STRLEN];
 
-			label_type = lsp_type_from_re_type(client->proto);
-			nexthop_add_labels(nexthop, label_type,
-					   api_nh->label_num,
-					   &api_nh->labels[0]);
-		}
+		prefix2str(&api.prefix, buf_prefix, sizeof(buf_prefix));
+		zlog_debug("%s: p=(%u:%u)%s, msg flags=0x%x, flags=0x%x",
+			   __func__, vrf_id, api.tableid, buf_prefix,
+			   (int)api.message, api.flags);
+	}
 
-		if (IS_ZEBRA_DEBUG_RECV) {
-			labelbuf[0] = '\0';
-			nhbuf[0] = '\0';
+	/* Allocate new route. */
+	re = XCALLOC(MTYPE_RE, sizeof(struct route_entry));
+	re->type = api.type;
+	re->instance = api.instance;
+	re->flags = api.flags;
+	re->uptime = monotime(NULL);
+	re->vrf_id = vrf_id;
 
-			nexthop2str(nexthop, nhbuf, sizeof(nhbuf));
+	if (api.tableid)
+		re->table = api.tableid;
+	else
+		re->table = zvrf->table_id;
 
-			if (nexthop->nh_label &&
-			    nexthop->nh_label->num_labels > 0) {
-				mpls_label2str(nexthop->nh_label->num_labels,
-					       nexthop->nh_label->label,
-					       labelbuf, sizeof(labelbuf),
-					       false);
-			}
+	if (!CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP)
+	    || api.nexthop_num == 0) {
+		flog_warn(EC_ZEBRA_RX_ROUTE_NO_NEXTHOPS,
+			  "%s: received a route without nexthops for prefix %pFX from client %s",
+			  __func__, &api.prefix,
+			  zebra_route_string(client->proto));
 
-			zlog_debug("%s: backup nh=%s, vrf_id=%d %s",
-				   __func__, nhbuf, api_nh->vrf_id, labelbuf);
-		}
+		XFREE(MTYPE_RE, re);
+		return;
+	}
 
-		/* Note that the order of the backup nexthops is significant,
-		 * so we don't sort this list as we do the primary nexthops,
-		 * we just append.
-		 */
-		if (last_nh)
-			NEXTHOP_APPEND(last_nh, nexthop);
-		else
-			bnhg->nhe->nhg.nexthop = nexthop;
+	/* Report misuse of the backup flag */
+	if (CHECK_FLAG(api.message, ZAPI_MESSAGE_BACKUP_NEXTHOPS) &&
+	    api.backup_nexthop_num == 0) {
+		if (IS_ZEBRA_DEBUG_RECV || IS_ZEBRA_DEBUG_EVENT)
+			zlog_debug("%s: client %s: BACKUP flag set but no backup nexthops, prefix %pFX",
+				__func__,
+				zebra_route_string(client->proto), &api.prefix);
+	}
 
-		last_nh = nexthop;
+	if (!zapi_read_nexthops(client, &api, re, &ng, NULL) ||
+	    !zapi_read_nexthops(client, &api, re, NULL, &bnhg)) {
+		XFREE(MTYPE_RE, re);
+		return;
 	}
 
 	if (CHECK_FLAG(api.message, ZAPI_MESSAGE_DISTANCE))