From 38d0c2f12b6c08b3ccadbc4f0d25d5624dd8f2f1 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 24 Nov 2020 13:56:12 +0100 Subject: [PATCH 1/2] ovs: rename variable @interface_is_internal is a bad name. The variable indicates whether the interface is the local interface. (cherry picked from commit e9e99b867780acfbd8e943451bb126af67209f40) --- src/devices/ovs/nm-ovsdb.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index e20632ede..c86dece9f 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -707,7 +707,7 @@ _add_interface(NMOvsdb * self, nm_auto_decref_json json_t *interfaces = NULL; nm_auto_decref_json json_t *new_interfaces = NULL; gboolean has_interface = FALSE; - gboolean interface_is_internal; + gboolean interface_is_local; gs_free char * bridge_cloned_mac = NULL; gs_free char * interface_cloned_mac = NULL; GError * error = NULL; @@ -721,10 +721,10 @@ _add_interface(NMOvsdb * self, new_ports = json_array(); new_interfaces = json_array(); - bridge_name = nm_connection_get_interface_name(bridge); - port_name = nm_connection_get_interface_name(port); - interface_name = nm_connection_get_interface_name(interface); - interface_is_internal = nm_streq0(bridge_name, interface_name); + bridge_name = nm_connection_get_interface_name(bridge); + port_name = nm_connection_get_interface_name(port); + interface_name = nm_connection_get_interface_name(interface); + interface_is_local = nm_streq0(bridge_name, interface_name); /* Determine cloned MAC addresses */ if (!nm_device_hw_addr_get_cloned(bridge_device, @@ -733,7 +733,7 @@ _add_interface(NMOvsdb * self, &bridge_cloned_mac, NULL, &error)) { - _LOGW("Cannot determine cloned mac for OVS %s '%s': %s", + _LOGW("Cannot determine cloned MAC for OVS %s '%s': %s", "bridge", bridge_name, error->message); @@ -746,14 +746,14 @@ _add_interface(NMOvsdb * self, &interface_cloned_mac, NULL, &error)) { - _LOGW("Cannot determine cloned mac for OVS %s '%s': %s", + _LOGW("Cannot determine cloned MAC for OVS %s '%s': %s", "interface", interface_name, error->message); g_clear_error(&error); } - if (interface_is_internal && !bridge_cloned_mac && interface_cloned_mac) { + if (interface_is_local && !bridge_cloned_mac && interface_cloned_mac) { _LOGT("'%s' is a local ovs-interface, the MAC will be set on ovs-bridge '%s'", interface_name, bridge_name); @@ -823,7 +823,7 @@ _add_interface(NMOvsdb * self, g_return_if_fail(ovs_bridge); _expect_bridge_ports(params, ovs_bridge->name, ports); _set_bridge_ports(params, bridge_name, new_ports); - if (bridge_cloned_mac && interface_is_internal) + if (bridge_cloned_mac && interface_is_local) _set_bridge_mac(params, bridge_name, bridge_cloned_mac); } From 031583c1d3f47eeff38c2880446302c269824a13 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 23 Nov 2020 09:34:31 +0100 Subject: [PATCH 2/2] ovs: avoid ovs error when same MAC is set on a local interface and bridge If the same MAC address is set on both the bridge connection and the interface connection, and the interface is local, NM currently sets the hwaddr record in both Bridge and Interface ovsdb tables. As a result, ovs complains with error: bridge|ERR|interface br0: ignoring mac in Interface record (use Bridge record to set local port's mac) Avoid this error: if the bridge and interface MACs are the same, just set the address in the Bridge table; if they are different, give a more detailed warning and ignore the interface MAC. https://bugzilla.redhat.com/show_bug.cgi?id=1899745 (cherry picked from commit c4beaac67b2927141a69ce38c5988d6b56812415) --- src/devices/ovs/nm-ovsdb.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index c86dece9f..27ef789f7 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -753,11 +753,27 @@ _add_interface(NMOvsdb * self, g_clear_error(&error); } - if (interface_is_local && !bridge_cloned_mac && interface_cloned_mac) { - _LOGT("'%s' is a local ovs-interface, the MAC will be set on ovs-bridge '%s'", - interface_name, - bridge_name); - bridge_cloned_mac = g_steal_pointer(&interface_cloned_mac); + /* For local interfaces, ovs complains if it finds a + * MAC address in the Interface table because it only takes + * the MAC from the Bridge table. + * Set any cloned MAC present in a local interface connection + * into the Bridge table, unless conflicting with the bridge MAC. */ + if (interface_is_local && interface_cloned_mac) { + if (bridge_cloned_mac && !nm_streq(interface_cloned_mac, bridge_cloned_mac)) { + _LOGW("Cloned MAC '%s' of local ovs-interface '%s' conflicts with MAC '%s' of bridge " + "'%s'", + interface_cloned_mac, + interface_name, + bridge_cloned_mac, + bridge_name); + nm_clear_g_free(&interface_cloned_mac); + } else { + nm_clear_g_free(&bridge_cloned_mac); + bridge_cloned_mac = g_steal_pointer(&interface_cloned_mac); + _LOGT("'%s' is a local ovs-interface, the MAC will be set on ovs-bridge '%s'", + interface_name, + bridge_name); + } } g_hash_table_iter_init(&iter, priv->bridges);