Hi Lorenzo, thanks for the patch. Should this patch have: Reported-at: https://issues.redhat.com/browse/FDP-2730
? Aside from that, Acked-by: Mark Michelson <[email protected]> On Wed, Dec 17, 2025 at 8:40 AM Lorenzo Bianconi via dev <[email protected]> wrote: > > We need to be able to monitor two vxlan interfaces (one for IPv4 and one > for IPv6) for dual stack EVPN logical switches since we can't configure a > single vxlan interface with multiple local IPs (one for v4 one for v6). > Fix the probelm allowing the CMS to specify a list of vxlan device names > using the following syntax: > > dynamic-routing-vxlan-ifname=vxlan-if1,vxlan-if2,..,vxlan-ifn > > Fixes: 36737306806e ("Add the capability to specify EVPN device names.") > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > controller/neighbor.c | 78 ++++++++++++++++++++--------- > northd/en-datapath-logical-switch.c | 57 ++++++++++----------- > ovn-nb.xml | 4 +- > 3 files changed, 85 insertions(+), 54 deletions(-) > > diff --git a/controller/neighbor.c b/controller/neighbor.c > index 075e672fa..6022cec82 100644 > --- a/controller/neighbor.c > +++ b/controller/neighbor.c > @@ -21,10 +21,13 @@ > #include "local_data.h" > #include "lport.h" > #include "openvswitch/ofp-parse.h" > +#include "openvswitch/vlog.h" > #include "ovn-sb-idl.h" > > #include "neighbor.h" > > +VLOG_DEFINE_THIS_MODULE(neighbor); > + > static const char *neighbor_interface_prefixes[] = { > [NEIGH_IFACE_BRIDGE] = "br-", > [NEIGH_IFACE_VXLAN] = "vxlan-", > @@ -43,10 +46,9 @@ static bool neighbor_interface_with_vni_exists( > struct vector *monitored_interfaces, > uint32_t vni); > static struct neighbor_interface_monitor * > -neighbor_interface_monitor_alloc(struct local_datapath *ld, > - enum neighbor_family family, > +neighbor_interface_monitor_alloc(enum neighbor_family family, > enum neighbor_interface_type type, > - uint32_t vni); > + uint32_t vni, const char *if_name); > static void neighbor_collect_mac_to_advertise( > const struct neighbor_ctx_in *, struct hmap *neighbors, > struct sset *advertised_pbs, const struct sbrec_datapath_binding *); > @@ -83,6 +85,23 @@ advertise_neigh_find(const struct hmap *neighbors, struct > eth_addr mac, > return NULL; > } > > +static void > +neigh_parse_device_name(struct sset *device_names, struct local_datapath *ld, > + enum neighbor_interface_type type, uint32_t vni) > +{ > + const char *names = smap_get_def(&ld->datapath->external_ids, > + neighbor_opt_name[type], ""); > + sset_clear(device_names); > + sset_from_delimited_string(device_names, names, ","); > + if (sset_is_empty(device_names)) { > + /* Default device name if not specified. */ > + char if_name[IFNAMSIZ + 1]; > + snprintf(if_name, sizeof if_name, "%s%"PRIu32, > + neighbor_interface_prefixes[type], vni); > + sset_add(device_names, if_name); > + } > +} > + > void > neighbor_run(struct neighbor_ctx_in *n_ctx_in, > struct neighbor_ctx_out *n_ctx_out) > @@ -104,25 +123,44 @@ neighbor_run(struct neighbor_ctx_in *n_ctx_in, > continue; > } > > - struct neighbor_interface_monitor *vxlan = > - neighbor_interface_monitor_alloc(ld, NEIGH_AF_BRIDGE, > - NEIGH_IFACE_VXLAN, vni); > - vector_push(n_ctx_out->monitored_interfaces, &vxlan); > + struct sset device_names = SSET_INITIALIZER(&device_names); > + neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_VXLAN, vni); > + const char *name; > + SSET_FOR_EACH (name, &device_names) { > + struct neighbor_interface_monitor *vxlan = > + neighbor_interface_monitor_alloc(NEIGH_AF_BRIDGE, > + NEIGH_IFACE_VXLAN, vni, > name); > + vector_push(n_ctx_out->monitored_interfaces, &vxlan); > + } > > + neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_LOOPBACK, > vni); > + if (sset_count(&device_names) > 1) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > + VLOG_WARN_RL(&rl, "too many names provided for loopback device"); > + } > struct neighbor_interface_monitor *lo = > - neighbor_interface_monitor_alloc(ld, NEIGH_AF_BRIDGE, > - NEIGH_IFACE_LOOPBACK, vni); > + neighbor_interface_monitor_alloc(NEIGH_AF_BRIDGE, > + NEIGH_IFACE_LOOPBACK, vni, > + SSET_FIRST(&device_names)); > vector_push(n_ctx_out->monitored_interfaces, &lo); > > + neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_BRIDGE, vni); > + if (sset_count(&device_names) > 1) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > + VLOG_WARN_RL(&rl, "too many names provided for bridge device"); > + } > struct neighbor_interface_monitor *br_v4 = > - neighbor_interface_monitor_alloc(ld, NEIGH_AF_INET, > - NEIGH_IFACE_BRIDGE, vni); > + neighbor_interface_monitor_alloc(NEIGH_AF_INET, > + NEIGH_IFACE_BRIDGE, vni, > + SSET_FIRST(&device_names)); > vector_push(n_ctx_out->monitored_interfaces, &br_v4); > > struct neighbor_interface_monitor *br_v6 = > - neighbor_interface_monitor_alloc(ld, NEIGH_AF_INET6, > - NEIGH_IFACE_BRIDGE, vni); > + neighbor_interface_monitor_alloc(NEIGH_AF_INET6, > + NEIGH_IFACE_BRIDGE, vni, > + SSET_FIRST(&device_names)); > vector_push(n_ctx_out->monitored_interfaces, &br_v6); > + sset_destroy(&device_names); > > enum neigh_redistribute_mode mode = > parse_neigh_dynamic_redistribute(&ld->datapath->external_ids); > @@ -200,10 +238,9 @@ neighbor_interface_with_vni_exists(struct vector > *monitored_interfaces, > } > > static struct neighbor_interface_monitor * > -neighbor_interface_monitor_alloc(struct local_datapath *ld, > - enum neighbor_family family, > +neighbor_interface_monitor_alloc(enum neighbor_family family, > enum neighbor_interface_type type, > - uint32_t vni) > + uint32_t vni, const char *if_name) > { > struct neighbor_interface_monitor *nim = xmalloc(sizeof *nim); > *nim = (struct neighbor_interface_monitor) { > @@ -212,15 +249,8 @@ neighbor_interface_monitor_alloc(struct local_datapath > *ld, > .type = type, > .vni = vni, > }; > + snprintf(nim->if_name, sizeof nim->if_name, "%s", if_name); > > - const char *if_name = smap_get(&ld->datapath->external_ids, > - neighbor_opt_name[type]); > - if (if_name) { > - snprintf(nim->if_name, sizeof nim->if_name, "%s", if_name); > - } else { > - snprintf(nim->if_name, sizeof nim->if_name, "%s%"PRIu32, > - neighbor_interface_prefixes[type], vni); > - } > return nim; > } > > diff --git a/northd/en-datapath-logical-switch.c > b/northd/en-datapath-logical-switch.c > index ed0385a48..0ad5ad261 100644 > --- a/northd/en-datapath-logical-switch.c > +++ b/northd/en-datapath-logical-switch.c > @@ -58,6 +58,26 @@ get_requested_tunnel_key(const struct nbrec_logical_switch > *nbs, > return requested_tunnel_key; > } > > +static bool > +check_dynamic_device_name(const char *if_names) > +{ > + struct sset device_names; > + sset_from_delimited_string(&device_names, if_names, ","); > + > + bool ret = true; > + const char *name; > + SSET_FOR_EACH (name, &device_names) { > + if (strlen(name) > IFNAMSIZ) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > + VLOG_WARN_RL(&rl, "dynamic-routing-ifname %s is too long", name); > + ret = false; > + break; > + } > + } > + sset_destroy(&device_names); > + return ret; > +} > + > static void > gather_external_ids(const struct nbrec_logical_switch *nbs, > struct smap *external_ids) > @@ -95,42 +115,23 @@ gather_external_ids(const struct nbrec_logical_switch > *nbs, > > const char *bridge_ifname = smap_get(&nbs->other_config, > "dynamic-routing-bridge-ifname"); > - if (bridge_ifname) { > - if (strlen(bridge_ifname) > IFNAMSIZ) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > - VLOG_WARN_RL(&rl, "dynamic-routing-bridge-ifname %s is too long", > - bridge_ifname); > - } else { > - smap_add(external_ids, "dynamic-routing-bridge-ifname", > - bridge_ifname); > - } > + if (bridge_ifname && check_dynamic_device_name(bridge_ifname)) { > + smap_add(external_ids, "dynamic-routing-bridge-ifname", > + bridge_ifname); > } > > const char *vxlan_ifname = smap_get(&nbs->other_config, > "dynamic-routing-vxlan-ifname"); > - if (vxlan_ifname) { > - if (strlen(vxlan_ifname) > IFNAMSIZ) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > - VLOG_WARN_RL(&rl, "dynamic-routing-vxlan-ifname %s is too long", > - vxlan_ifname); > - } else { > - smap_add(external_ids, "dynamic-routing-vxlan-ifname", > - vxlan_ifname); > - } > + if (vxlan_ifname && check_dynamic_device_name(vxlan_ifname)) { > + smap_add(external_ids, "dynamic-routing-vxlan-ifname", > + vxlan_ifname); > } > > const char *adv_ifname = smap_get(&nbs->other_config, > "dynamic-routing-advertise-ifname"); > - if (adv_ifname) { > - if (strlen(adv_ifname) > IFNAMSIZ) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > - VLOG_WARN_RL(&rl, > - "dynamic-routing-advertise-ifname %s is too long", > - adv_ifname); > - } else { > - smap_add(external_ids, "dynamic-routing-advertise-ifname", > - adv_ifname); > - } > + if (adv_ifname && check_dynamic_device_name(adv_ifname)) { > + smap_add(external_ids, "dynamic-routing-advertise-ifname", > + adv_ifname); > } > > const char *redistribute = > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 19ccfc7ba..a1edd8d35 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -804,8 +804,8 @@ > </column> > > <column name="other_config" key="dynamic-routing-vxlan-ifname"> > - Set the interface name for the vxlan device used for EVPN integration. > - The default name is <code>vxlan-$vni</code>. > + List of interface names used for the vxlan devices used for EVPN > + integration. The default name is <code>vxlan-$vni</code>. > Only relevant if <ref column="other_config" key="dynamic-routing-vni" > table="Logical_switch"/> is set to valid VNI. > </column> > -- > 2.52.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
