On Mon, Apr 13, 2026 at 12:25 PM Dumitru Ceara <[email protected]> wrote:
> 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! > Thank you Dumitru, > > > 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? > I used the hash instead of the id directly. There shouldn't by any duplicates though. > > > + } > > + 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 Yeah let's skip the group type for now. > > > > + [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 > > With the rest of the nits and comments addressed, applied to main. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
