zebra: properly set the NEXTHOP_GROUP_VALID flag

Properly set the NEXTHOP_GROUP_VALID flag and use it
as a conditional for installation decisions for individual
nexthop and groups containing it.

We set the NEXTHOP_GROUP_VALID flag it is:

1) A fully resolved active nexthop
or
2) Its a group that contains at least one VALID NHE

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>

1 files changed, 45 insertions, 2 deletions

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index 6956d4d34..c2582b93e 100644
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -1737,6 +1737,36 @@ static unsigned nexthop_active_check(struct route_node *rn,
 	return CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE);
 }
 
+/* Helper function called after resolution to walk nhg rb trees
+ * and toggle the NEXTHOP_GROUP_VALID flag if the nexthop
+ * is active on singleton NHEs.
+ */
+static bool zebra_nhg_set_valid_if_active(struct nhg_hash_entry *nhe)
+{
+	struct nhg_connected *rb_node_dep = NULL;
+	bool valid = false;
+
+	if (!zebra_nhg_depends_is_empty(nhe)) {
+		/* Is at least one depend valid? */
+		frr_each(nhg_connected_tree, &nhe->nhg_depends, rb_node_dep) {
+			if (zebra_nhg_set_valid_if_active(rb_node_dep->nhe))
+				valid = true;
+		}
+
+		goto done;
+	}
+
+	/* should be fully resolved singleton at this point */
+	if (CHECK_FLAG(nhe->nhg.nexthop->flags, NEXTHOP_FLAG_ACTIVE))
+		valid = true;
+
+done:
+	if (valid)
+		SET_FLAG(nhe->flags, NEXTHOP_GROUP_VALID);
+
+	return valid;
+}
+
 /*
  * Iterate over all nexthops of the given RIB entry and refresh their
  * ACTIVE flag.  If any nexthop is found to toggle the ACTIVE flag,
@@ -1811,8 +1841,11 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re)
 		route_entry_update_nhe(re, new_nhe);
 	}
 
+	/* Walk the NHE depends tree and toggle NEXTHOP_GROUP_VALID
+	 * flag where appropriate.
+	 */
 	if (curr_active)
-		SET_FLAG(re->nhe->flags, NEXTHOP_GROUP_VALID);
+		zebra_nhg_set_valid_if_active(re->nhe);
 
 	/*
 	 * Do not need these nexthops anymore since they
@@ -1871,6 +1904,15 @@ static uint8_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp,
 			/* This is a group within a group */
 			i = zebra_nhg_nhe2grp_internal(grp, i, depend, max_num);
 		} else {
+			if (!CHECK_FLAG(depend->flags, NEXTHOP_GROUP_VALID)) {
+				if (IS_ZEBRA_DEBUG_RIB_DETAILED
+				    || IS_ZEBRA_DEBUG_NHG)
+					zlog_debug(
+						"%s: Nexthop ID (%u) not valid, not appending to dataplane install group",
+						__func__, depend->id);
+				continue;
+			}
+
 			/* If the nexthop not installed/queued for install don't
 			 * put in the ID array.
 			 */
@@ -1930,7 +1972,8 @@ void zebra_nhg_install_kernel(struct nhg_hash_entry *nhe)
 		zebra_nhg_install_kernel(rb_node_dep->nhe);
 	}
 
-	if (!CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_INSTALLED)
+	if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_VALID)
+	    && !CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_INSTALLED)
 	    && !CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_QUEUED)) {
 		/* Change its type to us since we are installing it */
 		nhe->type = ZEBRA_ROUTE_NHG;