zebra: add handy res2str utility

Add a 2str utility for dplane result codes; use it in
a debug or two.

Signed-off-by: Mark Stapp <mjs@voltanet.io>

zebra/rt_netlink.c

@@ -1965,6 +1965,7 @@ static int netlink_route_multipath_ctx(int cmd, dplane_ctx_h ctx)
 						setsrc = 1;
 					}
 				}
+				continue;
 			}
 
 			if ((cmd == RTM_NEWROUTE
@@ -2205,6 +2206,7 @@ enum zebra_dplane_result kernel_route_update(dplane_ctx_h ctx)
 {
 	int cmd, ret;
 	const struct prefix *p = dplane_ctx_get_dest(ctx);
+	struct nexthop *nexthop;
 
 	if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE) {
 		cmd = RTM_DELROUTE;
@@ -2237,6 +2239,25 @@ enum zebra_dplane_result kernel_route_update(dplane_ctx_h ctx)
 	}
 
 	ret = netlink_route_multipath_ctx(cmd, ctx);
+	if ((cmd == RTM_NEWROUTE) && (ret == 0)) {
+		/* Update installed nexthops to signal which have been
+		 * installed.
+		 */
+		for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) {
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE))
+				continue;
+
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) {
+				SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
+
+				/* If we're only allowed a single nh, don't
+				 * continue.
+				 */
+				if (multipath_num == 1)
+					break;
+			}
+		}
+	}
 
 	return (ret == 0 ?
 		ZEBRA_DPLANE_REQUEST_SUCCESS : ZEBRA_DPLANE_REQUEST_FAILURE);

zebra/rt_socket.c

@@ -182,7 +182,7 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p,
 					       : NULL,
 					  smplsp, ifindex, bh_type, metric);
 
-			if (IS_ZEBRA_DEBUG_RIB) {
+			if (IS_ZEBRA_DEBUG_KERNEL) {
 				if (!gate) {
 					zlog_debug(
 						"%s: %s: attention! gate not found for re",
@@ -197,10 +197,15 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p,
 			 * did its work. */
 			case ZEBRA_ERR_NOERROR:
 				nexthop_num++;
-				if (IS_ZEBRA_DEBUG_RIB)
+				if (IS_ZEBRA_DEBUG_KERNEL)
 					zlog_debug(
 						"%s: %s: successfully did NH %s",
 						__func__, prefix_buf, gate_buf);
+
+				if (cmd == RTM_ADD)
+					SET_FLAG(nexthop->flags,
+						 NEXTHOP_FLAG_FIB);
+
 				break;
 
 			/* The only valid case for this error is kernel's
@@ -216,14 +221,8 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p,
 						"%s: rtm_write() returned %d for command %d",
 						__func__, error, cmd);
 				continue;
-				break;
 
-			/* Given that our NEXTHOP_FLAG_FIB matches real kernel
+			/* Note any unexpected status returns */
-			 * FIB, it isn't
-			 * normal to get any other messages in ANY case.
-			 */
-			case ZEBRA_ERR_RTNOEXIST:
-			case ZEBRA_ERR_RTUNREACH:
 			default:
 				flog_err(
 					EC_LIB_SYSTEM_CALL,
@@ -236,7 +235,7 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p,
 				break;
 			}
 		} /* if (cmd and flags make sense) */
-		else if (IS_ZEBRA_DEBUG_RIB)
+		else if (IS_ZEBRA_DEBUG_KERNEL)
 			zlog_debug("%s: odd command %s for flags %d", __func__,
 				   lookup_msg(rtm_type_str, cmd, NULL),
 				   nexthop->flags);
@@ -367,7 +366,10 @@ static int kernel_rtm_ipv6(int cmd, const struct prefix *p,
 				  (union sockunion *)mask,
 				  gate ? (union sockunion *)&sin_gate : NULL,
 				  smplsp, ifindex, bh_type, metric);
-		(void)error;
+
+		/* Update installed nexthop info on success */
+		if ((cmd == RTM_ADD) && (error == ZEBRA_ERR_NOERROR))
+			SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
 
 		nexthop_num++;
 	}
@@ -408,33 +410,34 @@ enum zebra_dplane_result kernel_route_update(dplane_ctx_h ctx)
 		goto done;
 	}
 
-	if (zserv_privs.change(ZPRIVS_RAISE))
+	frr_elevate_privs(ZPRIVS_RAISE) {
-		zlog_err("Can't raise privileges");
 
-	if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE)
+		if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE)
-		kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx),
+			kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx),
-			   dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx));
+				   dplane_ctx_get_ng(ctx),
-	else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_INSTALL)
+				   dplane_ctx_get_metric(ctx));
-		kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx),
+		else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_INSTALL)
-			   dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx));
+			kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx),
-	else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_UPDATE) {
+				   dplane_ctx_get_ng(ctx),
-		/*
+				   dplane_ctx_get_metric(ctx));
-		 * Must do delete and add separately - no update available
+		else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_UPDATE) {
-		 */
+			/* Must do delete and add separately -
-		kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx),
+			 * no update available
-			   dplane_ctx_get_old_ng(ctx),
+			 */
-			   dplane_ctx_get_old_metric(ctx));
+			kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx),
+				   dplane_ctx_get_old_ng(ctx),
+				   dplane_ctx_get_old_metric(ctx));
 
