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);
+ 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").",
+ 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);
+ 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
--
2.50.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
