On 4/2/26 10:25 AM, Ales Musil wrote: > The nexthop table contains entries about nexthop and groups of them. > This is utilized by EVPN in scenarios when there are remote VTEPs > configured active-active multihoming. Add support for syncing the > table from netlink, those data will be later on used to configure > the multihoming for OVN EVPN. > > Reported-at: https://redhat.atlassian.net/browse/FDP-3071 > Signed-off-by: Ales Musil <[email protected]> > ---
Hi Ales, Thanks for working on this, it looks good overall! > configure.ac | 1 + > controller/automake.mk | 3 + > controller/nexthop-exchange-stub.c | 42 ++++++ > controller/nexthop-exchange.c | 230 +++++++++++++++++++++++++++++ > controller/nexthop-exchange.h | 61 ++++++++ > controller/ovn-netlink-notifier.c | 43 ++++++ > controller/ovn-netlink-notifier.h | 1 + > m4/ovn.m4 | 16 ++ > tests/automake.mk | 2 + > tests/system-ovn-netlink.at | 117 +++++++++++++++ > tests/test-ovn-netlink.c | 57 +++++++ > 11 files changed, 573 insertions(+) > create mode 100644 controller/nexthop-exchange-stub.c > create mode 100644 controller/nexthop-exchange.c > create mode 100644 controller/nexthop-exchange.h > > diff --git a/configure.ac b/configure.ac > index 657769609..025dccc63 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -174,6 +174,7 @@ OVS_CHECK_PRAGMA_MESSAGE > OVN_CHECK_VERSION_SUFFIX > OVN_CHECK_OVS > OVN_CHECK_VIF_PLUG_PROVIDER > +OVN_CHECK_LINUX_NEXTHOP_WEIGHT > OVN_ENABLE_VIF_PLUG > OVS_CTAGS_IDENTIFIERS > AC_SUBST([OVS_CFLAGS]) > diff --git a/controller/automake.mk b/controller/automake.mk > index c37b89b6c..a779250cb 100644 > --- a/controller/automake.mk > +++ b/controller/automake.mk > @@ -78,6 +78,8 @@ controller_ovn_controller_SOURCES += \ > controller/neighbor-exchange-netlink.h \ > controller/neighbor-exchange-netlink.c \ > controller/neighbor-exchange.c \ > + controller/nexthop-exchange.h \ > + controller/nexthop-exchange.c \ > controller/route-exchange-netlink.h \ > controller/route-exchange-netlink.c \ > controller/route-exchange.c > @@ -86,6 +88,7 @@ controller_ovn_controller_SOURCES += \ > controller/host-if-monitor-stub.c \ > controller/ovn-netlink-notifier-stub.c \ > controller/neighbor-exchange-stub.c \ > + controller/nexthop-exchange-stub.c \ > controller/route-exchange-stub.c > endif > > diff --git a/controller/nexthop-exchange-stub.c > b/controller/nexthop-exchange-stub.c > new file mode 100644 > index 000000000..2742dc7e2 > --- /dev/null > +++ b/controller/nexthop-exchange-stub.c > @@ -0,0 +1,42 @@ > +/* Copyright (c) 2026, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > + > +#include "lib/netlink.h" > +#include "openvswitch/hmap.h" > +#include "openvswitch/ofpbuf.h" > + > +#include "nexthop-exchange.h" > + > +/* Populates 'nexthops' with all nexthop entries > + * (struct nexthop_entry) with fdb flag set that exist in the table. */ > +void > +nexthops_sync(struct hmap *nexthops OVS_UNUSED) > +{ > +} > + > +void > +nexthop_entry_format(struct ds *ds OVS_UNUSED, > + const struct nexthop_entry *nhe OVS_UNUSED) > +{ > +} > + > +int > +nh_table_parse(struct ofpbuf *buf OVS_UNUSED, > + struct nh_table_msg *change OVS_UNUSED) > +{ > + return 0; > +} > diff --git a/controller/nexthop-exchange.c b/controller/nexthop-exchange.c > new file mode 100644 > index 000000000..e1a5bfe1d > --- /dev/null > +++ b/controller/nexthop-exchange.c > @@ -0,0 +1,230 @@ > +/* Copyright (c) 2026, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > +#include <linux/rtnetlink.h> > +#include <linux/nexthop.h> > + > +#include "lib/netlink.h" > +#include "lib/netlink-socket.h" > +#include "openvswitch/ofpbuf.h" > +#include "openvswitch/vlog.h" > +#include "packets.h" > + > +#include "nexthop-exchange.h" > + > +VLOG_DEFINE_THIS_MODULE(nexthop_exchange_netlink); Nit: "nexthop_exchange" might be a bit more appropriate. > + > +#define NETNL_REQ_BUFFER_SIZE 128 > + > +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > + > +static int nh_table_parse__(struct ofpbuf *, size_t ofs, > + const struct nlmsghdr *, > + struct nh_table_msg *); > +static void nh_populate_grp_pointers(struct nexthop_entry *, struct hmap *); > + > +/* The following definition should be available in Linux 6.12 and might be > + * missing if we have older headers. */ > +#ifndef HAVE_NH_GRP_WEIGHT > +static uint16_t > +nexthop_grp_weight(const struct nexthop_grp *entry) > +{ > + return entry->weight + 1; > +} > +#endif > + > +/* Populates 'nexthops' with all nexthop entries > + * (struct nexthop_entry) with fdb flag set that exist in the table. */ > +void > +nexthops_sync(struct hmap *nexthops) > +{ > + uint64_t reply_stub[NL_DUMP_BUFSIZE / 8]; > + struct ofpbuf request, reply, buf; > + struct nl_dump dump; > + > + uint8_t request_stub[NETNL_REQ_BUFFER_SIZE]; > + ofpbuf_use_stub(&request, request_stub, sizeof request_stub); > + > + nl_msg_put_nlmsghdr(&request, sizeof(struct nhmsg), > + RTM_GETNEXTHOP, NLM_F_REQUEST); > + ofpbuf_put_zeros(&request, sizeof(struct nhmsg)); > + nl_dump_start(&dump, NETLINK_ROUTE, &request); > + ofpbuf_uninit(&request); > + > + ofpbuf_use_stub(&buf, reply_stub, sizeof reply_stub); > + while (nl_dump_next(&dump, &reply, &buf)) { > + struct nh_table_msg msg; > + > + if (!nh_table_parse(&reply, &msg)) { > + continue; > + } > + > + hmap_insert(nexthops, &msg.nhe->hmap_node, msg.nhe->id); Maybe hash_int(nhe->id)? Maybe also explicitly check for dups first? > + } > + ofpbuf_uninit(&buf); > + nl_dump_done(&dump); > + > + struct nexthop_entry *nhe; > + HMAP_FOR_EACH (nhe, hmap_node, nexthops) { > + nh_populate_grp_pointers(nhe, nexthops); > + } > +} > + > +void > +nexthop_entry_format(struct ds *ds, const struct nexthop_entry *nhe) > +{ > + ds_put_format(ds, "id=%"PRIu32", ", nhe->id); > + if (!nhe->n_grps) { > + ds_put_cstr(ds, "address="); > + ipv6_format_mapped(&nhe->addr, ds); > + } else { > + ds_put_cstr(ds, "group=["); > + for (size_t i = 0; i < nhe->n_grps; i++) { > + const struct nexthop_grp_entry *grp = &nhe->grps[i]; > + ds_put_format(ds, "%"PRIu32";", grp->id); > + if (grp->gateway) { > + ipv6_format_mapped(&grp->gateway->addr, ds); > + ds_put_char(ds, ';'); > + } > + ds_put_format(ds, "%"PRIu16", ", grp->weight); > + } > + > + ds_truncate(ds, ds->length - 2); > + ds_put_char(ds, ']'); > + } > +} > + > +/* Parse Netlink message in buf, which is expected to contain a UAPI nhmsg > + * header and associated nexthop attributes. This will allocate > + * 'struct nexthop_entry' which needs to be freed by the caller. > + * > + * Return RTNLGRP_NEXTHOP on success, and 0 on a parse error. */ > +int > +nh_table_parse(struct ofpbuf *buf, struct nh_table_msg *change) > +{ > + struct nlmsghdr *nlmsg = ofpbuf_at(buf, 0, NLMSG_HDRLEN); > + struct nhmsg *nh = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *nh); > + > + if (!nlmsg || !nh) { > + return 0; > + } > + > + return nh_table_parse__(buf, NLMSG_HDRLEN + sizeof *nh, > + nlmsg, change); > +} > + > +static int > +nh_table_parse__(struct ofpbuf *buf, size_t ofs, const struct nlmsghdr > *nlmsg, > + struct nh_table_msg *change) > +{ > + bool parsed; > + > + static const struct nl_policy policy[] = { > + [NHA_ID] = { .type = NL_A_U32 }, > + [NHA_FDB] = { .type = NL_A_FLAG, .optional = true }, > + [NHA_GROUP] = { .type = NL_A_UNSPEC, .optional = true, > + .min_len = sizeof(struct nexthop_grp) }, > + [NHA_GROUP_TYPE] = { .type = NL_A_U16, .optional = true }, Are we actually planning to support resilient groups [0] in OVN? If not we can skip parsing the group type. If we do, we should store it in the parsed 'struct nexthop_entry'. [0] https://github.com/torvalds/linux/blob/028ef9c96e96197026887c0f092424679298aae8/Documentation/networking/nexthop-group-resilient.rst#L171 > + [NHA_GATEWAY] = { .type = NL_A_UNSPEC, .optional = true, > + .min_len = sizeof(struct in_addr), > + .max_len = sizeof(struct in6_addr) }, > + }; > + > + struct nlattr *attrs[ARRAY_SIZE(policy)]; > + parsed = nl_policy_parse(buf, ofs, policy, attrs, ARRAY_SIZE(policy)); > + > + if (!parsed) { > + VLOG_DBG_RL(&rl, "received unparseable rtnetlink nexthop message"); > + return 0; > + } > + > + if (!nl_attr_get_flag(attrs[NHA_FDB])) { > + return 0; > + } > + > + const struct nexthop_grp *grps = NULL; > + struct in6_addr addr = in6addr_any; > + size_t n_grps = 0; > + > + if (attrs[NHA_GATEWAY]) { > + size_t nda_dst_size = nl_attr_get_size(attrs[NHA_GATEWAY]); > + > + switch (nda_dst_size) { > + case sizeof(uint32_t): > + in6_addr_set_mapped_ipv4(&addr, > + nl_attr_get_be32(attrs[NHA_GATEWAY])); > + break; > + case sizeof(struct in6_addr): > + addr = nl_attr_get_in6_addr(attrs[NHA_GATEWAY]); > + break; > + default: > + VLOG_DBG_RL(&rl, > + "nexthop message contains non-IPv4/IPv6 > NHA_GATEWAY"); > + return 0; > + } > + } else if (attrs[NHA_GROUP]) { > + n_grps = nl_attr_get_size(attrs[NHA_GROUP]) / sizeof *grps; > + grps = nl_attr_get(attrs[NHA_GROUP]); > + } else { > + VLOG_DBG_RL(&rl, "missing group or gateway nexthop attribute"); > + return 0; > + } > + > + size_t grp_size = n_grps * sizeof(struct nexthop_grp_entry); > + change->nlmsg_type = nlmsg->nlmsg_type; > + change->nhe = xmalloc(sizeof *change->nhe + grp_size); > + *change->nhe = (struct nexthop_entry) { > + .id = nl_attr_get_u32(attrs[NHA_ID]), > + .addr = addr, > + .n_grps = n_grps, > + }; > + > + for (size_t i = 0; i < n_grps; i++) { > + const struct nexthop_grp *grp = &grps[i]; > + change->nhe->grps[i] = (struct nexthop_grp_entry) { > + .id = grp->id, > + .weight = nexthop_grp_weight(grp), > + /* We need to parse all entries first > + * before adjusting the references. */ Nit: maybe say "adjusting the references in 'nh_populate_grp_pointers()'". Also the multiline comment split looks a bit weird to me. > + .gateway = NULL, > + }; > + } > + > + /* Success. */ > + return RTNLGRP_NEXTHOP; > +} > + > +static struct nexthop_entry * > +nexthop_find(struct hmap *nexthops, uint32_t id) > +{ > + struct nexthop_entry *nhe; > + HMAP_FOR_EACH_WITH_HASH (nhe, hmap_node, id, nexthops) { Potentially hash_int(id). > + if (nhe->id == id) { > + return nhe; > + } > + } > + > + return NULL; > +} > + > +static void > +nh_populate_grp_pointers(struct nexthop_entry *nhe, struct hmap *nexthops) > +{ > + for (size_t i = 0; i < nhe->n_grps; i++) { > + struct nexthop_grp_entry *grp = &nhe->grps[i]; > + grp->gateway = nexthop_find(nexthops, grp->id); > + } > +} > diff --git a/controller/nexthop-exchange.h b/controller/nexthop-exchange.h > new file mode 100644 > index 000000000..94f4e9bf2 > --- /dev/null > +++ b/controller/nexthop-exchange.h > @@ -0,0 +1,61 @@ > +/* Copyright (c) 2026, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#ifndef NEXTHOP_EXCHANGE_H > +#define NEXTHOP_EXCHANGE_H 1 > + > +#include <netinet/in.h> > +#include <stdint.h> > + > +#include "openvswitch/hmap.h" > + > +struct ds; > +struct ofpbuf; > + > +struct nexthop_grp_entry { > + /* The id of the nexthop gateway. */ > + uint32_t id; > + /* The weight of the entry. */ > + uint16_t weight; > + /* The pointer to the gateway entry. */ > + struct nexthop_entry *gateway; > +}; > + > +struct nexthop_entry { > + struct hmap_node hmap_node; > + /* The id of the nexthop. */ > + uint32_t id; > + /* Nexthop IP address, zeroed in case of group entry. */ > + struct in6_addr addr; > + /* Number of group entries, "0" in case gateway entry. */ Typo? "in case of gateway.." > + size_t n_grps; > + /* Array of group entries. */ > + struct nexthop_grp_entry grps[]; > +}; > + > +/* A digested version of a nexthop message sent down by the kernel to > indicate > + * that a nexthop entry has changed. */ > +struct nh_table_msg { > + /* E.g. RTM_NEWNEXTHOP, RTM_DELNEXTHOP. */ > + uint16_t nlmsg_type; > + /* The inner entry. */ > + struct nexthop_entry *nhe; > +}; > + > +void nexthops_sync(struct hmap *); Nit: because we use a raw hmap I'd keep the argument name here, i.e.: "struct hmap *nexthops". > +void nexthop_entry_format(struct ds *ds, const struct nexthop_entry *nhe); > +int nh_table_parse(struct ofpbuf *, struct nh_table_msg *change); > + > +#endif /* NEXTHOP_EXCHANGE_H */ > diff --git a/controller/ovn-netlink-notifier.c > b/controller/ovn-netlink-notifier.c > index defa1cd54..7d44f8765 100644 > --- a/controller/ovn-netlink-notifier.c > +++ b/controller/ovn-netlink-notifier.c > @@ -19,6 +19,7 @@ > #include <net/if.h> > > #include "neighbor-exchange-netlink.h" > +#include "nexthop-exchange.h" > #include "netlink-notifier.h" > #include "route-exchange-netlink.h" > #include "route-table.h" > @@ -48,11 +49,14 @@ struct ovn_netlink_notifier { > union ovn_notifier_msg_change { > struct route_table_msg route; > struct ne_table_msg neighbor; > + struct nh_table_msg nexthop; > }; > > static void ovn_netlink_route_change_handler(const void *change_, void *aux); > static void ovn_netlink_neighbor_change_handler(const void *change_, > void *aux); > +static void ovn_netlink_nexthop_change_handler(const void *change_, > + void *aux); > > static struct ovn_netlink_notifier notifiers[OVN_NL_NOTIFIER_MAX] = { > [OVN_NL_NOTIFIER_ROUTE_V4] = { > @@ -73,6 +77,12 @@ static struct ovn_netlink_notifier > notifiers[OVN_NL_NOTIFIER_MAX] = { > .change_handler = ovn_netlink_neighbor_change_handler, > .name = "neighbor", > }, > + [OVN_NL_NOTIFIER_NEXTHOP] = { > + .group = RTNLGRP_NEXTHOP, > + .msgs = VECTOR_EMPTY_INITIALIZER(struct nh_table_msg), > + .change_handler = ovn_netlink_nexthop_change_handler, > + .name = "nexthop", > + }, > }; > > static struct nln *nln_handle; > @@ -97,6 +107,11 @@ ovn_netlink_notifier_parse(struct ofpbuf *buf, void > *change_) > return ne_table_parse(buf, &change->neighbor); > } > > + if (nlmsg->nlmsg_type == RTM_NEWNEXTHOP || > + nlmsg->nlmsg_type == RTM_DELNEXTHOP) { > + return nh_table_parse(buf, &change->nexthop); > + } > + > return 0; > } > > @@ -136,6 +151,18 @@ ovn_netlink_neighbor_change_handler(const void *change_, > void *aux) > } > } > > +static void > +ovn_netlink_nexthop_change_handler(const void *change_, void *aux) > +{ > + if (!change_) { > + return; > + } > + > + struct ovn_netlink_notifier *notifier = aux; > + const union ovn_notifier_msg_change *change = change_; > + vector_push(¬ifier->msgs, &change->nexthop); > +} > + > static void > ovn_netlink_register_notifier(enum ovn_netlink_notifier_type type) > { > @@ -214,6 +241,22 @@ ovn_netlink_notifier_flush(enum > ovn_netlink_notifier_type type) > { > ovs_assert(type < OVN_NL_NOTIFIER_MAX); > struct ovn_netlink_notifier *notifier = ¬ifiers[type]; > + > + switch (type) { > + case OVN_NL_NOTIFIER_NEXTHOP: { > + struct nh_table_msg *msg; > + VECTOR_FOR_EACH_PTR (¬ifier->msgs, msg) { > + free(msg->nhe); > + } > + break; > + } > + case OVN_NL_NOTIFIER_ROUTE_V4: > + case OVN_NL_NOTIFIER_ROUTE_V6: > + case OVN_NL_NOTIFIER_NEIGHBOR: > + case OVN_NL_NOTIFIER_MAX: > + break; > + } > + > vector_clear(¬ifier->msgs); > } > > diff --git a/controller/ovn-netlink-notifier.h > b/controller/ovn-netlink-notifier.h > index b78fe466b..208a28d99 100644 > --- a/controller/ovn-netlink-notifier.h > +++ b/controller/ovn-netlink-notifier.h > @@ -24,6 +24,7 @@ enum ovn_netlink_notifier_type { > OVN_NL_NOTIFIER_ROUTE_V4, > OVN_NL_NOTIFIER_ROUTE_V6, > OVN_NL_NOTIFIER_NEIGHBOR, > + OVN_NL_NOTIFIER_NEXTHOP, > OVN_NL_NOTIFIER_MAX, > }; > > diff --git a/m4/ovn.m4 b/m4/ovn.m4 > index 8ccc1629e..93a959224 100644 > --- a/m4/ovn.m4 > +++ b/m4/ovn.m4 > @@ -585,3 +585,19 @@ AC_DEFUN([OVS_CHECK_LINUX_NETLINK], [ > [AC_DEFINE([HAVE_NLA_BITFIELD32], [1], > [Define to 1 if struct nla_bitfield32 is available.])]) > ]) > + > +dnl OVN_CHECK_LINUX_NEXTHOP_WEIGHT > +dnl > +dnl Configure Linux netlink nexthop compat. > +AC_DEFUN([OVN_CHECK_LINUX_NEXTHOP_WEIGHT], [ > + save_CFLAGS="$CFLAGS" > + CFLAGS="$CFLAGS -Werror=implicit-function-declaration" > + AC_COMPILE_IFELSE([ > + AC_LANG_PROGRAM([#include <linux/nexthop.h>], [ > + struct nexthop_grp grp = { 0 }; > + nexthop_grp_weight(&grp); > + ])], > + [AC_DEFINE([HAVE_NH_GRP_WEIGHT], [1], > + [Define to 1 if nexthop_grp_weight() is available.])]) > + CFLAGS="$save_CFLAGS" > + ]) > diff --git a/tests/automake.mk b/tests/automake.mk > index 75a4b00d7..46be217ae 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -303,6 +303,8 @@ tests_ovstest_SOURCES += \ > controller/host-if-monitor.h \ > controller/neighbor-exchange-netlink.c \ > controller/neighbor-exchange-netlink.h \ > + controller/nexthop-exchange.c \ > + controller/nexthop-exchange.h \ > controller/neighbor.c \ > controller/neighbor.h \ > controller/ovn-netlink-notifier.c \ > diff --git a/tests/system-ovn-netlink.at b/tests/system-ovn-netlink.at > index 26382f087..e5a663921 100644 > --- a/tests/system-ovn-netlink.at > +++ b/tests/system-ovn-netlink.at > @@ -537,3 +537,120 @@ AT_CHECK_UNQUOTED([ovstest test-ovn-netlink > route-table-notify \ > ]) > > AT_CLEANUP > + > +AT_SETUP([sync netlink nexthops - learn nexthops]) > +AT_KEYWORDS([netlink-nexthops]) > + > +check ip nexthop add id 1 via 192.168.1.1 fdb > +on_exit 'ip nexthop del id 1' > +check ip nexthop add id 2 via 192.168.1.2 fdb > +on_exit 'ip nexthop del id 2' > +check ip nexthop add id 5 via 192.168.1.5 fdb > +on_exit 'ip nexthop del id 5' > +check ip nexthop add id 10 group 1/2 fdb > +on_exit 'ip nexthop del id 10' > +check ip nexthop add id 11 group 1 fdb > +on_exit 'ip nexthop del id 11' > +check ip nexthop add id 3 group 5 fdb > +on_exit 'ip nexthop del id 3' > +check ip nexthop add id 12 group 1,100/2,200 fdb > +on_exit 'ip nexthop del id 12' Does this have a high chance of failing due to ids already existing? The nexthop table is for the whole default network namespace and the host running the CI might have the IDs in use already, right? I think we should also run this inside a separate netns like we do for "sync netlink nexthops - table notify". > + > +AT_CHECK([ovstest test-ovn-netlink nexthop-sync | sort], [0], [dnl > +Nexthop id=1, address=192.168.1.1 > +Nexthop id=10, group=[[1;192.168.1.1;1, 2;192.168.1.2;1]] > +Nexthop id=11, group=[[1;192.168.1.1;1]] > +Nexthop id=12, group=[[1;192.168.1.1;100, 2;192.168.1.2;200]] > +Nexthop id=2, address=192.168.1.2 > +Nexthop id=3, group=[[5;192.168.1.5;1]] > +Nexthop id=5, address=192.168.1.5 > +]) > + > +AT_CLEANUP > + > +AT_SETUP([sync netlink nexthops - table notify]) > +AT_KEYWORDS([netlink-nexthops]) > + > +ADD_NAMESPACES(nh) > +NS_EXEC([nh], [ip link set up lo]) > + > +dnl Should notify if an IPv4 nexthop is added. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop add id 1 via 10.0.0.1 fdb"], [0], [dnl > +Add nexthop id=1, address=10.0.0.1 > +]) > + > +dnl Should notify if an IPv6 nexthop is added. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop add id 2 via fd20::2 fdb"], [0], [dnl > +Add nexthop id=2, address=fd20::2 > +]) > + > +dnl Should notify if IPv4 nexthop group is added. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop add id 10 group 1 fdb"], [0], [dnl > +Add nexthop id=10, group=[[1;1]] > +]) > + > +dnl Should notify if IPv6 nexthop group is added. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop add id 11 group 2 fdb"], [0], [dnl > +Add nexthop id=11, group=[[2;1]] > +]) > + > +dnl Should notify if IPv4 nexthop group is removed. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop del id 10"], [0], [dnl > +Delete nexthop id=10, group=[[1;1]] > +]) > + > +dnl Should notify if IPv6 nexthop group is removed. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop del id 11"], [0], [dnl > +Delete nexthop id=11, group=[[2;1]] > +]) > + > +NS_EXEC([nh], [ip nexthop add id 3 via 10.0.0.3 fdb]) > +NS_EXEC([nh], [ip nexthop add id 10 group 1/3 fdb]) > +NS_EXEC([nh], [ip nexthop add id 4 via fd20::4 fdb]) > +NS_EXEC([nh], [ip nexthop add id 11 group 2/4 fdb]) > + > +dnl Should notify if an IPv4 nexthop group is removed and group modified. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop del id 3" | sort], [0], [dnl > +Add nexthop id=10, group=[[1;1]] > +Delete nexthop id=3, address=10.0.0.3 > +]) > + > +dnl Should notify if an IPv6 nexthop group is removed and group modified. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop del id 4" | sort], [0], [dnl > +Add nexthop id=11, group=[[2;1]] > +Delete nexthop id=4, address=fd20::4 > +]) > + > +dnl Should notify if an IPv4 nexthop group is removed and group modified. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop del id 1" | sort], [0], [dnl > +Delete nexthop id=1, address=10.0.0.1 > +Delete nexthop id=10, group=[[1;1]] > +]) > + > +dnl Should notify if an IPv6 nexthop group is removed and group modified. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop del id 2" | sort], [0], [dnl > +Delete nexthop id=11, group=[[2;1]] > +Delete nexthop id=2, address=fd20::2 > +]) > + > +dnl Should NOT notify if a blackhole nexthop is added. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop add id 1 blackhole"], [0], [dnl > +]) > + > +dnl Should NOT notify if non-FDB nexthop is added. > +NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ > + "ip nexthop add id 2 via 127.0.0.2 dev lo"], [0], [dnl > +]) > + > +AT_CLEANUP > diff --git a/tests/test-ovn-netlink.c b/tests/test-ovn-netlink.c > index 555b2df99..783e31340 100644 > --- a/tests/test-ovn-netlink.c > +++ b/tests/test-ovn-netlink.c > @@ -23,6 +23,7 @@ > > #include "controller/host-if-monitor.h" > #include "controller/neighbor-exchange-netlink.h" > +#include "controller/nexthop-exchange.h" > #include "controller/ovn-netlink-notifier.h" > #include "controller/neighbor.h" > #include "controller/route.h" > @@ -279,6 +280,59 @@ test_route_table_notify(struct ovs_cmdl_context *ctx) > ovn_netlink_notifiers_destroy(); > } > > +static void > +test_nexthop_sync(struct ovs_cmdl_context *ctx OVS_UNUSED) > +{ > + struct hmap nexthops = HMAP_INITIALIZER(&nexthops); > + struct ds ds = DS_EMPTY_INITIALIZER; > + > + nexthops_sync(&nexthops); > + > + struct nexthop_entry *nhe; > + HMAP_FOR_EACH (nhe, hmap_node, &nexthops) { > + ds_clear(&ds); > + nexthop_entry_format(&ds, nhe); > + printf("Nexthop %s\n", ds_cstr(&ds)); > + } > + > + HMAP_FOR_EACH_POP (nhe, hmap_node, &nexthops) { > + free(nhe); > + } > + > + ds_destroy(&ds); > + hmap_destroy(&nexthops); > +} > + > +static void > +test_nexthop_table_notify(struct ovs_cmdl_context *ctx) > +{ > + unsigned int shift = 1; > + > + const char *cmd = test_read_value(ctx, shift++, "shell_command"); > + if (!cmd) { > + return; > + } > + > + ovn_netlink_update_notifier(OVN_NL_NOTIFIER_NEXTHOP, true); > + run_command_under_notifier(cmd); > + > + struct vector *msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_NEXTHOP); > + Nit: no need for empty line, I think. > + struct ds ds = DS_EMPTY_INITIALIZER; > + > + struct nh_table_msg *msg; > + VECTOR_FOR_EACH_PTR (msgs, msg) { > + ds_clear(&ds); > + nexthop_entry_format(&ds, msg->nhe); > + printf("%s nexthop %s\n", > + msg->nlmsg_type == RTM_NEWNEXTHOP ? "Add" : "Delete", > + ds_cstr(&ds)); > + } > + > + ds_destroy(&ds); > + ovn_netlink_notifiers_destroy(); > +} > + > static void > test_ovn_netlink(int argc, char *argv[]) > { > @@ -291,6 +345,9 @@ test_ovn_netlink(int argc, char *argv[]) > {"route-sync", NULL, 1, INT_MAX, test_route_sync, OVS_RO}, > {"route-table-notify", NULL, 1, 1, > test_route_table_notify, OVS_RO}, > + {"nexthop-sync", NULL, 0, 0, test_nexthop_sync, OVS_RO}, > + {"nexthop-table-notify", NULL, 1, 1, > + test_nexthop_table_notify, OVS_RO}, > {NULL, NULL, 0, 0, NULL, OVS_RO}, > }; > struct ovs_cmdl_context ctx; With the small issues above addressed there's no need for a new revision in my opinion: Acked-by: Dumitru Ceara <[email protected]> Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