-		kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx),
+			kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx),
-			   dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx));
+				   dplane_ctx_get_ng(ctx),
-	} else {
+				   dplane_ctx_get_metric(ctx));
-		zlog_err("Invalid routing socket update op %u",
+		} else {
-			 dplane_ctx_get_op(ctx));
+			zlog_err("Invalid routing socket update op %s (%u)",
-		res = ZEBRA_DPLANE_REQUEST_FAILURE;
+				 dplane_op2str(dplane_ctx_get_op(ctx)),
-	}
+				 dplane_ctx_get_op(ctx));
-
+			res = ZEBRA_DPLANE_REQUEST_FAILURE;
-	if (zserv_privs.change(ZPRIVS_LOWER))
+		}
-		zlog_err("Can't lower privileges");
+	} /* Elevated privs */
 
 done:
 

zebra/zebra_dplane.c

@@ -308,6 +308,25 @@ const char *dplane_op2str(enum dplane_op_e op)
 	return ret;
 }
 
+const char *dplane_res2str(enum zebra_dplane_result res)
+{
+	const char *ret = "<Unknown>";
+
+	switch (res) {
+	case ZEBRA_DPLANE_REQUEST_FAILURE:
+		ret = "FAILURE";
+		break;
+	case ZEBRA_DPLANE_REQUEST_QUEUED:
+		ret = "QUEUED";
+		break;
+	case ZEBRA_DPLANE_REQUEST_SUCCESS:
+		ret = "SUCCESS";
+		break;
+	};
+
+	return ret;
+}
+
 const struct prefix *dplane_ctx_get_dest(const dplane_ctx_h ctx)
 {
 	DPLANE_CTX_VALID(ctx);
@@ -500,6 +519,7 @@ static int dplane_ctx_route_init(dplane_ctx_h ctx,
 	const struct prefix *p, *src_p;
 	struct zebra_ns *zns;
 	struct zebra_vrf *zvrf;
+	struct nexthop *nexthop;
 
 	if (!ctx || !rn || !re)
 		goto done;
@@ -558,6 +578,11 @@ static int dplane_ctx_route_init(dplane_ctx_h ctx,
 
 	/* TODO -- maybe use array of nexthops to avoid allocs? */
 
+	/* Ensure that the dplane's nexthop flag is clear. */
+	for (ALL_NEXTHOPS(ctx->zd_ng, nexthop)) {
+		UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
+	}
+
 	/* Trying out the sequence number idea, so we can try to detect
 	 * when a result is stale.
 	 */
@@ -654,7 +679,9 @@ dplane_route_update_internal(struct route_node *rn,
 			ctx->zd_old_metric = old_re->metric;
 
 #ifndef HAVE_NETLINK
-			/* Capture previous re's nexthops too for bsd, sigh */
+			/* For bsd, capture previous re's nexthops too, sigh.
+			 * We'll need these to do per-nexthop deletes.
+			 */
 			copy_nexthops(&(ctx->zd_old_ng.nexthop),
 				      old_re->ng.nexthop, NULL);
 #endif	/* !HAVE_NETLINK */

zebra/zebra_dplane.h

@@ -134,6 +134,7 @@ void dplane_ctx_dequeue(struct dplane_ctx_q_s *q, dplane_ctx_h *ctxp);
  * Accessors for information from the context object
  */
 enum zebra_dplane_result dplane_ctx_get_status(const dplane_ctx_h ctx);
+const char *dplane_res2str(enum zebra_dplane_result res);
 
 enum dplane_op_e dplane_ctx_get_op(const dplane_ctx_h ctx);
 const char *dplane_op2str(enum dplane_op_e op);

zebra/zebra_rib.c

@@ -1951,7 +1951,7 @@ static void rib_process_after(dplane_ctx_h ctx)
 	struct route_node *rn = NULL;
 	struct route_entry *re = NULL, *old_re = NULL, *rib;
 	bool is_update = false;
-	struct nexthop *nexthop;
+	struct nexthop *nexthop, *ctx_nexthop;
 	char dest_str[PREFIX_STRLEN] = "";
 	enum dplane_op_e op;
 	enum zebra_dplane_result status;
@@ -2000,9 +2000,9 @@ static void rib_process_after(dplane_ctx_h ctx)
 	status = dplane_ctx_get_status(ctx);
 
 	if (IS_ZEBRA_DEBUG_DPLANE_DETAIL) {
-		zlog_debug("%u:%s Processing dplane ctx %p, op %s result %d",
+		zlog_debug("%u:%s Processing dplane ctx %p, op %s result %s",
 			   dplane_ctx_get_vrf(ctx), dest_str, ctx,
-			   dplane_op2str(op), status);
+			   dplane_op2str(op), dplane_res2str(status));
 	}
 
 	if (op == DPLANE_OP_ROUTE_DELETE) {
@@ -2090,19 +2090,34 @@ static void rib_process_after(dplane_ctx_h ctx)
 	}
 
 	if (status == ZEBRA_DPLANE_REQUEST_SUCCESS) {
-		/* Set nexthop FIB flags */
+		/* Update zebra nexthop FIB flag for each
-		for (ALL_NEXTHOPS(re->ng, nexthop)) {
+		 * nexthop that was installed.
+		 */
+		for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), ctx_nexthop)) {
+
+			for (ALL_NEXTHOPS(re->ng, nexthop)) {
+				if (nexthop_same(ctx_nexthop, nexthop))
+					break;
+			}
+
+			if (nexthop == NULL)
+				continue;
+
 			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE))
 				continue;
 
-			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE))
+			if (CHECK_FLAG(ctx_nexthop->flags,
+				       NEXTHOP_FLAG_FIB))
 				SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
 			else
 				UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
 		}
 
