On 8/12/25 4:56 PM, Ales Musil via dev wrote:
> From: Dumitru Ceara <dce...@redhat.com>
>
> For each (Linux) interface that's interesting for the OVN control plane,
> watch for neighbor (fdb/arp) changes. This allows us to reconcile on
> changed neighbor entries from outside routing agents (e.g., FRR).
>
> This is the neighbor equivalent of the support added for route tables in
> 673d90f1173f ("controller: Watch for route changes.").
>
> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
> ---
> V3:
> - renamed fdb_table_change() to neighbor_table_change()
> - added tests
>
> Changes in V2:
> - fixed up log messages
> - changed code to make OVN only manage neighbor entries with VLAN 0
> - added advertise_neigh_find() helper
> ---
> Makefile.am | 5 +-
> controller/automake.mk | 5 +-
> controller/neighbor-table-notify-stub.c | 57 ++++++
> controller/neighbor-table-notify.c | 244 ++++++++++++++++++++++++
> controller/neighbor-table-notify.h | 45 +++++
> controller/test-ovn-netlink.c | 41 ++++
> tests/automake.mk | 1 +
> tests/system-ovn-netlink.at | 55 ++++++
> 8 files changed, 451 insertions(+), 2 deletions(-)
> create mode 100644 controller/neighbor-table-notify-stub.c
> create mode 100644 controller/neighbor-table-notify.c
> create mode 100644 controller/neighbor-table-notify.h
>
> diff --git a/Makefile.am b/Makefile.am
> index ea98fb5fb..d8e24d0ab 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -350,13 +350,16 @@ check-tabs:
> fi
> .PHONY: check-tabs
>
> +# NOTE: test-ovn-netlink.c excluded due to use of system() to execute
> +# ip neigh / bridge fdb commands provided as arguments by test suite.
> ALL_LOCAL += thread-safety-check
> thread-safety-check:
> @cd $(srcdir); \
> if test -e .git && (git --version) >/dev/null 2>&1 && \
> grep -n -f build-aux/thread-safety-blacklist \
> `git ls-files | grep -v $(submodules) | grep '\.[ch]$$'` /dev/null \
> - | $(EGREP) -v ':[ ]*/?\*'; \
> + | $(EGREP) -v ':[ ]*/?\*' \
> + | $(EGREP) -v '^controller/test-ovn-netlink.c'; \
> then \
> echo "See above for list of calls to functions that are"; \
> echo "blacklisted due to thread safety issues"; \
> diff --git a/controller/automake.mk b/controller/automake.mk
> index 3eb45475c..6af6ee2a9 100644
> --- a/controller/automake.mk
> +++ b/controller/automake.mk
> @@ -63,18 +63,21 @@ controller_ovn_controller_SOURCES = \
> controller/route.h \
> controller/route.c \
> controller/garp_rarp.h \
> - controller/garp_rarp.c
> + controller/garp_rarp.c \
> + controller/neighbor-table-notify.h
>
> if HAVE_NETLINK
> controller_ovn_controller_SOURCES += \
> controller/neighbor-exchange-netlink.h \
> controller/neighbor-exchange-netlink.c \
> + controller/neighbor-table-notify.c \
> controller/route-exchange-netlink.h \
> controller/route-exchange-netlink.c \
> controller/route-exchange.c \
> controller/route-table-notify.c
> else
> controller_ovn_controller_SOURCES += \
> + controller/neighbor-table-notify-stub.c \
> controller/route-exchange-stub.c \
> controller/route-table-notify-stub.c
> endif
> diff --git a/controller/neighbor-table-notify-stub.c
> b/controller/neighbor-table-notify-stub.c
> new file mode 100644
> index 000000000..bb4fe5991
> --- /dev/null
> +++ b/controller/neighbor-table-notify-stub.c
> @@ -0,0 +1,57 @@
> +/* Copyright (c) 2025, 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 <stdbool.h>
> +
> +#include "openvswitch/compiler.h"
> +#include "neighbor-table-notify.h"
> +
> +bool
> +neighbor_table_notify_run(void)
> +{
> + return false;
> +}
> +
> +void
> +neighbor_table_notify_wait(void)
> +{
> +}
> +
> +void
> +neighbor_table_add_watch_request(
> + struct hmap *neighbor_table_watches OVS_UNUSED,
> + int32_t if_index OVS_UNUSED,
> + const char *if_name OVS_UNUSED)
> +{
> +}
> +
> +void
> +neighbor_table_watch_request_cleanup(
> + struct hmap *neighbor_table_watches OVS_UNUSED)
> +{
> +}
> +
> +void
> +neighbor_table_notify_update_watches(
> + const struct hmap *neighbor_table_watches OVS_UNUSED)
> +{
> +}
> +
> +void
> +neighbor_table_notify_destroy(void)
> +{
> +}
> diff --git a/controller/neighbor-table-notify.c
> b/controller/neighbor-table-notify.c
> new file mode 100644
> index 000000000..dd0b320c4
> --- /dev/null
> +++ b/controller/neighbor-table-notify.c
> @@ -0,0 +1,244 @@
> +/* Copyright (c) 2025, 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 <net/if.h>
> +
> +#include "hash.h"
> +#include "hmapx.h"
> +#include "lib/util.h"
> +#include "netlink-notifier.h"
> +#include "openvswitch/vlog.h"
> +
> +#include "neighbor-exchange-netlink.h"
> +#include "neighbor-table-notify.h"
> +
> +VLOG_DEFINE_THIS_MODULE(neighbor_table_notify);
> +
> +struct neighbor_table_watch_request {
> + struct hmap_node node;
> + int32_t if_index;
> + char if_name[IFNAMSIZ + 1];
> +};
> +
> +struct neighbor_table_watch_entry {
> + struct hmap_node node;
> + int32_t if_index;
> + char if_name[IFNAMSIZ + 1];
> +};
> +
> +static struct hmap watches = HMAP_INITIALIZER(&watches);
> +static bool any_neighbor_table_changed;
> +static struct ne_table_msg nln_nmsg_change;
> +
> +static struct nln *nl_neighbor_handle;
> +static struct nln_notifier *nl_neighbor_notifier;
> +
> +static void neighbor_table_change(const void *change_, void *aux);
> +
> +static void
> +neighbor_table_register_notifiers(void)
> +{
> + VLOG_INFO("Adding neighbor table watchers.");
> + ovs_assert(!nl_neighbor_handle);
> +
> + nl_neighbor_handle = nln_create(NETLINK_ROUTE, ne_table_parse,
> + &nln_nmsg_change);
> + ovs_assert(nl_neighbor_handle);
> +
> + nl_neighbor_notifier =
> + nln_notifier_create(nl_neighbor_handle, RTNLGRP_NEIGH,
> + neighbor_table_change, NULL);
> + if (!nl_neighbor_notifier) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> + VLOG_WARN_RL(&rl, "Failed to create neighbor table watcher.");
> + }
> +}
> +
> +static void
> +neighbor_table_deregister_notifiers(void)
> +{
> + VLOG_INFO("Removing neighbor table watchers.");
> + ovs_assert(nl_neighbor_handle);
> +
> + nln_notifier_destroy(nl_neighbor_notifier);
> + nln_destroy(nl_neighbor_handle);
> + nl_neighbor_notifier = NULL;
> + nl_neighbor_handle = NULL;
> +}
> +
> +static uint32_t
> +neighbor_table_notify_hash_watch(int32_t if_index)
> +{
> + /* To allow lookups triggered by netlink messages, don't include the
> + * if_name in the hash. The netlink updates only include if_index. */
> + return hash_int(if_index, 0);
> +}
> +
> +static void
> +add_watch_entry(int32_t if_index, const char *if_name)
> +{
> + VLOG_INFO("Registering new neighbor table watcher "
> + "for if_index %s (%"PRId32").",
> + if_name, if_index);
> +
> + struct neighbor_table_watch_entry *we;
> + uint32_t hash = neighbor_table_notify_hash_watch(if_index);
> + we = xzalloc(sizeof *we);
> + we->if_index = if_index;
> + ovs_strzcpy(we->if_name, if_name, IFNAMSIZ + 1);
nit: sizeof wr->if_name.
> + hmap_insert(&watches, &we->node, hash);
> +
> + if (!nl_neighbor_handle) {
> + neighbor_table_register_notifiers();
> + }
> +}
> +
> +static void
> +remove_watch_entry(struct neighbor_table_watch_entry *we)
> +{
> + VLOG_INFO("Removing neighbor table watcher for table %s (%"PRId32").",
"for table" ?
Also, should these be debug logs, maybe?
> + we->if_name, we->if_index);
> + hmap_remove(&watches, &we->node);
> + free(we);
> +
> + if (hmap_is_empty(&watches)) {
> + neighbor_table_deregister_notifiers();
> + }
> +}
> +
> +bool
> +neighbor_table_notify_run(void)
> +{
> + any_neighbor_table_changed = false;
> +
> + if (nl_neighbor_handle) {
> + nln_run(nl_neighbor_handle);
> + }
> +
> + return any_neighbor_table_changed;
> +}
> +
> +void
> +neighbor_table_notify_wait(void)
> +{
> + if (nl_neighbor_handle) {
> + nln_wait(nl_neighbor_handle);
> + }
> +}
> +
> +void
> +neighbor_table_add_watch_request(struct hmap *neighbor_table_watches,
> + int32_t if_index, const char *if_name)
> +{
> + struct neighbor_table_watch_request *wr = xzalloc(sizeof *wr);
> +
> + wr->if_index = if_index;
> + ovs_strzcpy(wr->if_name, if_name, IFNAMSIZ + 1);
nit: sizeof wr->if_name.
> + hmap_insert(neighbor_table_watches, &wr->node,
> + neighbor_table_notify_hash_watch(wr->if_index));
> +}
> +
> +void
> +neighbor_table_watch_request_cleanup(struct hmap *neighbor_table_watches)
> +{
> + struct neighbor_table_watch_request *wr;
> + HMAP_FOR_EACH_POP (wr, node, neighbor_table_watches) {
> + free(wr);
> + }
> +}
> +
> +static struct neighbor_table_watch_entry *
> +find_watch_entry(int32_t if_index, const char *if_name)
> +{
> + struct neighbor_table_watch_entry *we;
> + uint32_t hash = neighbor_table_notify_hash_watch(if_index);
> + HMAP_FOR_EACH_WITH_HASH (we, node, hash, &watches) {
> + if (if_index == we->if_index && !strcmp(if_name, we->if_name)) {
> + return we;
> + }
> + }
> + return NULL;
> +}
> +
> +static struct neighbor_table_watch_entry *
> +find_watch_entry_by_if_index(int32_t if_index)
> +{
> + struct neighbor_table_watch_entry *we;
> + uint32_t hash = neighbor_table_notify_hash_watch(if_index);
> + HMAP_FOR_EACH_WITH_HASH (we, node, hash, &watches) {
> + if (if_index == we->if_index) {
> + return we;
> + }
> + }
> + return NULL;
> +}
> +
> +void
> +neighbor_table_notify_update_watches(const struct hmap
> *neighbor_table_watches)
> +{
> + struct hmapx sync_watches = HMAPX_INITIALIZER(&sync_watches);
> + struct neighbor_table_watch_entry *we;
> + HMAP_FOR_EACH (we, node, &watches) {
> + hmapx_add(&sync_watches, we);
> + }
> +
> + struct neighbor_table_watch_request *wr;
> + HMAP_FOR_EACH (wr, node, neighbor_table_watches) {
> + we = find_watch_entry(wr->if_index, wr->if_name);
> + if (we) {
> + hmapx_find_and_delete(&sync_watches, we);
> + } else {
> + add_watch_entry(wr->if_index, wr->if_name);
> + }
> + }
> +
> + struct hmapx_node *node;
> + HMAPX_FOR_EACH (node, &sync_watches) {
> + remove_watch_entry(node->data);
> + }
> +
> + hmapx_destroy(&sync_watches);
> +}
> +
> +void
> +neighbor_table_notify_destroy(void)
> +{
> + struct neighbor_table_watch_entry *we;
> + HMAP_FOR_EACH_SAFE (we, node, &watches) {
> + remove_watch_entry(we);
> + }
> +}
> +
> +static void
> +neighbor_table_change(const void *change_, void *aux OVS_UNUSED)
> +{
> + /* We currently track whether at least one recent neighbor table change
> + * was detected. If that's the case already there's no need to
> + * continue. */
> + if (any_neighbor_table_changed) {
> + return;
> + }
> +
> + const struct ne_table_msg *change = change_;
> +
> + if (change && !ne_is_ovn_owned(&change->nd)) {
> + if (find_watch_entry_by_if_index(change->nd.if_index)) {
> + any_neighbor_table_changed = true;
> + }
> + }
> +}
> diff --git a/controller/neighbor-table-notify.h
> b/controller/neighbor-table-notify.h
> new file mode 100644
> index 000000000..9f21271cc
> --- /dev/null
> +++ b/controller/neighbor-table-notify.h
> @@ -0,0 +1,45 @@
> +/* Copyright (c) 2025, 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 NEIGHBOR_TABLE_NOTIFY_H
> +#define NEIGHBOR_TABLE_NOTIFY_H 1
> +
> +#include <stdbool.h>
> +#include "openvswitch/hmap.h"
> +
> +/* Returns true if any neighbor table has changed enough that we need
> + * to learn new neighbor entries. */
> +bool neighbor_table_notify_run(void);
> +void neighbor_table_notify_wait(void);
> +
> +/* Add a watch request to the hmap. The hmap should later be passed to
> + * neighbor_table_notify_update_watches*/
> +void neighbor_table_add_watch_request(struct hmap *neighbor_table_watches,
> + int32_t if_index, const char *if_name);
> +
> +/* Cleanup all watch request in the provided hmap that where added using
> + * neighbor_table_add_watch_request. */
> +void neighbor_table_watch_request_cleanup(
> + struct hmap *neighbor_table_watches);
> +
> +/* Updates the list of neighbor table watches that are currently active.
> + * hmap should contain struct neighbor_table_watch_request */
> +void neighbor_table_notify_update_watches(
> + const struct hmap *neighbor_table_watches);
> +
> +/* Cleans up all neighbor table watches. */
> +void neighbor_table_notify_destroy(void);
> +
> +#endif /* NEIGHBOR_TABLE_NOTIFY_H */
> diff --git a/controller/test-ovn-netlink.c b/controller/test-ovn-netlink.c
> index 4134d9f0a..a26e2d478 100644
> --- a/controller/test-ovn-netlink.c
> +++ b/controller/test-ovn-netlink.c
> @@ -21,6 +21,7 @@
> #include "tests/test-utils.h"
>
> #include "neighbor-exchange-netlink.h"
> +#include "neighbor-table-notify.h"
> #include "neighbor.h"
>
> static void
> @@ -103,12 +104,52 @@ done:
> vector_destroy(&received_neighbors);
> }
>
> +static void
> +test_neighbor_table_notify(struct ovs_cmdl_context *ctx)
> +{
> + unsigned int shift = 1;
> +
> + const char *if_name = test_read_value(ctx, shift++, "if_name");
> + if (!if_name) {
> + return;
> + }
> +
> + unsigned int if_index;
> + if (!test_read_uint_value(ctx, shift++, "if_index", &if_index)) {
> + return;
> + }
> +
> + const char *cmd = test_read_value(ctx, shift++, "shell_command");
> + if (!cmd) {
> + return;
> + }
> +
> + const char *notify = test_read_value(ctx, shift++, "should_notify");
> + bool expect_notify = notify && !strcmp(notify, "true");
> +
> + struct hmap table_watches = HMAP_INITIALIZER(&table_watches);
> + neighbor_table_add_watch_request(&table_watches, if_index, if_name);
> + neighbor_table_notify_update_watches(&table_watches);
> +
> + neighbor_table_notify_run();
> + neighbor_table_notify_wait();
> +
> + int rc = system(cmd);
> + if (rc) {
> + exit(rc);
> + }
> + ovs_assert(neighbor_table_notify_run() == expect_notify);
> + neighbor_table_watch_request_cleanup(&table_watches);
> +}
> +
> static void
> test_ovn_netlink(int argc, char *argv[])
> {
> set_program_name(argv[0]);
> static const struct ovs_cmdl_command commands[] = {
> {"neighbor-sync", NULL, 2, INT_MAX, test_neighbor_sync, OVS_RO},
> + {"neighbor-table-notify", NULL, 3, 4,
> + test_neighbor_table_notify, OVS_RO},
> {NULL, NULL, 0, 0, NULL, OVS_RO},
> };
> struct ovs_cmdl_context ctx;
> diff --git a/tests/automake.mk b/tests/automake.mk
> index b2db67e99..ba6b6d7ba 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -320,6 +320,7 @@ if HAVE_NETLINK
> tests_ovstest_LDADD += \
> controller/neighbor.$(OBJEXT) \
> controller/neighbor-exchange-netlink.$(OBJEXT) \
> + controller/neighbor-table-notify.$(OBJEXT) \
> controller/route-exchange-netlink.$(OBJEXT) \
> controller/test-ovn-netlink.$(OBJEXT)
> endif
> diff --git a/tests/system-ovn-netlink.at b/tests/system-ovn-netlink.at
> index 6a21c0e56..068dfdc63 100644
> --- a/tests/system-ovn-netlink.at
> +++ b/tests/system-ovn-netlink.at
> @@ -176,3 +176,58 @@ OVN_NEIGH_EQUAL([br-test], [nud noarp], [20.20.20], [dnl
> OVN_NEIGH_V6_EQUAL([br-test], [nud noarp], [20::], [dnl
> 20::20 lladdr 00:00:00:00:20:00 NOARP])
> AT_CLEANUP
> +
> +AT_SETUP([sync netlink neighbors - table notify])
> +AT_KEYWORDS([netlink-neighbors])
> +
> +check ip link add br-test type bridge
> +on_exit 'ip link del br-test'
> +check ip link set br-test address 00:00:00:00:00:01
> +check ip address add dev br-test 10.10.10.1/24
> +check ip link set dev br-test up
> +
> +check ip link add lo-test type dummy
> +on_exit 'ip link del lo-test'
> +check ip link set lo-test master br-test
> +check ip link set lo-test address 00:00:00:00:00:02
> +check ip link set dev lo-test up
> +lo_if_index=$(netlink_if_index lo-test)
> +
> +check ip link add br-test-unused type bridge
> +on_exit 'ip link del br-test-unused'
> +check ip link set br-test-unused address 00:00:00:00:00:03
> +check ip address add dev br-test-unused 20.20.20.1/24
> +check ip link set dev br-test-unused up
> +
> +check ip link add lo-test-unused type dummy
> +on_exit 'ip link del lo-test-unused'
> +check ip link set lo-test-unused master br-test-unused
> +check ip link set lo-test-unused address 00:00:00:00:00:04
> +check ip link set dev lo-test-unused up
> +
> +dnl Should notify if an entry is added to a bridge port monitored by OVN.
> +check ovstest test-ovn-netlink neighbor-table-notify lo-test $lo_if_index \
> + 'bridge fdb add 00:00:00:00:00:05 dev lo-test' \
> + true
> +
> +dnl Should NOT notify if an entry is added to a bridge port that's not
> +dnl monitored by OVN.
> +check ovstest test-ovn-netlink neighbor-table-notify lo-test $lo_if_index \
> + 'bridge fdb add 00:00:00:00:00:05 dev lo-test-unused' \
> + false
> +
> +br_if_index=$(netlink_if_index br-test)
> +dnl Should notify if an entry is added to a bridge that's monitored by
> +dnl OVN.
> +check ovstest test-ovn-netlink neighbor-table-notify br-test $br_if_index \
> + 'ip neigh add 10.10.10.10 lladdr 00:00:00:00:10:00 \
> + dev br-test extern_learn' \
> + true
> +
> +dnl Should NOT notify if an entry is added to a bridge that's not monitored
> by
> +dnl OVN.
> +check ovstest test-ovn-netlink neighbor-table-notify br-test $br_if_index \
> + 'ip neigh add 20.20.20.20 lladdr 00:00:00:00:20:00 \
> + dev br-test-unused extern_learn' \
> + false
> +AT_CLEANUP
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
