The default values didn't allow dual-stack deployments. Moreover, they made very specific assumptions and would likely be overridden by the CMS in all deployments.
Instead make them mandatory if dynamic-routing-vni is configured for an EVPN enabled logical switch. Also: - made sure dynamic-routing-*-ifname are ignored if VNI not set (this is what our docs said) - moved the man page description of the options in the right place, in a group listing all EVPN options Reported-at: https://issues.redhat.com/browse/FDP-3136 Signed-off-by: Dumitru Ceara <[email protected]> --- NOTE: As discussed on-list [0] this patch targets 26.03 (main branch before hard freeze). [0] https://mail.openvswitch.org/pipermail/ovs-dev/2026-February/430230.html --- controller/neighbor.c | 67 ++++++++++++----------- northd/en-datapath-logical-switch.c | 85 +++++++++++++++++------------ ovn-nb.xml | 62 +++++++++++++-------- tests/multinode.at | 14 +++-- tests/ovn.at | 82 ++++++++++++++++++++++++++++ tests/system-common-macros.at | 9 +-- 6 files changed, 217 insertions(+), 102 deletions(-) diff --git a/controller/neighbor.c b/controller/neighbor.c index eae5eadb24..57df1e90c9 100644 --- a/controller/neighbor.c +++ b/controller/neighbor.c @@ -28,12 +28,6 @@ VLOG_DEFINE_THIS_MODULE(neighbor); -static const char *neighbor_interface_prefixes[] = { - [NEIGH_IFACE_BRIDGE] = "br-", - [NEIGH_IFACE_VXLAN] = "vxlan-", - [NEIGH_IFACE_LOOPBACK] = "lo-", -}; - static const char *neighbor_opt_name[] = { [NEIGH_IFACE_BRIDGE] = "dynamic-routing-bridge-ifname", [NEIGH_IFACE_VXLAN] = "dynamic-routing-vxlan-ifname", @@ -87,17 +81,16 @@ advertise_neigh_find(const struct hmap *neighbors, struct eth_addr mac, static void neigh_parse_device_name(struct sset *device_names, struct local_datapath *ld, - enum neighbor_interface_type type, uint32_t vni) + enum neighbor_interface_type type) { const char *names = smap_get_def(&ld->datapath->external_ids, neighbor_opt_name[type], ""); 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); + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_WARN_RL(&rl, "Datapath "UUID_FMT" misses %s", + UUID_ARGS(&ld->datapath->header_.uuid), + neighbor_opt_name[type]); } } @@ -123,7 +116,7 @@ neighbor_run(struct neighbor_ctx_in *n_ctx_in, } struct sset device_names; - neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_VXLAN, vni); + neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_VXLAN); const char *name; SSET_FOR_EACH (name, &device_names) { struct neighbor_interface_monitor *vxlan = @@ -133,49 +126,57 @@ neighbor_run(struct neighbor_ctx_in *n_ctx_in, } sset_destroy(&device_names); - neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_LOOPBACK, vni); + struct neighbor_interface_monitor *lo = NULL; + neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_LOOPBACK); if (sset_count(&device_names) > 1) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); VLOG_WARN_RL(&rl, "Datapath "UUID_FMT" too many names provided " "for loopback device", UUID_ARGS(&ld->datapath->header_.uuid)); } - struct neighbor_interface_monitor *lo = - neighbor_interface_monitor_alloc(NEIGH_AF_BRIDGE, - NEIGH_IFACE_LOOPBACK, vni, - SSET_FIRST(&device_names)); - vector_push(n_ctx_out->monitored_interfaces, &lo); + + if (!sset_is_empty(&device_names)) { + lo = neighbor_interface_monitor_alloc(NEIGH_AF_BRIDGE, + NEIGH_IFACE_LOOPBACK, vni, + SSET_FIRST(&device_names)); + vector_push(n_ctx_out->monitored_interfaces, &lo); + } sset_destroy(&device_names); - neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_BRIDGE, vni); + struct neighbor_interface_monitor *br_v4 = NULL; + struct neighbor_interface_monitor *br_v6 = NULL; + neigh_parse_device_name(&device_names, ld, NEIGH_IFACE_BRIDGE); if (sset_count(&device_names) > 1) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); VLOG_WARN_RL(&rl, "Datapath "UUID_FMT" too many names provided " "for bridge device", UUID_ARGS(&ld->datapath->header_.uuid)); } - struct neighbor_interface_monitor *br_v4 = - 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(NEIGH_AF_INET6, - NEIGH_IFACE_BRIDGE, vni, - SSET_FIRST(&device_names)); - vector_push(n_ctx_out->monitored_interfaces, &br_v6); + + if (!sset_is_empty(&device_names)) { + br_v4 = + neighbor_interface_monitor_alloc(NEIGH_AF_INET, + NEIGH_IFACE_BRIDGE, vni, + SSET_FIRST(&device_names)); + vector_push(n_ctx_out->monitored_interfaces, &br_v4); + + br_v6 = + 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); - if (nrm_mode_FDB_is_set(mode)) { + if (nrm_mode_FDB_is_set(mode) && lo) { neighbor_collect_mac_to_advertise(n_ctx_in, &lo->announced_neighbors, n_ctx_out->advertised_pbs, ld->datapath); } - if (nrm_mode_IP_is_set(mode)) { + if (nrm_mode_IP_is_set(mode) && br_v4 && br_v6) { neighbor_collect_ip_mac_to_advertise(n_ctx_in, &br_v4->announced_neighbors, &br_v6->announced_neighbors, diff --git a/northd/en-datapath-logical-switch.c b/northd/en-datapath-logical-switch.c index d37dfa2bd1..ef4c95d28c 100644 --- a/northd/en-datapath-logical-switch.c +++ b/northd/en-datapath-logical-switch.c @@ -59,6 +59,20 @@ get_requested_tunnel_key(const struct nbrec_logical_switch *nbs, return requested_tunnel_key; } +static const char * +get_dynamic_device_name(const struct nbrec_logical_switch *nbs, + const char *dynamic_routing_option) +{ + const char *ifname = smap_get(&nbs->other_config, dynamic_routing_option); + + if (!ifname) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_WARN_RL(&rl, "Missing %s for datapath %s", + dynamic_routing_option, nbs->name); + } + return ifname; +} + static bool check_dynamic_device_name(const char *dynamic_routing_option, const char *if_name) @@ -125,46 +139,47 @@ gather_external_ids(const struct nbrec_logical_switch *nbs, const char *vni = smap_get(&nbs->other_config, "dynamic-routing-vni"); if (vni) { smap_add(external_ids, "dynamic-routing-vni", vni); - } - const char *bridge_ifname = smap_get(&nbs->other_config, - "dynamic-routing-bridge-ifname"); - if (bridge_ifname && - check_dynamic_device_name("dynamic-routing-bridge-ifname", - bridge_ifname)) { - smap_add(external_ids, "dynamic-routing-bridge-ifname", - bridge_ifname); - } + const char *bridge_ifname = + get_dynamic_device_name(nbs, "dynamic-routing-bridge-ifname"); + if (bridge_ifname && + check_dynamic_device_name("dynamic-routing-bridge-ifname", + bridge_ifname)) { + smap_add(external_ids, "dynamic-routing-bridge-ifname", + bridge_ifname); + } - const char *vxlan_ifnames = smap_get(&nbs->other_config, - "dynamic-routing-vxlan-ifname"); - if (vxlan_ifnames && - check_dynamic_device_names("dynamic-routing-vxlan-ifname", - vxlan_ifnames)) { - smap_add(external_ids, "dynamic-routing-vxlan-ifname", - vxlan_ifnames); - } + const char *vxlan_ifnames = + get_dynamic_device_name(nbs, "dynamic-routing-vxlan-ifname"); + if (vxlan_ifnames && + check_dynamic_device_names("dynamic-routing-vxlan-ifname", + vxlan_ifnames)) { + smap_add(external_ids, "dynamic-routing-vxlan-ifname", + vxlan_ifnames); + } - const char *adv_ifname = smap_get(&nbs->other_config, - "dynamic-routing-advertise-ifname"); - if (adv_ifname && - check_dynamic_device_name("dynamic-routing-advertise-ifname", - adv_ifname)) { - smap_add(external_ids, "dynamic-routing-advertise-ifname", - adv_ifname); - } + const char *adv_ifname = + get_dynamic_device_name(nbs, "dynamic-routing-advertise-ifname"); + if (adv_ifname && + check_dynamic_device_name("dynamic-routing-advertise-ifname", + adv_ifname)) { + smap_add(external_ids, "dynamic-routing-advertise-ifname", + adv_ifname); + } - const char *redistribute = - smap_get(&nbs->other_config, "dynamic-routing-redistribute"); - if (redistribute) { - smap_add(external_ids, "dynamic-routing-redistribute", redistribute); - } + const char *redistribute = + smap_get(&nbs->other_config, "dynamic-routing-redistribute"); + if (redistribute) { + smap_add(external_ids, "dynamic-routing-redistribute", + redistribute); + } - const char *prefer_evpn_arp_local = - smap_get(&nbs->other_config, "dynamic-routing-arp-prefer-local"); - if (prefer_evpn_arp_local) { - smap_add(external_ids, "dynamic-routing-arp-prefer-local", - prefer_evpn_arp_local); + const char *prefer_evpn_arp_local = + smap_get(&nbs->other_config, "dynamic-routing-arp-prefer-local"); + if (prefer_evpn_arp_local) { + smap_add(external_ids, "dynamic-routing-arp-prefer-local", + prefer_evpn_arp_local); + } } /* For backwards-compatibility, also store the NB UUID in diff --git a/ovn-nb.xml b/ovn-nb.xml index ae37d03d15..d4cef12e04 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -795,27 +795,6 @@ <column name="external_ids" key="neutron:network_name"> Another name for the logical switch. </column> - - <column name="other_config" key="dynamic-routing-bridge-ifname"> - Set the interface name for the bridge used for EVPN integration. - The default name is <code>br-$vni</code>. - Only relevant if <ref column="other_config" key="dynamic-routing-vni" - table="Logical_switch"/> is set to valid VNI. - </column> - - <column name="other_config" key="dynamic-routing-vxlan-ifname"> - 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> - - <column name="other_config" key="dynamic-routing-advertise-ifname"> - Set the interface name for the advertise device used for EVPN - integration. The default name is <code>lo-$vni</code>. - Only relevant if <ref column="other_config" key="dynamic-routing-vni" - table="Logical_switch"/> is set to valid VNI. - </column> </group> <group title="IP Address Assignment"> @@ -919,6 +898,16 @@ dropped, even if use_ct_inv_match is set to true. Default: <code>false</code>. </column> + </group> + + <group title="EVPN Options"> + <p> + These options control the EVPN configuration of the logical switch. + To enable EVPN integration set + <ref column="other_config" key="dynamic-routing-vni"/> to a valid + VNI number to represent the EVPN L2 domain extended by the + logical switch. + </p> <column name="other_config" key="dynamic-routing-vni" type='{"type": "integer", "minInteger": 0, @@ -929,9 +918,14 @@ </p> <p> - The ovn-controller expects three interfaces to exist within the - BGP vrf: <code>br-$vni</code>, <code>lo-$vni</code> and - <code>vxlan-$vni</code>. + All of the following configuration options must also be provided + in order for the configuration to be valid: + <ref column="other_config" key="dynamic-routing-bridge-ifname" + table="Logical_switch"/>, + <ref column="other_config" key="dynamic-routing-vxlan-ifname" + table="Logical_switch"/>, + <ref column="other_config" key="dynamic-routing-advertise-ifname" + table="Logical_switch"/>, </p> <p> @@ -940,6 +934,26 @@ </p> </column> + <column name="other_config" key="dynamic-routing-bridge-ifname"> + Set the interface name for the bridge used for EVPN integration. + Only relevant if <ref column="other_config" key="dynamic-routing-vni" + table="Logical_switch"/> is set to valid VNI. + </column> + + <column name="other_config" key="dynamic-routing-vxlan-ifname"> + List of interface names used for the vxlan devices used for EVPN + integration. + Only relevant if <ref column="other_config" key="dynamic-routing-vni" + table="Logical_switch"/> is set to valid VNI. + </column> + + <column name="other_config" key="dynamic-routing-advertise-ifname"> + Set the interface name for the advertise device used for EVPN + integration. + Only relevant if <ref column="other_config" key="dynamic-routing-vni" + table="Logical_switch"/> is set to valid VNI. + </column> + <column name="other_config" key="dynamic-routing-fdb-prefer-local" type='{"type": "boolean"}'> <p> diff --git a/tests/multinode.at b/tests/multinode.at index b5331af247..e02bd6f074 100644 --- a/tests/multinode.at +++ b/tests/multinode.at @@ -3515,8 +3515,11 @@ m_wait_for_ports_up # Enable EVPN support for the distributed logical switch and redistribute # local FDBs. -check multinode_nbctl set logical_switch ls \ - other_config:dynamic-routing-vni=$vni \ +check multinode_nbctl set logical_switch ls \ + other_config:dynamic-routing-vni=$vni \ + other_config:dynamic-routing-bridge-ifname=br-$vni \ + other_config:dynamic-routing-vxlan-ifname=vxlan-$vni \ + other_config:dynamic-routing-advertise-ifname=lo-$vni \ other_config:dynamic-routing-redistribute=fdb,ip check multinode_nbctl --wait=hv sync dp_key=$(m_fetch_column Datapath_Binding tunnel_key external_ids:name=ls) @@ -4336,8 +4339,11 @@ check m_as ovn-gw-2 /data/create_fake_vm.sh w2 w2 \ m_wait_for_ports_up # Enable EVPN support for the distributed logical switch. -check multinode_nbctl set logical_switch ls \ - other_config:dynamic-routing-vni=$vni \ +check multinode_nbctl set logical_switch ls \ + other_config:dynamic-routing-vni=$vni \ + other_config:dynamic-routing-bridge-ifname=br-$vni \ + other_config:dynamic-routing-vxlan-ifname=vxlan-$vni \ + other_config:dynamic-routing-advertise-ifname=lo-$vni \ other_config:dynamic-routing-redistribute=fdb,ip check multinode_nbctl --wait=hv sync diff --git a/tests/ovn.at b/tests/ovn.at index 3a3b05ab7f..17a59eaebc 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -45700,3 +45700,85 @@ AT_CHECK([grep -q 'name="eth-unknown"' hv1/ovn-controller.log], [1]) OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([EVPN ifname configurations]) +ovn_start + +net_add n1 +sim_add hv +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +as northd +check ovn-appctl -t ovn-northd vlog/set jsonrpc:info +check ovn-appctl -t ovn-northd vlog/disable-rate-limit +as hv +check ovn-appctl -t ovn-controller vlog/disable-rate-limit + +clear_logs() { + as northd + check ovn-appctl -t ovn-northd vlog/close + check rm northd/ovn-northd.log + check ovn-appctl -t ovn-northd vlog/reopen + + as hv + check ovn-appctl -t ovn-controller vlog/close + check rm hv/ovn-controller.log + check ovn-appctl -t ovn-controller vlog/reopen +} + +check ovn-nbctl ls-add ls -- lsp-add ls lsp +check ovs-vsctl add-port br-int lsp -- \ + set Interface lsp external_ids:iface-id=lsp + +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +AS_BOX([Missing bridge ifname]) +clear_logs +check ovn-nbctl --wait=hv \ + -- set logical_switch ls other_config:dynamic-routing-vni=42 \ + -- set logical_switch ls other_config:dynamic-routing-vxlan-ifname=foo \ + -- set logical_switch ls other_config:dynamic-routing-advertise-ifname=bar + +OVS_WAIT_UNTIL([grep -q "WARN.*dynamic-routing-bridge-ifname" northd/ovn-northd.log]) +OVS_WAIT_UNTIL([grep -q "WARN.*dynamic-routing-bridge-ifname" hv/ovn-controller.log]) +check ovn-nbctl --wait=hv clear logical_switch ls other_config + +AS_BOX([Missing vxlan ifname]) +clear_logs +check ovn-nbctl --wait=hv \ + -- set logical_switch ls other_config:dynamic-routing-vni=42 \ + -- set logical_switch ls other_config:dynamic-routing-bridge-ifname=foo \ + -- set logical_switch ls other_config:dynamic-routing-advertise-ifname=bar + +OVS_WAIT_UNTIL([grep -q "WARN.*dynamic-routing-vxlan-ifname" northd/ovn-northd.log]) +OVS_WAIT_UNTIL([grep -q "WARN.*dynamic-routing-vxlan-ifname" hv/ovn-controller.log]) +check ovn-nbctl --wait=hv clear logical_switch ls other_config + +AS_BOX([Missing advertise ifname]) +clear_logs +check ovn-nbctl --wait=hv \ + -- set logical_switch ls other_config:dynamic-routing-vni=42 \ + -- set logical_switch ls other_config:dynamic-routing-bridge-ifname=foo \ + -- set logical_switch ls other_config:dynamic-routing-vxlan-ifname=bar + +OVS_WAIT_UNTIL([grep -q "WARN.*dynamic-routing-advertise-ifname" northd/ovn-northd.log]) +OVS_WAIT_UNTIL([grep -q "WARN.*dynamic-routing-advertise-ifname" hv/ovn-controller.log]) +check ovn-nbctl --wait=hv clear logical_switch ls other_config + +AS_BOX([Valid config]) +clear_logs +check ovn-nbctl --wait=hv \ + -- set logical_switch ls other_config:dynamic-routing-vni=42 \ + -- set logical_switch ls other_config:dynamic-routing-bridge-ifname=foo \ + -- set logical_switch ls other_config:dynamic-routing-vxlan-ifname=bar \ + -- set logical_switch ls other_config:dynamic-routing-advertise-ifname=xyz + +AT_CHECK([grep -q "WARN.*dynamic-routing" northd/ovn-northd.log], [1]) +AT_CHECK([grep -q "WARN.*dynamic-routing" hv/ovn-controller.log], [1]) + +OVN_CLEANUP([hv]) +AT_CLEANUP +]) diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 0f4f8952c6..5062f02b0e 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -213,12 +213,9 @@ m4_define([SET_EVPN_IFACE_NAMES], [[ $ifname = "default" ]] && VXLAN_V6_NAME=vxlan-v6-$vni || VXLAN_V6_NAME=vxlan-v6-$ifname [[ $ifname = "default" ]] && LO_NAME=lo-$vni || LO_NAME=lo-$ifname - if [[ $ifname != "default" ]]; then - check ovn-nbctl set logical_switch $switch \ - other_config:dynamic-routing-bridge-ifname=$BR_NAME \ - other_config:dynamic-routing-advertise-ifname=$LO_NAME - fi - check ovn-nbctl set logical_switch $switch \ + check ovn-nbctl set logical_switch $switch \ + other_config:dynamic-routing-bridge-ifname=$BR_NAME \ + other_config:dynamic-routing-advertise-ifname=$LO_NAME \ other_config:dynamic-routing-vxlan-ifname=$VXLAN_NAME","$VXLAN_V6_NAME export BR_NAME VXLAN_NAME VXLAN_V6_NAME LO_NAME -- 2.52.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