-		if (zvrf)
+		if (zvrf) {
 			zvrf->installs++;
+			/* Set flag for nexthop tracking processing */
+			zvrf->flags |= ZEBRA_VRF_RIB_SCHEDULED;
+		}
 
 		/* Redistribute */
 		/* TODO -- still calling the redist api using the route_entries,
@@ -2179,9 +2194,9 @@ static unsigned int process_subq(struct list *subq, uint8_t qindex)
 }
 
 /*
- * All meta queues have been processed. Trigger next-hop evaluation.
+ * Perform next-hop tracking processing after RIB updates.
  */
-static void meta_queue_process_complete(struct work_queue *dummy)
+static void do_nht_processing(void)
 {
 	struct vrf *vrf;
 	struct zebra_vrf *zvrf;
@@ -2196,6 +2211,10 @@ static void meta_queue_process_complete(struct work_queue *dummy)
 		if (zvrf == NULL || !(zvrf->flags & ZEBRA_VRF_RIB_SCHEDULED))
 			continue;
 
+		if (IS_ZEBRA_DEBUG_RIB_DETAILED || IS_ZEBRA_DEBUG_NHT)
+			zlog_debug("NHT processing check for zvrf %s",
+				   zvrf_name(zvrf));
+
 		zvrf->flags &= ~ZEBRA_VRF_RIB_SCHEDULED;
 		zebra_evaluate_rnh(zvrf, AF_INET, 0, RNH_NEXTHOP_TYPE, NULL);
 		zebra_evaluate_rnh(zvrf, AF_INET, 0, RNH_IMPORT_CHECK_TYPE,
@@ -2217,6 +2236,14 @@ static void meta_queue_process_complete(struct work_queue *dummy)
 	}
 }
 
+/*
+ * All meta queues have been processed. Trigger next-hop evaluation.
+ */
+static void meta_queue_process_complete(struct work_queue *dummy)
+{
+	do_nht_processing();
+}
+
 /* Dispatch the meta queue by picking, processing and unlocking the next RN from
  * a non-empty sub-queue with lowest priority. wq is equal to zebra->ribq and
  * data
@@ -3300,6 +3327,9 @@ static int rib_process_dplane_results(struct thread *thread)
 
 	} while (1);
 
+	/* Check for nexthop tracking processing after finishing with results */
+	do_nht_processing();
+
 	return 0;
 }
 

zebra/zebra_rnh.c

@@ -530,6 +530,7 @@ zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, int family,
 	struct route_table *route_table;
 	struct route_node *rn;
 	struct route_entry *re;
+	struct nexthop *nexthop;
 
 	*prn = NULL;
 
@@ -562,12 +563,26 @@ zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, int family,
 			if (!CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED))
 				continue;
 
+			/* Just being SELECTED isn't quite enough - must
+			 * have an installed nexthop to be useful.
+			 */
+			for (nexthop = re->ng.nexthop; nexthop;
+			     nexthop = nexthop->next)
+				if ((CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB)
+				     || CHECK_FLAG(nexthop->flags,
+						   NEXTHOP_FLAG_RECURSIVE))
+				    && CHECK_FLAG(nexthop->flags,
+						  NEXTHOP_FLAG_ACTIVE))
+					break;
+
+			if (nexthop == NULL)
+				continue;
+
 			if (CHECK_FLAG(rnh->flags, ZEBRA_NHT_CONNECTED)) {
 				if ((re->type == ZEBRA_ROUTE_CONNECT)
 				    || (re->type == ZEBRA_ROUTE_STATIC))
 					break;
 				if (re->type == ZEBRA_ROUTE_NHRP) {
-					struct nexthop *nexthop;
 
 					for (nexthop = re->ng.nexthop; nexthop;
 					     nexthop = nexthop->next)