zebra: Allow multiple connected routes to be choosen for kernel routes

This bug should only really affect kernel routes.  To reproduce:

a) Have multiple connected routes that point to the same prefix
swp8  up      default         169.254.0.250/30
swp9  up      default         169.254.0.250/30

b) Have a kernel route that uses one of those connected routes
7.6.2.8 via 169.254.0.249 dev swp8 proto static
(But have it choose a non-selected connected nexthop)

c) Introduce an event that causes the rib table to be reprocessed,
say a unrelated interface going up / down

  This causes the route to be lost with this message:
2022/03/28 21:21:53 ZEBRA: [YXCJP-0WZWV] netlink_nexthop_msg_encode: ID (3454): 169.254.0.249, via swp8(1383) vrf default(0)
2022/03/28 21:21:53 ZEBRA: [YF2E6-J60JH] nexthop_active: 169.254.0.249, via swp8 given ifindex does not match nexthops ifindex found found: directly connected, swp9

Effectively the nexthop that zebra is choosing would not be the one
that the kernel route has choosen and FRR removes the route:
022/03/28 21:21:53 ZEBRA: [NM15X-X83N9] rib_process: (0:254):7.6.2.8/32: rn 0x56042e632e90, removing re 0x56042e6316e0
2022/03/28 21:21:53 ZEBRA: [Y53JX-CBC5H] rib_unlink: (0:254):7.6.2.8/32: rn 0x56042e632e90, re 0x56042e6316e0
2022/03/28 21:21:53 ZEBRA: [KT8QQ-45WQ0] rib_gc_dest: (0:?):7.6.2.8/32: removing dest from table

What is happening?

Zebra is not looking at all connected routes and if any of them
would have the appropriate ifindex and just blindly rejecting
the route.

So when nexthop resolution happens and it matches a connected
route and the dest->selected nexthop ifindex does not match, let's sort
through the rest of them and see if any of them match and if so
let's keep the route.

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

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index 1b926dba5ff797d70846fe66ab5b8e8a03b04f99..02894632eadac22f303132b11e5d1927b29670cb 100644 (file)
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -43,6 +43,7 @@
 #include "zebra_dplane.h"
 #include "zebra/interface.h"
 #include "zebra/zapi_msg.h"
+#include "zebra/rib.h"
 
 DEFINE_MTYPE_STATIC(ZEBRA, NHG, "Nexthop Group Entry");
 DEFINE_MTYPE_STATIC(ZEBRA, NHG_CONNECTED, "Nexthop Group Connected");
@@ -1959,6 +1960,61 @@ static int resolve_backup_nexthops(const struct nexthop *nexthop,
        return 0;
 }
 
+/*
+ * So this nexthop resolution has decided that a connected route
+ * is the correct choice.  At this point in time if FRR has multiple
+ * connected routes that all point to the same prefix one will be
+ * selected, *but* the particular interface may not be the one
+ * that the nexthop points at.  Let's look at all the available
+ * connected routes on this node and if any of them auto match
+ * the routes nexthops ifindex that is good enough for a match
+ *
+ * This code is depending on the fact that a nexthop->ifindex is 0
+ * if it is not known, if this assumption changes, yummy!
+ * Additionally a ifindx of 0 means figure it out for us.
+ */
+static struct route_entry *
+zebra_nhg_connected_ifindex(struct route_node *rn, struct route_entry *match,
+                           int32_t curr_ifindex)
+{
+       struct nexthop *newhop = match->nhe->nhg.nexthop;
+       struct route_entry *re;
+
+       assert(newhop); /* What a kick in the patooey */
+
+       if (curr_ifindex == 0)
+               return match;
+
+       if (curr_ifindex == newhop->ifindex)
+               return match;
+
+       /*
+        * At this point we know that this route is matching a connected
+        * but there are possibly a bunch of connected routes that are
+        * alive that should be considered as well.  So let's iterate over
+        * all the re's and see if they are connected as well and maybe one
+        * of those ifindexes match as well.
+        */
+       RNODE_FOREACH_RE (rn, re) {
+               if (re->type != ZEBRA_ROUTE_CONNECT)
+                       continue;
+
+               if (CHECK_FLAG(re->status, ROUTE_ENTRY_REMOVED))
+                       continue;
+
+               /*
+                * zebra has a connected route that is not removed
+                * let's test if it is good
+                */
+               newhop = re->nhe->nhg.nexthop;
+               assert(newhop);
+               if (curr_ifindex == newhop->ifindex)
+                       return re;
+       }
+
+       return match;
+}
+
 /*
  * Given a nexthop we need to properly recursively resolve,
  * do a table lookup to find and match if at all possible.
@@ -2210,24 +2266,23 @@ static int nexthop_active(struct nexthop *nexthop, struct nhg_hash_entry *nhe,
                }
 
                if (match->type == ZEBRA_ROUTE_CONNECT) {
-                       /* Directly point connected route. */
+                       match = zebra_nhg_connected_ifindex(rn, match,
+                                                           nexthop->ifindex);
+
                        newhop = match->nhe->nhg.nexthop;
-                       if (newhop) {
-                               if (nexthop->type == NEXTHOP_TYPE_IPV4
-                                   || nexthop->type == NEXTHOP_TYPE_IPV6)
-                                       nexthop->ifindex = newhop->ifindex;
-                               else if (nexthop->ifindex != newhop->ifindex) {
-                                       if (IS_ZEBRA_DEBUG_RIB_DETAILED)
-                                               zlog_debug(
-                                                       "%s: %pNHv given ifindex does not match nexthops ifindex found found: %pNHv",
-                                                       __func__, nexthop,
-                                                       newhop);
-                                       /*
-                                        * NEXTHOP_TYPE_*_IFINDEX but ifindex
-                                        * doesn't match what we found.
-                                        */
-                                       return 0;
-                               }
+                       if (nexthop->type == NEXTHOP_TYPE_IPV4 ||
+                           nexthop->type == NEXTHOP_TYPE_IPV6)
+                               nexthop->ifindex = newhop->ifindex;
+                       else if (nexthop->ifindex != newhop->ifindex) {
+                               if (IS_ZEBRA_DEBUG_RIB_DETAILED)
+                                       zlog_debug(
+                                               "%s: %pNHv given ifindex does not match nexthops ifindex found: %pNHv",
+                                               __func__, nexthop, newhop);
+                               /*
+                                * NEXTHOP_TYPE_*_IFINDEX but ifindex
+                                * doesn't match what we found.
+                                */
+                               return 0;
                        }
 
                        if (IS_ZEBRA_DEBUG_NHG_DETAIL)