Hi Han, Thanks for the patch!
On 7/21/26 9:03 AM, Han Zhou wrote: > Routes can carry lightweight tunnel (LWT) encapsulation metadata; for IP > encapsulation this includes a tunnel id (e.g. a VXLAN VNI). FRR uses > this to attach the L3 VNI to EVPN type-5 routes. > > Parse the LWT IP/IP6 encap tunnel id (LWTUNNEL_IP_ID) from RTA_ENCAP into > a new 'vni' field in struct route_data, so that users of the route-table > library (such as OVN) can learn it. Why only parse the VNI though? Even the test you're adding is setting an explicit LWTUNNEL_IP_DST. I understand that in your OVN use case only the VNI is set on the route but this is a generic library. I think it might be nicer if we parsed all the lwtunnel attributes. > > Assisted-by: Claude Opus 4.8, Cursor > Signed-off-by: Han Zhou <[email protected]> > --- > lib/route-table.c | 42 ++++++++++++++++++++++++++++++++++++ > lib/route-table.h | 6 ++++++ > tests/system-route.at | 14 ++++++++++++ > tests/test-lib-route-table.c | 6 +++++- > 4 files changed, 67 insertions(+), 1 deletion(-) > > diff --git a/lib/route-table.c b/lib/route-table.c > index 2a13a5cc7d93..ecaf6fc4e42b 100644 > --- a/lib/route-table.c > +++ b/lib/route-table.c > @@ -27,6 +27,7 @@ > #include <linux/rtnetlink.h> > #include <net/if.h> > > +#include "byte-order.h" > #include "coverage.h" > #include "hash.h" > #include "netdev.h" > @@ -48,6 +49,16 @@ > #define FRA_SUPPRESS_PREFIXLEN 14 /* Linux 3.12 */ > #define FRA_TABLE 15 /* Linux 2.6.19 */ > #define FRA_PROTOCOL 21 /* Linux 4.17 */ > +#define RTA_ENCAP_TYPE 21 /* Linux 4.3 */ > +#define RTA_ENCAP 22 /* Linux 4.3 */ > + > +/* Lightweight tunnel encapsulation types and attributes (Linux 4.3+), > + * from linux/lwtunnel.h. Defined here so we can parse EVPN type-5 routes > + * that carry the L3 VNI without depending on newer build headers. */ > +#define OVN_LWTUNNEL_ENCAP_IP 2 > +#define OVN_LWTUNNEL_ENCAP_IP6 4 Why don't you use the uapi/linux/lwtunnel.h definitions directly (LWTUNNEL_ENCAP_IP and LWTUNNEL_ENCAP_IP6)? Having OVN specific macro names here reads weird. > +/* LWTUNNEL_IP_ID and LWTUNNEL_IP6_ID share the same attribute number (1). */ > +#define OVN_LWTUNNEL_IP_ID 1 > > /* Linux 4.1 added RTA_VIA. */ > #ifndef HAVE_RTA_VIA > @@ -468,6 +479,8 @@ 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_ENCAP_TYPE] = { .type = NL_A_U16, .optional = true }, > + [RTA_ENCAP] = { .type = NL_A_NESTED, .optional = true }, > }; > > static const struct nl_policy policy6[] = { > @@ -480,6 +493,8 @@ 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_ENCAP_TYPE] = { .type = NL_A_U16, .optional = true }, > + [RTA_ENCAP] = { .type = NL_A_NESTED, .optional = true }, > }; > > struct nlattr *attrs[ARRAY_SIZE(policy)]; > @@ -585,6 +600,27 @@ 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_ENCAP_TYPE] && attrs[RTA_ENCAP]) { > + uint16_t encap_type = nl_attr_get_u16(attrs[RTA_ENCAP_TYPE]); > + > + if (encap_type == OVN_LWTUNNEL_ENCAP_IP || > + encap_type == OVN_LWTUNNEL_ENCAP_IP6) { > + static const struct nl_policy encap_policy[] = { > + [OVN_LWTUNNEL_IP_ID] = { .type = NL_A_BE64, > + .optional = true }, > + }; > + struct nlattr *encap_attrs[ARRAY_SIZE(encap_policy)]; > + > + if (nl_parse_nested(attrs[RTA_ENCAP], encap_policy, > + encap_attrs, ARRAY_SIZE(encap_policy)) > + && encap_attrs[OVN_LWTUNNEL_IP_ID]) { > + ovs_be64 id = > + nl_attr_get_be64(encap_attrs[OVN_LWTUNNEL_IP_ID]); > + change->rd.vni = ntohll(id); We truncate a 64bit integer into 32bits. Is that acceptable? I understand for VXLAN VNI that's fine but is that true in general? > + change->rd.vni_present = true; > + } > + } > + } > if (attrs[RTA_VIA]) { > const struct rtvia *rtvia = nl_attr_get(attrs[RTA_VIA]); > ovs_be32 addr; > @@ -668,6 +704,12 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, > } > ovs_list_push_back_all(&change->rd.nexthops, > &mp_change.rd.nexthops); > + /* Per-nexthop LWT encap may carry the VNI. Adopt the first > + * one seen for the route as a whole. */ > + if (mp_change.rd.vni_present && !change->rd.vni_present) { > + change->rd.vni = mp_change.rd.vni; > + change->rd.vni_present = true; > + } > } > } > if (route_type_needs_nexthop(rtm->rtm_type) > diff --git a/lib/route-table.h b/lib/route-table.h > index b49fbb14ebe0..672d4e96d1d7 100644 > --- a/lib/route-table.h > +++ b/lib/route-table.h > @@ -141,6 +141,12 @@ 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. */ > + > + /* Lightweight tunnel (LWT) encapsulation, e.g. as used by EVPN type-5 > + * routes to carry the L3 VNI (RTA_ENCAP_TYPE == LWTUNNEL_ENCAP_IP/IP6 > + * with a nested LWTUNNEL_IP_ID/LWTUNNEL_IP6_ID tunnel id). */ > + bool vni_present; /* True if 'vni' was extracted. */ Tunnel ID 0 is probably valid so I guess that's why we need this bool. > + uint32_t vni; /* Valid only if 'vni_present'. */ Maybe VNI is too VXLAN-specific? In the end this is just a tunnel ID. > }; > > struct rule_data { > diff --git a/tests/system-route.at b/tests/system-route.at > index a074c51f9fd0..ed70a2443e76 100644 > --- a/tests/system-route.at > +++ b/tests/system-route.at > @@ -331,6 +331,20 @@ AT_CHECK([ovstest test-lib-route-table-dump | \ > 192.168.10.12/32 rtm_protocol: RTPROT_BOOT > ]) > > +dnl Add route with a lightweight-tunnel (LWT) IP encapsulation tunnel id, as > +dnl used to carry an EVPN L3 VNI. The tunnel id must be parsed into the > route's > +dnl vni field. > +AT_CHECK([ip route add 192.168.10.13/32 encap ip id 5000 dst 10.0.0.19 \ > + dev p1-route via 10.0.0.18], [0], [stdout]) > +AT_CHECK([ovstest test-lib-route-table-dump | grep '^192.168.10.13' | \ > + grep -o 'vni: [[0-9]]*'], [0], [dnl > +vni: 5000 > +]) > + > +dnl A route without an encap id has no vni field. > +AT_CHECK([ovstest test-lib-route-table-dump | grep '^192.168.10.12' | \ > + grep 'vni:'], [1]) > + I think it would probably be a good idea to add tests for ip6 encaps too. > AT_CLEANUP > > dnl Checks that OVS ignores unsupported routing rules. > diff --git a/tests/test-lib-route-table.c b/tests/test-lib-route-table.c > index f99f056c8ddc..3bdaf517f132 100644 > --- a/tests/test-lib-route-table.c > +++ b/tests/test-lib-route-table.c > @@ -86,11 +86,15 @@ 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, > 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); > + if (rd->vni_present) { > + printf(" vni: %"PRIu32, rd->vni); > + } > + printf("\n"); > > LIST_FOR_EACH (rdnh, nexthop_node, &rd->nexthops) { > ds_clear(&nexthop_addr); Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
