zebra: When processing route_entries ignore unusable routes

When zebra is processing routes to determine what to send
to the rib, suppose we have two routes (a) a route processed
earlier that none of it's nexthops were active and (b)
a route that has good nexthops but has a worse admin distance.

rib_process, would not relook at (a)'s nexthops because
the ROUTE_ENTRY_CHANGED flag was not true and it would
win when compared to (b) because it's admin distance
was better, leaving us with a state where we would
attempt and fail to install route (a) because it
was not valid.

Modify the code to consider the number of nexthops
we have as a determiner if we can use the route.

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

diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 4fa62151fc2a280f73d592d0b9def351ab1d0c75..5d4eeaafd61eacf46e1f75e2980dd53668b44b7d 100644 (file)
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -1075,46 +1075,56 @@ static void rib_process(struct route_node *rn)
                if (CHECK_FLAG(re->status, ROUTE_ENTRY_REMOVED))
                        continue;
 
-               /* Skip unreachable nexthop. */
-               /* This first call to nexthop_active_update is merely to
-                * determine if there's any change to nexthops associated
-                * with this RIB entry. Now, rib_process() can be invoked due
-                * to an external event such as link down or due to
-                * next-hop-tracking evaluation. In the latter case,
-                * a decision has already been made that the NHs have changed.
-                * So, no need to invoke a potentially expensive call again.
-                * Further, since the change might be in a recursive NH which
-                * is not caught in the nexthop_active_update() code. Thus, we
-                * might miss changes to recursive NHs.
+               /*
+                * If the route entry has changed, verify/resolve
+                * the nexthops associated with the entry.
+                *
+                * In any event if we have nexthops that are not active
+                * then we cannot use this particular route entry so
+                * skip it.
                 */
-               if (CHECK_FLAG(re->status, ROUTE_ENTRY_CHANGED)
-                   && !nexthop_active_update(rn, re)) {
-                       if (re->type == ZEBRA_ROUTE_TABLE) {
-                               /* XXX: HERE BE DRAGONS!!!!!
-                                * In all honesty, I have not yet figured out
-                                * what this part does or why the
-                                * ROUTE_ENTRY_CHANGED test above is correct
-                                * or why we need to delete a route here, and
-                                * also not whether this concerns both selected
-                                * and fib route, or only selected
-                                * or only fib
-                                *
-                                * This entry was denied by the 'ip protocol
-                                * table' route-map, we need to delete it */
-                               if (re != old_selected) {
-                                       if (IS_ZEBRA_DEBUG_RIB)
-                                               zlog_debug(
-                                                       "%s: %s(%u):%s: imported via import-table but denied by the ip protocol table route-map",
-                                                       __func__,
-                                                       VRF_LOGNAME(vrf),
-                                                       vrf_id, buf);
-                                       rib_unlink(rn, re);
-                               } else
-                                       SET_FLAG(re->status,
-                                                ROUTE_ENTRY_REMOVED);
-                       }
+               if (CHECK_FLAG(re->status, ROUTE_ENTRY_CHANGED)) {
+                       if (!nexthop_active_update(rn, re)) {
+                               if (re->type == ZEBRA_ROUTE_TABLE) {
+                                       /* XXX: HERE BE DRAGONS!!!!!
+                                        * In all honesty, I have not yet
+                                        * figured out what this part does or
+                                        * why the ROUTE_ENTRY_CHANGED test
+                                        * above is correct or why we need to
+                                        * delete a route here, and also not
+                                        * whether this concerns both selected
+                                        * and fib route, or only selected
+                                        * or only fib
+                                        *
+                                        * This entry was denied by the 'ip
+                                        * protocol
+                                        * table' route-map, we need to delete
+                                        * it */
+                                       if (re != old_selected) {
+                                               if (IS_ZEBRA_DEBUG_RIB)
+                                                       zlog_debug(
+                                                               "%s: %s(%u):%s: imported via import-table but denied by the ip protocol table route-map",
+                                                               __func__,
+                                                               VRF_LOGNAME(
+                                                                       vrf),
+                                                               vrf_id, buf);
+                                               rib_unlink(rn, re);
+                                       } else
+                                               SET_FLAG(re->status,
+                                                        ROUTE_ENTRY_REMOVED);
+                               }
 
-                       continue;
+                               continue;
+                       }
+               } else {
+                       /*
+                        * If the re has not changed and the nhg we have is
+                        * not usable, then we cannot use this route entry
+                        * for consideration, as that the route will just
+                        * not install if it is selected.
+                        */
+                       if (!nexthop_group_active_nexthop_num(&re->nhe->nhg))
+                               continue;
                }
 
                /* Infinite distance. */