On 8/12/25 2:59 PM, Xavier Simonart wrote: > Hi Ales, Dumitru > Hi Xavier,
> Thanks for the patch > One comment below > > Thanks > Xavier > > On Wed, Aug 6, 2025 at 8:27 PM Ales Musil via dev <ovs- > d...@openvswitch.org <mailto:ovs-dev@openvswitch.org>> wrote: > > From: Dumitru Ceara <dce...@redhat.com <mailto:dce...@redhat.com>> > > When monitoring and updating (Linux) neighbor tables through Netlink, > the interfaces in question are identified by if-index. It's more > CMS-friendly to allow OVN to be configured to track interfaces by name. > > In order to achieve that, we introduce a new host-if-monitor module > which tracks the if-index <-> if-name mapping for relevant interfaces. > > NOTE: OVS has the lib/if-notifier.[ch] module already. However, that is > not really useful for the OVN use case because it doesn't really pass > the struct rtnetlink_change message to its registered callbacks. > > Signed-off-by: Dumitru Ceara <dce...@redhat.com > <mailto:dce...@redhat.com>> > --- > controller/automake.mk <http://automake.mk> | 5 +- > controller/host-if-monitor-stub.c | 43 +++++++++ > controller/host-if-monitor.c | 150 ++++++++++++++++++++++++++++++ > controller/host-if-monitor.h | 30 ++++++ > 4 files changed, 227 insertions(+), 1 deletion(-) > create mode 100644 controller/host-if-monitor-stub.c > create mode 100644 controller/host-if-monitor.c > create mode 100644 controller/host-if-monitor.h > > diff --git a/controller/automake.mk <http://automake.mk> b/ > controller/automake.mk <http://automake.mk> > index 6af6ee2a9..6eca5e8ed 100644 > --- a/controller/automake.mk <http://automake.mk> > +++ b/controller/automake.mk <http://automake.mk> > @@ -64,10 +64,12 @@ controller_ovn_controller_SOURCES = \ > controller/route.c \ > controller/garp_rarp.h \ > controller/garp_rarp.c \ > - controller/neighbor-table-notify.h > + controller/neighbor-table-notify.h \ > + controller/host-if-monitor.h > > if HAVE_NETLINK > controller_ovn_controller_SOURCES += \ > + controller/host-if-monitor.c \ > controller/neighbor-exchange-netlink.h \ > controller/neighbor-exchange-netlink.c \ > controller/neighbor-table-notify.c \ > @@ -77,6 +79,7 @@ controller_ovn_controller_SOURCES += \ > controller/route-table-notify.c > else > controller_ovn_controller_SOURCES += \ > + controller/host-if-monitor-stub.c \ > controller/neighbor-table-notify-stub.c \ > controller/route-exchange-stub.c \ > controller/route-table-notify-stub.c > diff --git a/controller/host-if-monitor-stub.c b/controller/host-if- > monitor-stub.c > new file mode 100644 > index 000000000..b174cf996 > --- /dev/null > +++ b/controller/host-if-monitor-stub.c > @@ -0,0 +1,43 @@ > +/* 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 <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 "host-if-monitor.h" > + > +void > +host_if_monitor_wait(void) > +{ > +} > + > +bool > +host_if_monitor_run(void) > +{ > + return false; > +} > + > +void > +host_if_monitor_update_watches(const struct sset *if_names OVS_UNUSED) > +{ > +} > + > +int32_t > +host_if_monitor_ifname_toindex(const char *if_name OVS_UNUSED) > +{ > + return 0; > +} > diff --git a/controller/host-if-monitor.c b/controller/host-if-monitor.c > new file mode 100644 > index 000000000..26edb03d0 > --- /dev/null > +++ b/controller/host-if-monitor.c > @@ -0,0 +1,150 @@ > +/* 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 <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 "lib/rtnetlink.h" > +#include "lib/simap.h" > +#include "openvswitch/vlog.h" > + > +#include "host-if-monitor.h" > + > +VLOG_DEFINE_THIS_MODULE(host_if_monitor); > + > +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + > +struct host_if_monitor { > + struct nln_notifier *link_notifier; > + > + struct sset watched_interfaces; > + struct simap ifname_to_ifindex; > + > + bool changes_detected; > +}; > + > +static struct host_if_monitor monitor = (struct host_if_monitor) { > + .link_notifier = NULL, > + .watched_interfaces = > SSET_INITIALIZER(&monitor.watched_interfaces), > + .ifname_to_ifindex = SIMAP_INITIALIZER(&monitor.ifname_to_ifindex), > + .changes_detected = false, > +}; > + > +static void if_notifier_cb(const struct rtnetlink_change *, void *aux); > + > +void > +host_if_monitor_wait(void) > +{ > + rtnetlink_wait(); > +} > + > +bool > +host_if_monitor_run(void) > +{ > + monitor.changes_detected = false; > + > + /* If any relevant interface if-index <-> if-name mapping > changes are > + * dected, monitor.changes_detected will be updated accordingly > by the > + * if_notifier_cb(). */ > + rtnetlink_run(); > + > + return monitor.changes_detected; > +} > + > +void > +host_if_monitor_update_watches(const struct sset *if_names) > +{ > + struct sset new_if_names = SSET_INITIALIZER(&new_if_names); > + const char *if_name; > + > + /* The notifier only triggers the callback on interface updates. > + * For newly added ones we need to fetch the initial if_index > ourselves. > + */ > + SSET_FOR_EACH (if_name, if_names) { > + if (!sset_contains(&monitor.watched_interfaces, if_name)) { > + sset_add(&new_if_names, if_name); > + } > + } > + > + if (!sset_equals(&monitor.watched_interfaces, if_names)) { > + sset_destroy(&monitor.watched_interfaces); > + sset_clone(&monitor.watched_interfaces, if_names); > + } > > If/when interfaces are removed from monitor.watched_interfaces (e.g. > datapath becoming not local), > then monitor.ifname_to_ifindex will not be cleaned when interfaces are > deleted. > Good catch, yes, you're right. We'll address this in v3. Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
