A route can describe its next hop with a separate nexthop object (see
'ip nexthop') that it refers to through a nexthop id carried in the
RTA_NH_ID attribute, instead of encoding the next hop inline.  Routing
daemons commonly install routes this way, as it lets many routes share
one next hop and be redirected at once.

With the net.ipv4.nexthop_compat_mode sysctl turned off the kernel
reports nothing but that id.  Such a route then carries none of RTA_OIF,
RTA_GATEWAY, RTA_VIA and RTA_MULTIPATH, so it was considered unparseable
and dropped.

Accept RTA_NH_ID and store it in 'struct route_data' so that users can
look the nexthop object up in the kernel nexthop table themselves.  The
placeholder next hop is removed from the list whenever the route carries
nothing but the id, so an empty list tells the user that the next hop
still has to be resolved.

Assisted-by: Claude Opus 5, Cursor
Signed-off-by: Han Zhou <[email protected]>
---
 lib/route-table.c            | 21 +++++++++++--
 lib/route-table.h            |  7 +++++
 tests/system-route.at        | 57 ++++++++++++++++++++++++++++++++++++
 tests/test-lib-route-table.c |  5 ++--
 4 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/lib/route-table.c b/lib/route-table.c
index 2a13a5cc7d93..157479da850c 100644
--- a/lib/route-table.c
+++ b/lib/route-table.c
@@ -45,6 +45,7 @@
  * in case we're building with old headers. (We can't test for it with #ifdef
  * because it's an enum.) */
 #define RTA_MARK 16 /* Linux 2.6.36 */
+#define RTA_NH_ID 30 /* Linux 5.3 */
 #define FRA_SUPPRESS_PREFIXLEN 14 /* Linux 3.12 */
 #define FRA_TABLE 15 /* Linux 2.6.19 */
 #define FRA_PROTOCOL 21 /* Linux 4.17 */
@@ -468,6 +469,7 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
         [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
         [RTA_VIA] = { .type = NL_A_RTA_VIA, .optional = true },
         [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true },
+        [RTA_NH_ID] = { .type = NL_A_U32, .optional = true },
     };
 
     static const struct nl_policy policy6[] = {
@@ -480,6 +482,7 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
         [RTA_PRIORITY] = { .type = NL_A_U32, .optional = true },
         [RTA_VIA] = { .type = NL_A_RTA_VIA, .optional = true },
         [RTA_MULTIPATH] = { .type = NL_A_NESTED, .optional = true },
+        [RTA_NH_ID] = { .type = NL_A_U32, .optional = true },
     };
 
     struct nlattr *attrs[ARRAY_SIZE(policy)];
@@ -585,6 +588,9 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
         if (attrs[RTA_PRIORITY]) {
             change->rd.rta_priority = nl_attr_get_u32(attrs[RTA_PRIORITY]);
         }
+        if (attrs[RTA_NH_ID]) {
+            change->rd.rta_nhid = nl_attr_get_u32(attrs[RTA_NH_ID]);
+        }
         if (attrs[RTA_VIA]) {
             const struct rtvia *rtvia = nl_attr_get(attrs[RTA_VIA]);
             ovs_be32 addr;
@@ -672,9 +678,10 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
         }
         if (route_type_needs_nexthop(rtm->rtm_type)
             && !attrs[RTA_OIF] && !attrs[RTA_GATEWAY]
-            && !attrs[RTA_VIA] && !attrs[RTA_MULTIPATH]) {
+            && !attrs[RTA_VIA] && !attrs[RTA_MULTIPATH]
+            && !attrs[RTA_NH_ID]) {
             VLOG_DBG_RL(&rl, "route message needs an RTA_OIF, RTA_GATEWAY, "
-                             "RTA_VIA or RTA_MULTIPATH attribute");
+                             "RTA_VIA, RTA_MULTIPATH or RTA_NH_ID attribute");
             goto error_out;
         }
         /* Add any additional RTA attribute processing before RTA_MULTIPATH. */
@@ -684,6 +691,16 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
         if (!route_type_needs_nexthop(rtm->rtm_type)) {
             route_data_destroy_nexthops__(&change->rd);
         }
