zebra: Allow nhg's to be reused when multiple interfaces are going amuck

donaldsharp · donaldsharp · commit 46044a45a7a9 · 2025-05-09T18:14:33.000-04:00
Currently if there are multiple interfaces going down it is possible
to have a ships in the night situation with trying to reuse a nhg.

Imagine that you have a route w/ 4 way ecmp
  nexthop A interface a
  nexthop B interface b
  nexthop C interface c
  nexthop D interface d

Suppose interface a goes down, zebra receives this data
marks singleton nexthop A down and then recurses up the
tree to the 4 way ecmp and sets nexthop A as inactive.
Zebra then will notify the upper level protocol.

The upper level protocol will refigure the route and send
it down with 3 way ecmp.  At the same time if interface
b goes down and zebra handles that interface down event
before the new route installation, then when
zebra_nhg_rib_compare_old_nhe is called it will not
match the old and new ones up as that the old will be:

  nexthop A  &lt;inactive&gt;
  nexthop B  &lt;inactive&gt;
  nexthop C
  nexthop D

New will be:

   nexthop B  &lt;inactive&gt;
   nexthop C
   nexthop D

Currently zebra_nhg_nexthop_compare on the old skips all
the inactive but it never skips the nexthops at are inactive
on the new.

Modify the code to allow the new nhop to be skipped if it
is the same nexthop being looked at as the old and it
is not active as well.  This allows zebra to choose
the same nhg in the above case to continue working.

Signed-off-by: Donald Sharp &lt;sharpd@nvidia.com&gt;
diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
@@ -3041,13 +3041,27 @@ static bool zebra_nhg_nexthop_compare(const struct nexthop *nhop,
 
 	while (nhop && old_nhop) {
 		if (IS_ZEBRA_DEBUG_NHG_DETAIL)
-			zlog_debug("%s: %pRN Comparing %pNHvv(%u) to old: %pNHvv(%u)",
-				   __func__, rn, nhop, nhop->flags, old_nhop,
-				   old_nhop->flags);
+			zlog_debug("%s: %pRN Comparing %pNHvv(%u) ACTIVE: %d to old: %pNHvv(%u) ACTIVE: %d nexthop same: %d",
+				   __func__, rn, nhop, nhop->flags,
+				   CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_ACTIVE), old_nhop,
+				   old_nhop->flags, CHECK_FLAG(old_nhop->flags, NEXTHOP_FLAG_ACTIVE),
+				   nexthop_same_no_ifindex(nhop, old_nhop));
 		if (!CHECK_FLAG(old_nhop->flags, NEXTHOP_FLAG_ACTIVE)) {
 			if (IS_ZEBRA_DEBUG_NHG_DETAIL)
 				zlog_debug("%s: %pRN Old is not active going to the next one",
 					   __func__, rn);
+
+			/*
+			 * If the new nexthop is not active and the old nexthop is also not active,
+			 * then we know that we can skip both the old and new nexthops.
+			 */
+			if (!CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_ACTIVE) &&
+			    nexthop_same_no_ifindex(nhop, old_nhop)) {
+				if (IS_ZEBRA_DEBUG_NHG_DETAIL)
+					zlog_debug("%s: %pRN new is not active going to the next one",
+						   __func__, rn);
+				nhop = nhop->next;
+			}
 			old_nhop = old_nhop->next;
 			continue;
 		}
