The netlink notifiers used a lot of code that was more or less identical to each other. Extract the common code into separate module which allows the definition of listeners and their specific data. This should make it easier to add any new notifier, which will be the case in the future. It should also make it slightly easier to track individual updates and changes that could be processed incrementally instead of full recompute when there is any change.
Signed-off-by: Ales Musil <[email protected]> --- controller/automake.mk | 13 +- controller/neighbor-exchange.c | 4 +- controller/neighbor-exchange.h | 4 +- controller/neighbor-table-notify.c | 244 ----------------- controller/neighbor-table-notify.h | 45 ---- controller/ovn-controller.c | 169 ++++++++---- ...ify-stub.c => ovn-netlink-notifier-stub.c} | 35 ++- controller/ovn-netlink-notifier.c | 251 ++++++++++++++++++ controller/ovn-netlink-notifier.h | 38 +++ controller/route-exchange-netlink.h | 1 + controller/route-exchange.c | 4 +- controller/route-exchange.h | 2 +- controller/route-table-notify-stub.c | 55 ---- controller/route-table-notify.c | 238 ----------------- controller/route-table-notify.h | 44 --- tests/automake.mk | 4 +- tests/system-ovn-netlink.at | 59 ++-- tests/test-ovn-netlink.c | 55 ++-- 18 files changed, 502 insertions(+), 763 deletions(-) delete mode 100644 controller/neighbor-table-notify.c delete mode 100644 controller/neighbor-table-notify.h rename controller/{neighbor-table-notify-stub.c => ovn-netlink-notifier-stub.c} (51%) create mode 100644 controller/ovn-netlink-notifier.c create mode 100644 controller/ovn-netlink-notifier.h delete mode 100644 controller/route-table-notify-stub.c delete mode 100644 controller/route-table-notify.c delete mode 100644 controller/route-table-notify.h diff --git a/controller/automake.mk b/controller/automake.mk index d6809df10..c37b89b6c 100644 --- a/controller/automake.mk +++ b/controller/automake.mk @@ -32,6 +32,7 @@ controller_ovn_controller_SOURCES = \ controller/lport.h \ controller/ofctrl.c \ controller/ofctrl.h \ + controller/ovn-netlink-notifier.h \ controller/neighbor.c \ controller/neighbor.h \ controller/neighbor-of.c \ @@ -63,33 +64,29 @@ controller_ovn_controller_SOURCES = \ controller/ecmp-next-hop-monitor.h \ controller/ecmp-next-hop-monitor.c \ controller/route-exchange.h \ - controller/route-table-notify.h \ controller/route.h \ controller/route.c \ controller/garp_rarp.h \ controller/garp_rarp.c \ controller/neighbor-exchange.h \ - controller/neighbor-table-notify.h \ controller/host-if-monitor.h if HAVE_NETLINK controller_ovn_controller_SOURCES += \ controller/host-if-monitor.c \ + controller/ovn-netlink-notifier.c \ controller/neighbor-exchange-netlink.h \ controller/neighbor-exchange-netlink.c \ controller/neighbor-exchange.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 + controller/route-exchange.c else controller_ovn_controller_SOURCES += \ controller/host-if-monitor-stub.c \ + controller/ovn-netlink-notifier-stub.c \ controller/neighbor-exchange-stub.c \ - controller/neighbor-table-notify-stub.c \ - controller/route-exchange-stub.c \ - controller/route-table-notify-stub.c + controller/route-exchange-stub.c endif controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la diff --git a/controller/neighbor-exchange.c b/controller/neighbor-exchange.c index e40f39e24..47e757712 100644 --- a/controller/neighbor-exchange.c +++ b/controller/neighbor-exchange.c @@ -21,7 +21,6 @@ #include "neighbor.h" #include "neighbor-exchange.h" #include "neighbor-exchange-netlink.h" -#include "neighbor-table-notify.h" #include "openvswitch/poll-loop.h" #include "openvswitch/vlog.h" #include "ovn-util.h" @@ -136,8 +135,7 @@ neighbor_exchange_run(const struct neighbor_exchange_ctx_in *n_ctx_in, break; } - neighbor_table_add_watch_request(&n_ctx_out->neighbor_table_watches, - if_index, nim->if_name); + vector_push(n_ctx_out->neighbor_table_watches, &if_index); vector_destroy(&received_neighbors); } } diff --git a/controller/neighbor-exchange.h b/controller/neighbor-exchange.h index b4257f14c..32c87a8ab 100644 --- a/controller/neighbor-exchange.h +++ b/controller/neighbor-exchange.h @@ -30,8 +30,8 @@ struct neighbor_exchange_ctx_in { }; struct neighbor_exchange_ctx_out { - /* Contains struct neighbor_table_watch_request. */ - struct hmap neighbor_table_watches; + /* Contains int32_t representing if_index. */ + struct vector *neighbor_table_watches; /* Contains 'struct evpn_remote_vtep'. */ struct hmap *remote_vteps; /* Contains 'struct evpn_static_entry', remote FDB entries learned through diff --git a/controller/neighbor-table-notify.c b/controller/neighbor-table-notify.c deleted file mode 100644 index 04caa21df..000000000 --- a/controller/neighbor-table-notify.c +++ /dev/null @@ -1,244 +0,0 @@ -/* 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_DBG("Registering new neighbor table watcher " - "for interface %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, sizeof we->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_DBG("Removing neighbor table watcher for interface %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, 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 deleted file mode 100644 index 9f21271cc..000000000 --- a/controller/neighbor-table-notify.h +++ /dev/null @@ -1,45 +0,0 @@ -/* 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/ovn-controller.c b/controller/ovn-controller.c index 5b7eb3014..41abd3bae 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -53,6 +53,7 @@ #include "openvswitch/vlog.h" #include "ovn/actions.h" #include "ovn/features.h" +#include "ovn-netlink-notifier.h" #include "lib/chassis-index.h" #include "lib/extend-table.h" #include "lib/ip-mcast-index.h" @@ -92,12 +93,12 @@ #include "acl-ids.h" #include "route.h" #include "route-exchange.h" -#include "route-table-notify.h" +#include "route-table.h" #include "garp_rarp.h" #include "host-if-monitor.h" #include "neighbor.h" #include "neighbor-exchange.h" -#include "neighbor-table-notify.h" +#include "neighbor-exchange-netlink.h" #include "evpn-arp.h" #include "evpn-binding.h" #include "evpn-fdb.h" @@ -5610,6 +5611,30 @@ route_sb_datapath_binding_handler(struct engine_node *node, return EN_HANDLED_UNCHANGED; } +static int +table_id_cmp(const void *a_, const void *b_) +{ + const uint32_t *a = a_; + const uint32_t *b = b_; + + return *a < *b ? -1 : *a > *b; +} + +static void +route_table_notify_update(struct vector *watches) +{ + vector_qsort(watches, table_id_cmp); + + bool enabled = !vector_is_empty(watches); + ovn_netlink_update_notifier(OVN_NL_NOTIFIER_ROUTE_V4, enabled); + ovn_netlink_update_notifier(OVN_NL_NOTIFIER_ROUTE_V6, enabled); +} + +struct ed_type_route_table_notify { + /* Vector of ordered 'uint32_t' representing table_ids. */ + struct vector watches; +}; + struct ed_type_route_exchange { /* We need the idl to check if the Learned_Route table exists. */ struct ovsdb_idl *sb_idl; @@ -5635,6 +5660,8 @@ en_route_exchange_run(struct engine_node *node, void *data) struct ed_type_route *route_data = engine_get_input_data("route", node); + struct ed_type_route_table_notify *rt_notify = + engine_get_input_data("route_table_notify", node); /* There can not actually be any routes to advertise unless we also have * the Learned_Route table, since they where introduced in the same @@ -5643,6 +5670,8 @@ en_route_exchange_run(struct engine_node *node, void *data) return EN_STALE; } + vector_clear(&rt_notify->watches); + struct route_exchange_ctx_in r_ctx_in = { .ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn, .sbrec_learned_route_by_datapath = sbrec_learned_route_by_datapath, @@ -5651,15 +5680,11 @@ en_route_exchange_run(struct engine_node *node, void *data) }; struct route_exchange_ctx_out r_ctx_out = { .sb_changes_pending = false, + .route_table_watches = &rt_notify->watches, }; - hmap_init(&r_ctx_out.route_table_watches); - route_exchange_run(&r_ctx_in, &r_ctx_out); - route_table_notify_update_watches(&r_ctx_out.route_table_watches); - - route_table_watch_request_cleanup(&r_ctx_out.route_table_watches); - hmap_destroy(&r_ctx_out.route_table_watches); + route_table_notify_update(&rt_notify->watches); re->sb_changes_pending = r_ctx_out.sb_changes_pending; @@ -5693,23 +5718,40 @@ en_route_exchange_cleanup(void *data OVS_UNUSED) { } -struct ed_type_route_table_notify { - /* For incremental processing this could be tracked per datapath in - * the future. */ - bool changed; -}; - +/* The route_table_notify node is an input node, but the watches are + * populated by route_exchange node. The reason being that engine + * periodically runs input nodes to check if there are updates, so it + * could process the other nodes, however the route_table_notify cannot + * be dependent on other node because it wouldn't be input node anymore. */ static enum engine_node_state en_route_table_notify_run(struct engine_node *node OVS_UNUSED, void *data) { + enum engine_node_state state = EN_UNCHANGED; struct ed_type_route_table_notify *rtn = data; - enum engine_node_state state; - if (rtn->changed) { - state = EN_UPDATED; - } else { - state = EN_UNCHANGED; + struct vector *msgs; + uint32_t *table_id; + + msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V4); + VECTOR_FOR_EACH_PTR (msgs, table_id) { + if (vector_bsearch(&rtn->watches, table_id, table_id_cmp)) { + state = EN_UPDATED; + break; + } } - rtn->changed = false; + + if (state != EN_UPDATED) { + msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_ROUTE_V6); + VECTOR_FOR_EACH_PTR (msgs, table_id) { + if (vector_bsearch(&rtn->watches, table_id, table_id_cmp)) { + state = EN_UPDATED; + break; + } + } + } + + ovn_netlink_notifier_flush(OVN_NL_NOTIFIER_ROUTE_V4); + ovn_netlink_notifier_flush(OVN_NL_NOTIFIER_ROUTE_V6); + return state; } @@ -5718,14 +5760,19 @@ static void * en_route_table_notify_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) { - struct ed_type_route_table_notify *rtn = xzalloc(sizeof *rtn); - rtn->changed = true; + struct ed_type_route_table_notify *rtn = xmalloc(sizeof *rtn); + + *rtn = (struct ed_type_route_table_notify) { + .watches = VECTOR_EMPTY_INITIALIZER(uint32_t), + }; return rtn; } static void en_route_table_notify_cleanup(void *data OVS_UNUSED) { + struct ed_type_route_table_notify *rtn = data; + vector_destroy(&rtn->watches); } struct ed_type_route_exchange_status { @@ -6226,10 +6273,32 @@ neighbor_sb_port_binding_handler(struct engine_node *node, void *data) return EN_HANDLED_UNCHANGED; } +static int +if_index_cmp(const void *a_, const void *b_) +{ + const int32_t *a = a_; + const int32_t *b = b_; + + return *a < *b ? -1 : *a > *b; +} + +static void +neighbor_table_notify_update(struct vector *watches) +{ + vector_qsort(watches, if_index_cmp); + + bool enabled = !vector_is_empty(watches); + ovn_netlink_update_notifier(OVN_NL_NOTIFIER_NEIGHBOR, enabled); +} + +/* The neighbor_table_notify node is an input node, but the watches are + * populated by en_neighbor_exchange node. The reason being that engine + * periodically runs input nodes to check if there are updates, so it + * could process the other nodes, however the neighbor_table_notify cannot + * be dependent on other node because it wouldn't be input node anymore. */ struct ed_type_neighbor_table_notify { - /* For incremental processing this could be tracked per interface in - * the future. */ - bool changed; + /* Vector of ordered 'int32_t' representing if_indexes. */ + struct vector watches; }; static void * @@ -6239,7 +6308,7 @@ en_neighbor_table_notify_init(struct engine_node *node OVS_UNUSED, struct ed_type_neighbor_table_notify *ntn = xmalloc(sizeof *ntn); *ntn = (struct ed_type_neighbor_table_notify) { - .changed = true, + .watches = VECTOR_EMPTY_INITIALIZER(int32_t), }; return ntn; } @@ -6247,20 +6316,31 @@ en_neighbor_table_notify_init(struct engine_node *node OVS_UNUSED, static void en_neighbor_table_notify_cleanup(void *data OVS_UNUSED) { + struct ed_type_neighbor_table_notify *ntn = data; + vector_destroy(&ntn->watches); } static enum engine_node_state en_neighbor_table_notify_run(struct engine_node *node OVS_UNUSED, void *data) { + enum engine_node_state state = EN_UNCHANGED; struct ed_type_neighbor_table_notify *ntn = data; - enum engine_node_state state; - if (ntn->changed) { - state = EN_UPDATED; - } else { - state = EN_UNCHANGED; + struct vector *msgs; + struct ne_table_msg *ne_msg; + + msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_NEIGHBOR); + VECTOR_FOR_EACH_PTR (msgs, ne_msg) { + if (vector_bsearch(&ntn->watches, + &ne_msg->nd.if_index, + if_index_cmp)) { + state = EN_UPDATED; + break; + } } - ntn->changed = false; + + ovn_netlink_notifier_flush(OVN_NL_NOTIFIER_NEIGHBOR); + return state; } @@ -6307,27 +6387,26 @@ en_neighbor_exchange_run(struct engine_node *node, void *data_) struct ed_type_neighbor_exchange *data = data_; const struct ed_type_neighbor *neighbor_data = engine_get_input_data("neighbor", node); + struct ed_type_neighbor_table_notify *nt_notify = + engine_get_input_data("neighbor_table_notify", node); evpn_remote_vteps_clear(&data->remote_vteps); evpn_static_entries_clear(&data->static_fdbs); evpn_static_entries_clear(&data->static_arps); + vector_clear(&nt_notify->watches); struct neighbor_exchange_ctx_in n_ctx_in = { .monitored_interfaces = &neighbor_data->monitored_interfaces, }; struct neighbor_exchange_ctx_out n_ctx_out = { - .neighbor_table_watches = - HMAP_INITIALIZER(&n_ctx_out.neighbor_table_watches), + .neighbor_table_watches = &nt_notify->watches, .remote_vteps = &data->remote_vteps, .static_fdbs = &data->static_fdbs, .static_arps = &data->static_arps, }; neighbor_exchange_run(&n_ctx_in, &n_ctx_out); - neighbor_table_notify_update_watches(&n_ctx_out.neighbor_table_watches); - - neighbor_table_watch_request_cleanup(&n_ctx_out.neighbor_table_watches); - hmap_destroy(&n_ctx_out.neighbor_table_watches); + neighbor_table_notify_update(&nt_notify->watches); return EN_UPDATED; } @@ -7792,18 +7871,12 @@ main(int argc, char *argv[]) &transport_zones, bridge_table); - struct ed_type_route_table_notify *rtn = - engine_get_internal_data(&en_route_table_notify); - rtn->changed = route_table_notify_run(); + ovn_netlink_notifiers_run(); struct ed_type_host_if_monitor *hifm = engine_get_internal_data(&en_host_if_monitor); hifm->changed = host_if_monitor_run(); - struct ed_type_neighbor_table_notify *ntn = - engine_get_internal_data(&en_neighbor_table_notify); - ntn->changed = neighbor_table_notify_run(); - struct ed_type_route_exchange_status *rt_res = engine_get_internal_data(&en_route_exchange_status); rt_res->netlink_trigger_run = @@ -8131,9 +8204,8 @@ main(int argc, char *argv[]) } binding_wait(); - route_table_notify_wait(); host_if_monitor_wait(); - neighbor_table_notify_wait(); + ovn_netlink_notifiers_wait(); } unixctl_server_run(unixctl); @@ -8306,8 +8378,7 @@ loop_done: ovsrcu_exit(); dns_resolve_destroy(); route_exchange_destroy(); - route_table_notify_destroy(); - neighbor_table_notify_destroy(); + ovn_netlink_notifiers_destroy(); exit(retval); } diff --git a/controller/neighbor-table-notify-stub.c b/controller/ovn-netlink-notifier-stub.c similarity index 51% rename from controller/neighbor-table-notify-stub.c rename to controller/ovn-netlink-notifier-stub.c index bb4fe5991..a90aa6a4a 100644 --- a/controller/neighbor-table-notify-stub.c +++ b/controller/ovn-netlink-notifier-stub.c @@ -1,4 +1,5 @@ -/* Copyright (c) 2025, Red Hat, Inc. +/* Copyright (c) 2025, STACKIT GmbH & Co. KG + * 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. @@ -14,44 +15,42 @@ */ #include <config.h> - -#include <stdbool.h> +#include <stddef.h> #include "openvswitch/compiler.h" -#include "neighbor-table-notify.h" +#include "ovn-netlink-notifier.h" +#include "vec.h" + +static struct vector empty = VECTOR_EMPTY_INITIALIZER(uint8_t); -bool -neighbor_table_notify_run(void) +void +ovn_netlink_update_notifier(enum ovn_netlink_notifier_type type OVS_UNUSED, + bool enabled OVS_UNUSED) { - return false; } -void -neighbor_table_notify_wait(void) +struct vector * +ovn_netlink_get_msgs(enum ovn_netlink_notifier_type type OVS_UNUSED) { + return ∅ } 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) +ovn_netlink_notifier_flush(enum ovn_netlink_notifier_type type OVS_UNUSED) { } void -neighbor_table_watch_request_cleanup( - struct hmap *neighbor_table_watches OVS_UNUSED) +ovn_netlink_notifiers_run(void) { } void -neighbor_table_notify_update_watches( - const struct hmap *neighbor_table_watches OVS_UNUSED) +ovn_netlink_notifiers_wait(void) { } void -neighbor_table_notify_destroy(void) +ovn_netlink_notifiers_destroy(void) { } diff --git a/controller/ovn-netlink-notifier.c b/controller/ovn-netlink-notifier.c new file mode 100644 index 000000000..defa1cd54 --- /dev/null +++ b/controller/ovn-netlink-notifier.c @@ -0,0 +1,251 @@ +/* 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 <net/if.h> + +#include "neighbor-exchange-netlink.h" +#include "netlink-notifier.h" +#include "route-exchange-netlink.h" +#include "route-table.h" +#include "vec.h" + +#include "openvswitch/vlog.h" + +#include "ovn-netlink-notifier.h" + +VLOG_DEFINE_THIS_MODULE(ovn_netlink_notifier); + +#define NOTIFIER_MSGS_CAPACITY_THRESHOLD 1024 + +struct ovn_netlink_notifier { + /* Group for which we want to receive the notification. */ + int group; + /* The notifier pointers. */ + struct nln_notifier *nln_notifier; + /* Messages received by given notifier. */ + struct vector msgs; + /* Notifier change handler. */ + nln_notify_func *change_handler; + /* Name of the notifier. */ + const char *name; +}; + +union ovn_notifier_msg_change { + struct route_table_msg route; + struct ne_table_msg neighbor; +}; + +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 struct ovn_netlink_notifier notifiers[OVN_NL_NOTIFIER_MAX] = { + [OVN_NL_NOTIFIER_ROUTE_V4] = { + .group = RTNLGRP_IPV4_ROUTE, + .msgs = VECTOR_EMPTY_INITIALIZER(uint32_t), + .change_handler = ovn_netlink_route_change_handler, + .name = "route-ipv4", + }, + [OVN_NL_NOTIFIER_ROUTE_V6] = { + .group = RTNLGRP_IPV6_ROUTE, + .msgs = VECTOR_EMPTY_INITIALIZER(uint32_t), + .change_handler = ovn_netlink_route_change_handler, + .name = "route-ipv6", + }, + [OVN_NL_NOTIFIER_NEIGHBOR] = { + .group = RTNLGRP_NEIGH, + .msgs = VECTOR_EMPTY_INITIALIZER(struct ne_table_msg), + .change_handler = ovn_netlink_neighbor_change_handler, + .name = "neighbor", + }, +}; + +static struct nln *nln_handle; +static union ovn_notifier_msg_change nln_msg_change; + +static int +ovn_netlink_notifier_parse(struct ofpbuf *buf, void *change_) +{ + struct nlmsghdr *nlmsg = ofpbuf_at(buf, 0, NLMSG_HDRLEN); + if (!nlmsg) { + return 0; + } + + union ovn_notifier_msg_change *change = change_; + if (nlmsg->nlmsg_type == RTM_NEWROUTE || + nlmsg->nlmsg_type == RTM_DELROUTE) { + return route_table_parse(buf, &change->route); + } + + if (nlmsg->nlmsg_type == RTM_NEWNEIGH || + nlmsg->nlmsg_type == RTM_DELNEIGH) { + return ne_table_parse(buf, &change->neighbor); + } + + return 0; +} + +static void +ovn_netlink_route_change_handler(const void *change_, void *aux) +{ + if (!change_) { + return; + } + + struct ovn_netlink_notifier *notifier = aux; + union ovn_notifier_msg_change *change = + CONST_CAST(union ovn_notifier_msg_change *, change_); + + struct route_data *rd = &change->route.rd; + if (rd->rtm_protocol != RTPROT_OVN) { + /* We just cannot copy the whole route_data because it has reference + * to self for the nexthop list. */ + vector_push(¬ifier->msgs, &rd->rta_table_id); + } + + route_data_destroy(rd); +} + +static void +ovn_netlink_neighbor_change_handler(const void *change_, void *aux) +{ + if (!change_) { + return; + } + + struct ovn_netlink_notifier *notifier = aux; + const union ovn_notifier_msg_change *change = change_; + + if (!ne_is_ovn_owned(&change->neighbor.nd)) { + vector_push(¬ifier->msgs, &change->neighbor); + } +} + +static void +ovn_netlink_register_notifier(enum ovn_netlink_notifier_type type) +{ + ovs_assert(type < OVN_NL_NOTIFIER_MAX); + + struct ovn_netlink_notifier *notifier = ¬ifiers[type]; + if (notifier->nln_notifier) { + return; + } + + VLOG_INFO("Adding %s table watchers.", notifier->name); + if (!nln_handle) { + nln_handle = nln_create(NETLINK_ROUTE, ovn_netlink_notifier_parse, + &nln_msg_change); + ovs_assert(nln_handle); + } + + notifier->nln_notifier = nln_notifier_create(nln_handle, notifier->group, + notifier->change_handler, + notifier); + + if (!notifier->nln_notifier) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_WARN_RL(&rl, "Failed to create %s table watcher.", + notifier->name); + } +} + +static void +ovn_netlink_deregister_notifier(enum ovn_netlink_notifier_type type) +{ + ovs_assert(type < OVN_NL_NOTIFIER_MAX); + + struct ovn_netlink_notifier *notifier = ¬ifiers[type]; + if (!notifier->nln_notifier) { + return; + } + + VLOG_INFO("Removing %s table watchers.", notifier->name); + nln_notifier_destroy(notifier->nln_notifier); + notifier->nln_notifier = NULL; + + size_t i; + for (i = 0; i < OVN_NL_NOTIFIER_MAX; i++) { + if (notifiers[i].nln_notifier) { + break; + } + } + + if (i == OVN_NL_NOTIFIER_MAX) { + /* This was the last notifier, destroy the handle too. */ + nln_destroy(nln_handle); + nln_handle = NULL; + } +} + +void +ovn_netlink_update_notifier(enum ovn_netlink_notifier_type type, bool enabled) +{ + if (enabled) { + ovn_netlink_register_notifier(type); + } else { + ovn_netlink_deregister_notifier(type); + } +} + +struct vector * +ovn_netlink_get_msgs(enum ovn_netlink_notifier_type type) +{ + ovs_assert(type < OVN_NL_NOTIFIER_MAX); + return ¬ifiers[type].msgs; +} + +void +ovn_netlink_notifier_flush(enum ovn_netlink_notifier_type type) +{ + ovs_assert(type < OVN_NL_NOTIFIER_MAX); + struct ovn_netlink_notifier *notifier = ¬ifiers[type]; + vector_clear(¬ifier->msgs); +} + +void +ovn_netlink_notifiers_run(void) +{ + for (size_t i = 0; i < OVN_NL_NOTIFIER_MAX; i++) { + if (vector_capacity(¬ifiers[i].msgs) > + NOTIFIER_MSGS_CAPACITY_THRESHOLD) { + vector_shrink_to_fit(¬ifiers[i].msgs); + } + } + + if (nln_handle) { + nln_run(nln_handle); + } +} + +void +ovn_netlink_notifiers_wait(void) +{ + if (nln_handle) { + nln_wait(nln_handle); + } +} + +void +ovn_netlink_notifiers_destroy(void) +{ + for (size_t i = 0; i < OVN_NL_NOTIFIER_MAX; i++) { + ovn_netlink_notifier_flush(i); + ovn_netlink_deregister_notifier(i); + vector_destroy(¬ifiers[i].msgs); + } +} diff --git a/controller/ovn-netlink-notifier.h b/controller/ovn-netlink-notifier.h new file mode 100644 index 000000000..b78fe466b --- /dev/null +++ b/controller/ovn-netlink-notifier.h @@ -0,0 +1,38 @@ +/* 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 OVN_NETLINK_NOTIFIER_H +#define OVN_NETLINK_NOTIFIER_H 1 + +#include <stdbool.h> + +struct vector; + +enum ovn_netlink_notifier_type { + OVN_NL_NOTIFIER_ROUTE_V4, + OVN_NL_NOTIFIER_ROUTE_V6, + OVN_NL_NOTIFIER_NEIGHBOR, + OVN_NL_NOTIFIER_MAX, +}; + +void ovn_netlink_update_notifier(enum ovn_netlink_notifier_type type, + bool enabled); +struct vector *ovn_netlink_get_msgs(enum ovn_netlink_notifier_type type); +void ovn_netlink_notifier_flush(enum ovn_netlink_notifier_type type); +void ovn_netlink_notifiers_run(void); +void ovn_netlink_notifiers_wait(void); +void ovn_netlink_notifiers_destroy(void); + +#endif /* OVN_NETLINK_NOTIFIER_H */ diff --git a/controller/route-exchange-netlink.h b/controller/route-exchange-netlink.h index 3ebd4546f..8ba8a1039 100644 --- a/controller/route-exchange-netlink.h +++ b/controller/route-exchange-netlink.h @@ -39,6 +39,7 @@ struct in6_addr; struct hmap; struct vector; +struct advertise_route_entry; struct re_nl_received_route_node { const struct sbrec_datapath_binding *db; diff --git a/controller/route-exchange.c b/controller/route-exchange.c index ae44ffe69..82727f4e4 100644 --- a/controller/route-exchange.c +++ b/controller/route-exchange.c @@ -31,7 +31,6 @@ #include "ha-chassis.h" #include "local_data.h" #include "route.h" -#include "route-table-notify.h" #include "route-exchange.h" #include "route-exchange-netlink.h" @@ -306,8 +305,7 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, r_ctx_in->sbrec_learned_route_by_datapath, &r_ctx_out->sb_changes_pending); - route_table_add_watch_request(&r_ctx_out->route_table_watches, - table_id); + vector_push(r_ctx_out->route_table_watches, &table_id); vector_destroy(&received_routes); } diff --git a/controller/route-exchange.h b/controller/route-exchange.h index e3791c331..25db35568 100644 --- a/controller/route-exchange.h +++ b/controller/route-exchange.h @@ -30,7 +30,7 @@ struct route_exchange_ctx_in { }; struct route_exchange_ctx_out { - struct hmap route_table_watches; + struct vector *route_table_watches; bool sb_changes_pending; }; diff --git a/controller/route-table-notify-stub.c b/controller/route-table-notify-stub.c deleted file mode 100644 index 460c81dbe..000000000 --- a/controller/route-table-notify-stub.c +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (c) 2025, STACKIT GmbH & Co. KG - * - * 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 "route-table-notify.h" - -bool -route_table_notify_run(void) -{ - return false; -} - -void -route_table_notify_wait(void) -{ -} - -void -route_table_add_watch_request(struct hmap *route_table_watches OVS_UNUSED, - uint32_t table_id OVS_UNUSED) -{ -} - -void -route_table_watch_request_cleanup(struct hmap *route_table_watches OVS_UNUSED) -{ -} - -void -route_table_notify_update_watches( - const struct hmap *route_table_watches OVS_UNUSED) -{ -} - -void -route_table_notify_destroy(void) -{ -} diff --git a/controller/route-table-notify.c b/controller/route-table-notify.c deleted file mode 100644 index 9fa2e0ea6..000000000 --- a/controller/route-table-notify.c +++ /dev/null @@ -1,238 +0,0 @@ -/* - * Copyright (c) 2025, STACKIT GmbH & Co. KG - * - * 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 <net/if.h> -#include <linux/rtnetlink.h> - -#include "netlink-notifier.h" -#include "openvswitch/vlog.h" - -#include "binding.h" -#include "hash.h" -#include "hmapx.h" -#include "route-table.h" -#include "route.h" -#include "route-table-notify.h" -#include "route-exchange-netlink.h" - -VLOG_DEFINE_THIS_MODULE(route_table_notify); - -struct route_table_watch_request { - struct hmap_node node; - uint32_t table_id; -}; - -struct route_table_watch_entry { - struct hmap_node node; - uint32_t table_id; -}; - -static struct hmap watches = HMAP_INITIALIZER(&watches); -static bool any_route_table_changed; -static struct route_table_msg nln_rtmsg_change; - -static struct nln *nl_route_handle; -static struct nln_notifier *nl_route_notifier_v4; -static struct nln_notifier *nl_route_notifier_v6; - -static void route_table_change(const void *change_, void *aux); - -static void -route_table_register_notifiers(void) -{ - VLOG_INFO("Adding route table watchers."); - ovs_assert(!nl_route_handle); - - nl_route_handle = nln_create(NETLINK_ROUTE, route_table_parse, - &nln_rtmsg_change); - ovs_assert(nl_route_handle); - - nl_route_notifier_v4 = - nln_notifier_create(nl_route_handle, RTNLGRP_IPV4_ROUTE, - route_table_change, NULL); - if (!nl_route_notifier_v4) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_WARN_RL(&rl, "Failed to create ipv4 route table watcher."); - } - - nl_route_notifier_v6 = - nln_notifier_create(nl_route_handle, RTNLGRP_IPV6_ROUTE, - route_table_change, NULL); - if (!nl_route_notifier_v6) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_WARN_RL(&rl, "Failed to create ipv6 route table watcher."); - } -} - -static void -route_table_deregister_notifiers(void) -{ - VLOG_INFO("Removing route table watchers."); - ovs_assert(nl_route_handle); - - nln_notifier_destroy(nl_route_notifier_v4); - nln_notifier_destroy(nl_route_notifier_v6); - nln_destroy(nl_route_handle); - nl_route_notifier_v4 = NULL; - nl_route_notifier_v6 = NULL; - nl_route_handle = NULL; -} - -static uint32_t -route_table_notify_hash_watch(uint32_t table_id) -{ - return hash_int(table_id, 0); -} - -void -route_table_add_watch_request(struct hmap *route_table_watches, - uint32_t table_id) -{ - struct route_table_watch_request *wr = xzalloc(sizeof *wr); - wr->table_id = table_id; - hmap_insert(route_table_watches, &wr->node, - route_table_notify_hash_watch(wr->table_id)); -} - -void -route_table_watch_request_cleanup(struct hmap *route_table_watches) -{ - struct route_table_watch_request *wr; - HMAP_FOR_EACH_POP (wr, node, route_table_watches) { - free(wr); - } -} - -static struct route_table_watch_entry * -find_watch_entry(uint32_t table_id) -{ - struct route_table_watch_entry *we; - uint32_t hash = route_table_notify_hash_watch(table_id); - HMAP_FOR_EACH_WITH_HASH (we, node, hash, &watches) { - if (table_id == we->table_id) { - return we; - } - } - return NULL; -} - -static void -route_table_change(const void *change_, void *aux OVS_UNUSED) -{ - if (!change_) { - return; - } - - /* We currently track whether at least one recent route table change - * was detected. If that's the case already there's no need to - * continue. */ - struct route_table_msg *change = - CONST_CAST(struct route_table_msg *, change_); - if (!any_route_table_changed && change->rd.rtm_protocol != RTPROT_OVN) { - if (find_watch_entry(change->rd.rta_table_id)) { - any_route_table_changed = true; - } - } - - route_data_destroy(&change->rd); -} - -static void -add_watch_entry(uint32_t table_id) -{ - VLOG_INFO("Registering new route table watcher for table %d.", - table_id); - - struct route_table_watch_entry *we; - uint32_t hash = route_table_notify_hash_watch(table_id); - we = xzalloc(sizeof *we); - we->table_id = table_id; - hmap_insert(&watches, &we->node, hash); - - if (!nl_route_handle) { - route_table_register_notifiers(); - } -} - -static void -remove_watch_entry(struct route_table_watch_entry *we) -{ - VLOG_INFO("Removing route table watcher for table %d.", we->table_id); - hmap_remove(&watches, &we->node); - free(we); - - if (hmap_is_empty(&watches)) { - route_table_deregister_notifiers(); - } -} - -bool -route_table_notify_run(void) -{ - any_route_table_changed = false; - - if (nl_route_handle) { - nln_run(nl_route_handle); - } - - return any_route_table_changed; -} - -void -route_table_notify_wait(void) -{ - if (nl_route_handle) { - nln_wait(nl_route_handle); - } -} - -void -route_table_notify_update_watches(const struct hmap *route_table_watches) -{ - struct hmapx sync_watches = HMAPX_INITIALIZER(&sync_watches); - struct route_table_watch_entry *we; - HMAP_FOR_EACH (we, node, &watches) { - hmapx_add(&sync_watches, we); - } - - struct route_table_watch_request *wr; - HMAP_FOR_EACH (wr, node, route_table_watches) { - we = find_watch_entry(wr->table_id); - if (we) { - hmapx_find_and_delete(&sync_watches, we); - } else { - add_watch_entry(wr->table_id); - } - } - - struct hmapx_node *node; - HMAPX_FOR_EACH (node, &sync_watches) { - remove_watch_entry(node->data); - } - - hmapx_destroy(&sync_watches); -} - -void -route_table_notify_destroy(void) -{ - struct route_table_watch_entry *we; - HMAP_FOR_EACH_SAFE (we, node, &watches) { - remove_watch_entry(we); - } -} diff --git a/controller/route-table-notify.h b/controller/route-table-notify.h deleted file mode 100644 index a2bc05a49..000000000 --- a/controller/route-table-notify.h +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (c) 2025, STACKIT GmbH & Co. KG - * - * 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 ROUTE_TABLE_NOTIFY_H -#define ROUTE_TABLE_NOTIFY_H 1 - -#include <stdbool.h> -#include "openvswitch/hmap.h" - -/* Returns true if any route table has changed enough that we need - * to learn new routes. */ -bool route_table_notify_run(void); -void route_table_notify_wait(void); - -/* Add a watch request to the hmap. The hmap should later be passed to - * route_table_notify_update_watches*/ -void route_table_add_watch_request(struct hmap *route_table_watches, - uint32_t table_id); - -/* Cleanup all watch request in the provided hmap that where added using - * route_table_add_watch_request. */ -void route_table_watch_request_cleanup(struct hmap *route_table_watches); - -/* Updates the list of route table watches that are currently active. - * hmap should contain struct route_table_watch_request */ -void route_table_notify_update_watches(const struct hmap *route_table_watches); - -/* Cleans up all route table watches. */ -void route_table_notify_destroy(void); - -#endif /* ROUTE_TABLE_NOTIFY_H */ diff --git a/tests/automake.mk b/tests/automake.mk index 2dfc0bfa7..75a4b00d7 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -303,10 +303,10 @@ tests_ovstest_SOURCES += \ controller/host-if-monitor.h \ controller/neighbor-exchange-netlink.c \ controller/neighbor-exchange-netlink.h \ - controller/neighbor-table-notify.c \ - controller/neighbor-table-notify.h \ controller/neighbor.c \ controller/neighbor.h \ + controller/ovn-netlink-notifier.c \ + controller/ovn-netlink-notifier.h \ controller/route-exchange-netlink.c \ controller/route-exchange-netlink.h \ tests/test-ovn-netlink.c diff --git a/tests/system-ovn-netlink.at b/tests/system-ovn-netlink.at index 4e581aa74..8bf1055d1 100644 --- a/tests/system-ovn-netlink.at +++ b/tests/system-ovn-netlink.at @@ -229,6 +229,7 @@ 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 +br_if_index=$(netlink_if_index br-test) check ip link add lo-test type dummy on_exit 'ip link del lo-test' @@ -237,43 +238,47 @@ 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 +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink neighbor-table-notify \ + 'bridge fdb add 00:00:00:00:00:05 dev lo-test'], [0], [dnl +Add neighbor ifindex=$lo_if_index vlan=0 eth=00:00:00:00:00:05 dst=:: port=0 +]) + +dnl Should notify if an entry is removed from a bridge port monitored by OVN. +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink neighbor-table-notify \ + 'bridge fdb del 00:00:00:00:00:05 dev lo-test'], [0], [dnl +Delete neighbor ifindex=$lo_if_index vlan=0 eth=00:00:00:00:00:05 dst=:: port=0 +]) -dnl Should NOT notify if an entry is added to a bridge port that's not +dnl Should NOT notify if an static entry is added to a bridge port 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 +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink neighbor-table-notify \ + 'bridge fdb add 00:00:00:00:00:06 dev lo-test master static'], [0], [dnl +]) -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 \ +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink neighbor-table-notify \ 'ip neigh add 10.10.10.10 lladdr 00:00:00:00:10:00 \ - dev br-test extern_learn' \ - true + dev br-test extern_learn'], [0], [dnl +Add neighbor ifindex=$br_if_index vlan=0 eth=00:00:00:00:10:00 dst=10.10.10.10 port=0 +]) -dnl Should NOT notify if an entry is added to a bridge that's not monitored by +dnl Should notify if an entry is removed from a bridge that's monitored by dnl OVN. -check ovstest test-ovn-netlink neighbor-table-notify br-test $br_if_index \ +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink neighbor-table-notify \ + 'ip neigh del 10.10.10.10 lladdr 00:00:00:00:10:00 \ + dev br-test' | sort], [0], [dnl +Add neighbor ifindex=$br_if_index vlan=0 eth=00:00:00:00:00:00 dst=10.10.10.10 port=0 +Delete neighbor ifindex=$br_if_index vlan=0 eth=00:00:00:00:00:00 dst=10.10.10.10 port=0 +]) + +dnl Should NOT notify if an noarp entry is added to a bridge port +dnl monitored by OVN. +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink neighbor-table-notify \ 'ip neigh add 20.20.20.20 lladdr 00:00:00:00:20:00 \ - dev br-test-unused extern_learn' \ - false + dev br-test nud noarp'], [0], [dnl +]) AT_CLEANUP AT_SETUP([netlink - host-if-monitor]) diff --git a/tests/test-ovn-netlink.c b/tests/test-ovn-netlink.c index 6e9b46d04..efc3c9e5e 100644 --- a/tests/test-ovn-netlink.c +++ b/tests/test-ovn-netlink.c @@ -23,7 +23,7 @@ #include "controller/host-if-monitor.h" #include "controller/neighbor-exchange-netlink.h" -#include "controller/neighbor-table-notify.h" +#include "controller/ovn-netlink-notifier.h" #include "controller/neighbor.h" #include "controller/route.h" #include "controller/route-exchange-netlink.h" @@ -109,41 +109,48 @@ done: } static void -test_neighbor_table_notify(struct ovs_cmdl_context *ctx) +run_command_under_notifier(const char *cmd) { - unsigned int shift = 1; + ovn_netlink_notifiers_run(); + ovn_netlink_notifiers_wait(); - const char *if_name = test_read_value(ctx, shift++, "if_name"); - if (!if_name) { - return; + int rc = system(cmd); + if (rc) { + exit(rc); } - unsigned int if_index; - if (!test_read_uint_value(ctx, shift++, "if_index", &if_index)) { - return; - } + ovn_netlink_notifiers_run(); +} + +static void +test_neighbor_table_notify(struct ovs_cmdl_context *ctx) +{ + unsigned int shift = 1; 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); + ovn_netlink_update_notifier(OVN_NL_NOTIFIER_NEIGHBOR, true); + run_command_under_notifier(cmd); - neighbor_table_notify_run(); - neighbor_table_notify_wait(); + struct vector *msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_NEIGHBOR); - int rc = system(cmd); - if (rc) { - exit(rc); + struct ne_table_msg *msg; + VECTOR_FOR_EACH_PTR (msgs, msg) { + char addr_s[INET6_ADDRSTRLEN + 1]; + printf("%s neighbor ifindex=%"PRId32" vlan=%"PRIu16" " + "eth=" ETH_ADDR_FMT " dst=%s port=%"PRIu16"\n", + msg->nlmsg_type == RTM_NEWNEIGH ? "Add" : "Delete", + msg->nd.if_index, msg->nd.vlan, ETH_ADDR_ARGS(msg->nd.lladdr), + ipv6_string_mapped(addr_s, &msg->nd.addr) + ? addr_s + : "(invalid)", + msg->nd.port); } - ovs_assert(neighbor_table_notify_run() == expect_notify); - neighbor_table_watch_request_cleanup(&table_watches); + + ovn_netlink_notifiers_destroy(); } static void @@ -249,7 +256,7 @@ 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, + {"neighbor-table-notify", NULL, 1, 1, test_neighbor_table_notify, OVS_RO}, {"host-if-monitor", NULL, 2, 3, test_host_if_monitor, OVS_RO}, {"route-sync", NULL, 1, INT_MAX, test_route_sync, OVS_RO}, -- 2.53.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
