bgpd, lib, pimd: Make nexthop_update decoding common

Create a zapi_nexthop_update_decode function that both
pim and bgp use to decode the message from zebra.

There probably could be further optimizations but I opted
to keep the code as similiar as is possible between the
originals because they both make some assumptions about
code flow that I do not fully understand yet.

The real goal here is that I want to create a new
user of the nexthop tracking code from a higher level
daemon and I see no need to re-implement this damn
code again for a 3rd time.

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

bgpd/bgp_nht.c

@@ -320,18 +320,15 @@ void bgp_delete_connected_nexthop(afi_t afi, struct peer *peer)
 
 void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 {
-	struct stream *s;
 	struct bgp_node *rn = NULL;
 	struct bgp_nexthop_cache *bnc;
 	struct nexthop *nexthop;
 	struct nexthop *oldnh;
 	struct nexthop *nhlist_head = NULL;
 	struct nexthop *nhlist_tail = NULL;
-	uint32_t metric;
-	u_char nexthop_num;
-	struct prefix p;
 	int i;
 	struct bgp *bgp;
+	struct zapi_route nhr;
 
 	bgp = bgp_lookup_by_vrf_id(vrf_id);
 	if (!bgp) {
@@ -341,33 +338,21 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 		return;
 	}
 
-	s = zclient->ibuf;
+	zapi_nexthop_update_decode(zclient->ibuf, &nhr);
-
-	memset(&p, 0, sizeof(struct prefix));
-	p.family = stream_getw(s);
-	p.prefixlen = stream_getc(s);
-	switch (p.family) {
-	case AF_INET:
-		p.u.prefix4.s_addr = stream_get_ipv4(s);
-		break;
-	case AF_INET6:
-		stream_get(&p.u.prefix6, s, 16);
-		break;
-	default:
-		break;
-	}
 
 	if (command == ZEBRA_NEXTHOP_UPDATE)
 		rn = bgp_node_lookup(
-			bgp->nexthop_cache_table[family2afi(p.family)], &p);
+			bgp->nexthop_cache_table[family2afi(nhr.prefix.family)],
+			&nhr.prefix);
 	else if (command == ZEBRA_IMPORT_CHECK_UPDATE)
 		rn = bgp_node_lookup(
-			bgp->import_check_table[family2afi(p.family)], &p);
+			bgp->import_check_table[family2afi(nhr.prefix.family)],
+			&nhr.prefix);
 
 	if (!rn || !rn->info) {
 		if (BGP_DEBUG(nht, NHT)) {
 			char buf[PREFIX2STR_BUFFER];
-			prefix2str(&p, buf, sizeof(buf));
+			prefix2str(&nhr.prefix, buf, sizeof(buf));
 			zlog_debug("parse nexthop update(%s): rn not found",
 				   buf);
 		}
@@ -380,62 +365,34 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 	bgp_unlock_node(rn);
 	bnc->last_update = bgp_clock();
 	bnc->change_flags = 0;
-	stream_getc(s); // Distance but not currently used
-	metric = stream_getl(s);
-	nexthop_num = stream_getc(s);
 
 	/* debug print the input */
 	if (BGP_DEBUG(nht, NHT)) {
 		char buf[PREFIX2STR_BUFFER];
-		prefix2str(&p, buf, sizeof(buf));
+		prefix2str(&nhr.prefix, buf, sizeof(buf));
 		zlog_debug(
 			"%u: Rcvd NH update %s - metric %d/%d #nhops %d/%d flags 0x%x",
-			vrf_id, buf, metric, bnc->metric, nexthop_num,
+			vrf_id, buf, nhr.metric, bnc->metric, nhr.nexthop_num,
 			bnc->nexthop_num, bnc->flags);
 	}
 
-	if (metric != bnc->metric)
+	if (nhr.metric != bnc->metric)
 		bnc->change_flags |= BGP_NEXTHOP_METRIC_CHANGED;
 
-	if (nexthop_num != bnc->nexthop_num)
+	if (nhr.nexthop_num != bnc->nexthop_num)
 		bnc->change_flags |= BGP_NEXTHOP_CHANGED;
 
-	if (nexthop_num) {
+	if (nhr.nexthop_num) {
 		/* notify bgp fsm if nbr ip goes from invalid->valid */
 		if (!bnc->nexthop_num)
 			UNSET_FLAG(bnc->flags, BGP_NEXTHOP_PEER_NOTIFIED);
 
 		bnc->flags |= BGP_NEXTHOP_VALID;
-		bnc->metric = metric;
+		bnc->metric = nhr.metric;
-		bnc->nexthop_num = nexthop_num;
+		bnc->nexthop_num = nhr.nexthop_num;
 
-		for (i = 0; i < nexthop_num; i++) {
+		for (i = 0; i < nhr.nexthop_num; i++) {
-			nexthop = nexthop_new();
+			nexthop = nexthop_from_zapi_nexthop(&nhr.nexthops[i]);
-			nexthop->type = stream_getc(s);
-			switch (nexthop->type) {
-			case NEXTHOP_TYPE_IPV4:
-				nexthop->gate.ipv4.s_addr = stream_get_ipv4(s);
-				nexthop->ifindex = stream_getl(s);
-				break;
-			case NEXTHOP_TYPE_IFINDEX:
-				nexthop->ifindex = stream_getl(s);
-				break;
-			case NEXTHOP_TYPE_IPV4_IFINDEX:
-				nexthop->gate.ipv4.s_addr = stream_get_ipv4(s);
-				nexthop->ifindex = stream_getl(s);
-				break;
-			case NEXTHOP_TYPE_IPV6:
-				stream_get(&nexthop->gate.ipv6, s, 16);
-				nexthop->ifindex = stream_getl(s);
-				break;
-			case NEXTHOP_TYPE_IPV6_IFINDEX:
-				stream_get(&nexthop->gate.ipv6, s, 16);
-				nexthop->ifindex = stream_getl(s);
-				break;
-			default:
-				/* do nothing */
-				break;
-			}
 
 			if (BGP_DEBUG(nht, NHT)) {
 				char buf[NEXTHOP_STRLEN];
@@ -470,7 +427,7 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id)
 		bnc->nexthop = nhlist_head;
 	} else {
 		bnc->flags &= ~BGP_NEXTHOP_VALID;
-		bnc->nexthop_num = nexthop_num;
+		bnc->nexthop_num = nhr.nexthop_num;
 
 		/* notify bgp fsm if nbr ip goes from valid->invalid */
 		UNSET_FLAG(bnc->flags, BGP_NEXTHOP_PEER_NOTIFIED);

lib/zclient.c

@@ -1212,6 +1212,72 @@ stream_failure:
 	return false;
 }
 
+struct nexthop *nexthop_from_zapi_nexthop(struct zapi_nexthop *znh)
+{
+	struct nexthop *n = nexthop_new();
+
+	n->type = znh->type;
+	n->ifindex = znh->ifindex;
+	n->gate = znh->gate;
+
+	/*
+	 * This function does not currently handle labels
+	 */
+
+	return n;
+}
+
+bool zapi_nexthop_update_decode(struct stream *s, struct zapi_route *nhr)
+{
+	uint32_t i;
+
+	memset(nhr, 0, sizeof(*nhr));
+
+	STREAM_GETW(s, nhr->prefix.family);
+	STREAM_GETC(s, nhr->prefix.prefixlen);
+	switch(nhr->prefix.family) {
+	case AF_INET:
+		STREAM_GET(&nhr->prefix.u.prefix4.s_addr, s, IPV4_MAX_BYTELEN);
+		break;
+	case AF_INET6:
+		STREAM_GET(&nhr->prefix.u.prefix6, s, IPV6_MAX_BYTELEN);
+		break;
+	default:
+		break;
+	}
+
+	STREAM_GETC(s, nhr->distance);
+	STREAM_GETL(s, nhr->metric);
+	STREAM_GETC(s, nhr->nexthop_num);
+
+	for (i = 0; i < nhr->nexthop_num ; i++) {
+		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;
+		}
+	}
+
+	return true;
+stream_failure:
+	return false;
+}
+
 /*
  * send a ZEBRA_REDISTRIBUTE_ADD or ZEBRA_REDISTRIBUTE_DELETE
  * for the route type (ZEBRA_ROUTE_KERNEL etc.). The zebra server will

lib/zclient.h

@@ -491,6 +491,9 @@ extern int zapi_route_encode(u_char, 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,
 			      enum zapi_route_notify_owner *note);
+extern struct nexthop *nexthop_from_zapi_nexthop(struct zapi_nexthop *znh);
+extern bool zapi_nexthop_update_decode(struct stream *s,
+				       struct zapi_route *nhr);
 
 static inline void zapi_route_set_blackhole(struct zapi_route *api,
 					    enum blackhole_type bh_type)

pimd/pim_nht.c

@@ -608,13 +608,9 @@ int pim_ecmp_nexthop_search(struct pim_instance *pim,
 int pim_parse_nexthop_update(int command, struct zclient *zclient,
 			     zebra_size_t length, vrf_id_t vrf_id)
 {
-	struct stream *s;
-	struct prefix p;
 	struct nexthop *nexthop;
 	struct nexthop *nhlist_head = NULL;
 	struct nexthop *nhlist_tail = NULL;
-	uint32_t metric, distance;
-	u_char nexthop_num = 0;
 	int i;
 	struct pim_rpf rpf;
 	struct pim_nexthop_cache *pnc = NULL;
@@ -623,30 +619,16 @@ int pim_parse_nexthop_update(int command, struct zclient *zclient,
 	struct interface *ifp1 = NULL;
 	struct vrf *vrf = vrf_lookup_by_id(vrf_id);
 	struct pim_instance *pim;
+	struct zapi_route nhr;
 
 	if (!vrf)
 		return 0;
 	pim = vrf->info;
 
-	s = zclient->ibuf;
+	zapi_nexthop_update_decode(zclient->ibuf, &nhr);
-	memset(&p, 0, sizeof(struct prefix));
-	p.family = stream_getw(s);
-	p.prefixlen = stream_getc(s);
-	switch (p.family) {
-	case AF_INET:
-		p.u.prefix4.s_addr = stream_get_ipv4(s);
-		break;
-	case AF_INET6:
-		stream_get(&p.u.prefix6, s, 16);
-		break;
-	default:
-		break;
-	}
 
 	if (command == ZEBRA_NEXTHOP_UPDATE) {
-		rpf.rpf_addr.family = p.family;
+		prefix_copy(&rpf.rpf_addr, &nhr.prefix);
-		rpf.rpf_addr.prefixlen = p.prefixlen;
-		rpf.rpf_addr.u.prefix4.s_addr = p.u.prefix4.s_addr;
 		pnc = pim_nexthop_cache_find(pim, &rpf);
 		if (!pnc) {
 			if (PIM_DEBUG_PIM_NHT) {
@@ -666,34 +648,20 @@ int pim_parse_nexthop_update(int command, struct zclient *zclient,
 	}
 
 	pnc->last_update = pim_time_monotonic_usec();
-	distance = stream_getc(s);
-	metric = stream_getl(s);
-	nexthop_num = stream_getc(s);
 
-	if (nexthop_num) {
+	if (nhr.nexthop_num) {
 		pnc->nexthop_num = 0; // Only increment for pim enabled rpf.
 
-		for (i = 0; i < nexthop_num; i++) {
+		for (i = 0; i < nhr.nexthop_num; i++) {
-			nexthop = nexthop_new();
+			nexthop = nexthop_from_zapi_nexthop(&nhr.nexthops[i]);
-			nexthop->type = stream_getc(s);
 			switch (nexthop->type) {
 			case NEXTHOP_TYPE_IPV4:
-				nexthop->gate.ipv4.s_addr = stream_get_ipv4(s);
-				nexthop->ifindex = stream_getl(s);
-				break;
 			case NEXTHOP_TYPE_IFINDEX:
-				nexthop->ifindex = stream_getl(s);
-				break;
 			case NEXTHOP_TYPE_IPV4_IFINDEX:
-				nexthop->gate.ipv4.s_addr = stream_get_ipv4(s);
-				nexthop->ifindex = stream_getl(s);
-				break;
 			case NEXTHOP_TYPE_IPV6:
-				stream_get(&nexthop->gate.ipv6, s, 16);
+			case NEXTHOP_TYPE_BLACKHOLE:
 				break;
 			case NEXTHOP_TYPE_IPV6_IFINDEX:
-				stream_get(&nexthop->gate.ipv6, s, 16);
-				nexthop->ifindex = stream_getl(s);
 				ifp1 = if_lookup_by_index(nexthop->ifindex,
 							  pim->vrf_id);
 				nbr = pim_neighbor_find_if(ifp1);
@@ -707,9 +675,6 @@ int pim_parse_nexthop_update(int command, struct zclient *zclient,
 						PIM_NET_INADDR_ANY;
 				}
 
-				break;
-			default:
-				/* do nothing */
 				break;
 			}
 
@@ -731,19 +696,21 @@ int pim_parse_nexthop_update(int command, struct zclient *zclient,
 
 			if (PIM_DEBUG_PIM_NHT) {
 				char p_str[PREFIX2STR_BUFFER];
-				prefix2str(&p, p_str, sizeof(p_str));
+
+				prefix2str(&nhr.prefix, p_str, sizeof(p_str));
 				zlog_debug(
 					"%s: NHT addr %s(%s) %d-nhop via %s(%s) type %d distance:%u metric:%u ",
 					__PRETTY_FUNCTION__, p_str,
 					pim->vrf->name, i + 1,
 					inet_ntoa(nexthop->gate.ipv4),
-					ifp->name, nexthop->type, distance,
+					ifp->name, nexthop->type, nhr.distance,
-					metric);
+					nhr.metric);
 			}
 
 			if (!ifp->info) {
 				if (PIM_DEBUG_PIM_NHT) {
 					char buf[NEXTHOP_STRLEN];
+
 					zlog_debug(
 						"%s: multicast not enabled on input interface %s(%s) (ifindex=%d, addr %s)",
 						__PRETTY_FUNCTION__, ifp->name,
@@ -771,23 +738,24 @@ int pim_parse_nexthop_update(int command, struct zclient *zclient,
 		pnc->nexthop = nhlist_head;
 		if (pnc->nexthop_num) {
 			pnc->flags |= PIM_NEXTHOP_VALID;
-			pnc->distance = distance;
+			pnc->distance = nhr.distance;
-			pnc->metric = metric;
+			pnc->metric = nhr.metric;
 		}
 	} else {
 		pnc->flags &= ~PIM_NEXTHOP_VALID;
-		pnc->nexthop_num = nexthop_num;
+		pnc->nexthop_num = nhr.nexthop_num;
 		nexthops_free(pnc->nexthop);
 		pnc->nexthop = NULL;
 	}
 
 	if (PIM_DEBUG_PIM_NHT) {
 		char buf[PREFIX2STR_BUFFER];
-		prefix2str(&p, buf, sizeof(buf));
+		prefix2str(&nhr.prefix, buf, sizeof(buf));
 		zlog_debug(
 			"%s: NHT Update for %s(%s) num_nh %d num_pim_nh %d vrf:%u up %ld rp %d",
-			__PRETTY_FUNCTION__, buf, pim->vrf->name, nexthop_num,
+			__PRETTY_FUNCTION__, buf, pim->vrf->name,
-			pnc->nexthop_num, vrf_id, pnc->upstream_hash->count,
+			nhr.nexthop_num, pnc->nexthop_num, vrf_id,
+			pnc->upstream_hash->count,
 			listcount(pnc->rp_list));
 	}