+
+        /* When the route resolves through a separate nexthop object
+         * (RTA_NH_ID) there is no inline next hop information in this message.
+         * Drop the empty placeholder next hop so that consumers can tell that
+         * the next hop(s) have to be resolved via the kernel nexthop table. */
+        if (change->rd.rta_nhid
+            && !attrs[RTA_OIF] && !attrs[RTA_GATEWAY]
+            && !attrs[RTA_VIA] && !attrs[RTA_MULTIPATH]) {
+            route_data_destroy_nexthops__(&change->rd);
+        }
     } else {
         VLOG_DBG_RL(&rl, "received unparseable rtnetlink route message");
         goto error_out;
diff --git a/lib/route-table.h b/lib/route-table.h
index b49fbb14ebe0..4ef0b6c8d0f6 100644
--- a/lib/route-table.h
+++ b/lib/route-table.h
@@ -141,6 +141,13 @@ struct route_data {
     uint32_t rta_mark;           /* 0 if missing. */
     uint32_t rta_table_id;       /* 0 if missing. */
     uint32_t rta_priority;       /* 0 if missing. */
+
+    /* Id of the nexthop object (RTA_NH_ID) this route resolves through, 0 if
+     * missing.  When set, the route's next hop(s) are not described inline in
+     * this message; they are stored in a separate nexthop object that has to
+     * be looked up in the kernel nexthop table.  In that case the 'nexthops'
+     * list above is left empty. */
+    uint32_t rta_nhid;
 };
 
 struct rule_data {
diff --git a/tests/system-route.at b/tests/system-route.at
index a074c51f9fd0..3f28e3d93cbf 100644
--- a/tests/system-route.at
+++ b/tests/system-route.at
@@ -333,6 +333,63 @@ AT_CHECK([ovstest test-lib-route-table-dump | \
 
 AT_CLEANUP
 
+dnl Checks that routes whose next hop is described by a separate nexthop
+dnl object (referenced through a nexthop id, RTA_NH_ID) are parsed and expose
+dnl the nexthop id, instead of being dropped.
+AT_SETUP([route-table - route with nexthop id])
+AT_KEYWORDS([route])
+
+dnl Skip if the running kernel / iproute2 does not support nexthop objects.
+AT_SKIP_IF([! ip nexthop show >/dev/null 2>&1])
+
+dnl Make the kernel describe these routes through the nexthop id alone.  In
+dnl compatibility mode, which is the default, it also reports the resolved
+dnl next hop inline, and that is precisely what must not be relied upon here.
+compat_mode=$(sysctl -n net.ipv4.nexthop_compat_mode)
+on_exit "sysctl -wq net.ipv4.nexthop_compat_mode=$compat_mode"
+AT_CHECK([sysctl -wq net.ipv4.nexthop_compat_mode=0])
+
+dnl Create a port.  A nexthop object can only reference a device that has
+dnl carrier, which an unattached tap port does not have.
+AT_CHECK([ip link add p1-route type dummy])
+AT_CHECK([ip link set p1-route up])
+on_exit 'ip link del p1-route'
+
+AT_CHECK([ip addr add 10.0.0.17/24 dev p1-route], [0], [stdout])
+AT_CHECK([ip -6 addr add fc00:db8:cafe::17/64 dev p1-route], [0], [stdout])
+
+dnl Create standalone nexthop objects and a nexthop group referencing them.
+on_exit 'ip nexthop del id 110; ip nexthop del id 100; ip nexthop del id 101; 
ip nexthop del id 102'
+AT_CHECK([ip nexthop add id 100 via 10.0.0.18 dev p1-route])
+AT_CHECK([ip nexthop add id 101 via 10.0.0.19 dev p1-route])
+AT_CHECK([ip nexthop add id 110 group 100/101])
+AT_CHECK([ip nexthop add id 102 via fc00:db8:cafe::18 dev p1-route])
+
+dnl Add routes that reference their next hop indirectly through a nexthop id.
+AT_CHECK([ip route add 192.168.10.0/24 nhid 100], [0], [stdout])
+AT_CHECK([ip route add 192.168.20.0/24 nhid 110], [0], [stdout])
+OVS_WAIT_UNTIL([ip -6 route add fc00:db8:beef::/64 nhid 102])
+
+dnl The routes must be parsed as relevant and expose the referenced nexthop id.
+AT_CHECK([ovstest test-lib-route-table-dump | \
+          awk '/^192.168.10.0/{print$1" "$3" "$19" "$20}'], [0], [dnl
+192.168.10.0/24 1 rta_nhid: 100
+])
+AT_CHECK([ovstest test-lib-route-table-dump | \
+          awk '/^192.168.20.0/{print$1" "$3" "$19" "$20}'], [0], [dnl
+192.168.20.0/24 1 rta_nhid: 110
+])
+AT_CHECK([ovstest test-lib-route-table-dump | \
+          awk '/^fc00:db8:beef::/{print$1" "$3" "$19" "$20}'], [0], [dnl
+fc00:db8:beef::/64 1 rta_nhid: 102
+])
+
+dnl Since the next hop is not encoded inline, no nexthop entry is expected.
+AT_CHECK([ovstest test-lib-route-table-dump | \
+          grep -E '192.168.10.0.*nexthop family'], [1])
+
+AT_CLEANUP
+
 dnl Checks that OVS ignores unsupported routing rules.
 AT_SETUP([ovs-route - unsupported rules])
 AT_KEYWORDS([route])
diff --git a/tests/test-lib-route-table.c b/tests/test-lib-route-table.c
index f99f056c8ddc..d6fe20cb180e 100644
--- a/tests/test-lib-route-table.c
+++ b/tests/test-lib-route-table.c
@@ -86,11 +86,12 @@ test_lib_route_table_handle_msg(const struct 
route_table_msg *change,
 
     printf("%s/%u relevant: %d nlmsg_type: %d rtm_protocol: %s (%u) "
            "rtn_local: %d rta_prefsrc: %s rta_mark: %"PRIu32" "
-           "rta_table_id: %s rta_priority: %"PRIu32"\n",
+           "rta_table_id: %s rta_priority: %"PRIu32" rta_nhid: %"PRIu32"\n",
            ds_cstr(&rta_dst), rd->rtm_dst_len, change->relevant,
            change->nlmsg_type, rt_prot_name(rd->rtm_protocol),
            rd->rtm_protocol, rd->rtn_local, ds_cstr(&rta_prefsrc),
-           rd->rta_mark, rt_table_name(rd->rta_table_id), rd->rta_priority);
+           rd->rta_mark, rt_table_name(rd->rta_table_id), rd->rta_priority,
+           rd->rta_nhid);
 
     LIST_FOR_EACH (rdnh, nexthop_node, &rd->nexthops) {
         ds_clear(&nexthop_addr);
-- 
2.38.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to