From 9f9d1a37fac2264aa3e698982e7100c4916552c0 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 25 Oct 2019 10:36:52 +0200 Subject: [PATCH 1/2] ovs: fix memory leak (cherry picked from commit 508c7679cfb06f9d5149e637737d28a958f9131c) (cherry picked from commit ad17cfff249e7edde0011dc80f09f496ba2fe020) (cherry picked from commit 836b9e24a475fa9ef5b86d9ca34c751336da4b52) --- src/devices/ovs/nm-ovsdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 5b50f8406..8b6dca7a5 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -1068,7 +1068,7 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg) OvsdbMethodCall *call = NULL; OvsdbMethodCallback callback; gpointer user_data; - GError *local = NULL; + gs_free_error GError *local = NULL; if (json_unpack_ex (msg, &json_error, 0, "{s?:o, s?:s, s?:o, s?:o, s?:o}", "id", &json_id, -- 2.21.0 From 0fbf23ec85da1d8df74ead27301395bb027e3645 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 24 Oct 2019 15:59:43 +0200 Subject: [PATCH 2/2] ovs: allow changing mac address of bridges and interfaces Allow changing the cloned MAC address for OVS bridges and interfaces. The MAC address set on the bridge is propagated by ovs to the local interface (the one with the same name as the bridge), while all other internal interfaces use the address defined in the interface connection. https://bugzilla.redhat.com/show_bug.cgi?id=1763734 https://bugzilla.redhat.com/show_bug.cgi?id=1740557 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/321 (cherry picked from commit 101e65d2bb1920853da4621e220b4825860ec46d) (cherry picked from commit b366234a3a1deb4a6924744f6b7cafa9f2650e3e) (cherry picked from commit 5a4a5f637bb65e9833836e724b3beea19158e9f8) --- clients/common/nm-meta-setting-desc.c | 1 + src/devices/ovs/nm-device-ovs-port.c | 5 ++ src/devices/ovs/nm-ovsdb.c | 88 ++++++++++++++++++++++----- src/devices/ovs/nm-ovsdb.h | 1 + 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index 9b5debcfe..e68a57e34 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -7864,6 +7864,7 @@ const NMMetaSettingInfoEditor nm_meta_setting_infos_editor[] = { .valid_parts = NM_META_SETTING_VALID_PARTS ( NM_META_SETTING_VALID_PART_ITEM (CONNECTION, TRUE), NM_META_SETTING_VALID_PART_ITEM (OVS_BRIDGE, TRUE), + NM_META_SETTING_VALID_PART_ITEM (WIRED, FALSE), ), ), SETTING_INFO (OVS_INTERFACE, diff --git a/src/devices/ovs/nm-device-ovs-port.c b/src/devices/ovs/nm-device-ovs-port.c index 8a93a5a9d..39e64b3f1 100644 --- a/src/devices/ovs/nm-device-ovs-port.c +++ b/src/devices/ovs/nm-device-ovs-port.c @@ -102,6 +102,7 @@ enslave_slave (NMDevice *device, NMDevice *slave, NMConnection *connection, gboo { NMActiveConnection *ac_port = NULL; NMActiveConnection *ac_bridge = NULL; + NMDevice *bridge_device; if (!configure) return TRUE; @@ -111,10 +112,14 @@ enslave_slave (NMDevice *device, NMDevice *slave, NMConnection *connection, gboo if (!ac_bridge) ac_bridge = ac_port; + bridge_device = nm_active_connection_get_device (ac_bridge); + nm_ovsdb_add_interface (nm_ovsdb_get (), nm_active_connection_get_applied_connection (ac_bridge), nm_device_get_applied_connection (device), nm_device_get_applied_connection (slave), + bridge_device, + slave, add_iface_cb, g_object_ref (slave)); return TRUE; diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c index 8b6dca7a5..82e25f396 100644 --- a/src/devices/ovs/nm-ovsdb.c +++ b/src/devices/ovs/nm-ovsdb.c @@ -28,6 +28,7 @@ #include "devices/nm-device.h" #include "platform/nm-platform.h" #include "nm-core-internal.h" +#include "devices/nm-device.h" /*****************************************************************************/ @@ -131,6 +132,8 @@ typedef struct { NMConnection *bridge; NMConnection *port; NMConnection *interface; + NMDevice *bridge_device; + NMDevice *interface_device; }; }; } OvsdbMethodCall; @@ -183,6 +186,7 @@ static void ovsdb_call_method (NMOvsdb *self, OvsdbCommand command, const char *ifname, NMConnection *bridge, NMConnection *port, NMConnection *interface, + NMDevice *bridge_device, NMDevice *interface_device, OvsdbMethodCallback callback, gpointer user_data) { NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self); @@ -205,6 +209,8 @@ ovsdb_call_method (NMOvsdb *self, OvsdbCommand command, call->bridge = nm_simple_connection_new_clone (bridge); call->port = nm_simple_connection_new_clone (port); call->interface = nm_simple_connection_new_clone (interface); + call->bridge_device = g_object_ref (bridge_device); + call->interface_device = g_object_ref (interface_device); break; case OVSDB_DEL_INTERFACE: call->ifname = g_strdup (ifname); @@ -337,17 +343,32 @@ _set_port_interfaces (json_t *params, const char *ifname, json_t *new_interfaces * Returns an commands that adds new interface from a given connection. */ static void -_insert_interface (json_t *params, NMConnection *interface) +_insert_interface (json_t *params, NMConnection *interface, NMDevice *interface_device) { const char *type = NULL; NMSettingOvsInterface *s_ovs_iface; NMSettingOvsPatch *s_ovs_patch; json_t *options = json_array (); + gs_free char *cloned_mac = NULL; + gs_free_error GError *error = NULL; + json_t *row; s_ovs_iface = nm_connection_get_setting_ovs_interface (interface); if (s_ovs_iface) type = nm_setting_ovs_interface_get_interface_type (s_ovs_iface); + if (!nm_device_hw_addr_get_cloned (interface_device, + interface, + FALSE, + &cloned_mac, + NULL, + &error)) { + _LOGW ("Cannot determine cloned mac for OVS %s '%s': %s", + "interface", + nm_connection_get_interface_name (interface), + error->message); + } + json_array_append_new (options, json_string ("map")); s_ovs_patch = nm_connection_get_setting_ovs_patch (interface); if (s_ovs_patch) { @@ -358,14 +379,22 @@ _insert_interface (json_t *params, NMConnection *interface) json_array_append_new (options, json_array ()); } + row = json_pack ("{s:s, s:s, s:o, s:[s, [[s, s]]]}", + "name", nm_connection_get_interface_name (interface), + "type", type ?: "", + "options", options, + "external_ids", "map", + "NM.connection.uuid", nm_connection_get_uuid (interface)); + + if (cloned_mac) + json_object_set_new (row, "mac", json_string (cloned_mac)); + json_array_append_new (params, - json_pack ("{s:s, s:s, s:{s:s, s:s, s:o, s:[s, [[s, s]]]}, s:s}", - "op", "insert", "table", "Interface", "row", - "name", nm_connection_get_interface_name (interface), - "type", type ?: "", - "options", options, - "external_ids", "map", "NM.connection.uuid", nm_connection_get_uuid (interface), - "uuid-name", "rowInterface")); + json_pack ("{s:s, s:s, s:o, s:s}", + "op", "insert", + "table", "Interface", + "row", row, + "uuid-name", "rowInterface")); } /** @@ -429,7 +458,7 @@ _insert_port (json_t *params, NMConnection *port, json_t *new_interfaces) * Returns an commands that adds new bridge from a given connection. */ static void -_insert_bridge (json_t *params, NMConnection *bridge, json_t *new_ports) +_insert_bridge (json_t *params, NMConnection *bridge, NMDevice *bridge_device, json_t *new_ports) { NMSettingOvsBridge *s_ovs_bridge; const char *fail_mode = NULL; @@ -437,9 +466,23 @@ _insert_bridge (json_t *params, NMConnection *bridge, json_t *new_ports) gboolean rstp_enable = FALSE; gboolean stp_enable = FALSE; json_t *row; + gs_free_error GError *error = NULL; + gs_free char *cloned_mac = NULL; s_ovs_bridge = nm_connection_get_setting_ovs_bridge (bridge); + if (!nm_device_hw_addr_get_cloned (bridge_device, + bridge, + FALSE, + &cloned_mac, + NULL, + &error)) { + _LOGW ("Cannot determine cloned mac for OVS %s '%s': %s", + "bridge", + nm_connection_get_interface_name (bridge), + error->message); + } + row = json_object (); if (s_ovs_bridge) { @@ -464,6 +507,12 @@ _insert_bridge (json_t *params, NMConnection *bridge, json_t *new_ports) json_pack ("[s, [[s, s]]]", "map", "NM.connection.uuid", nm_connection_get_uuid (bridge))); + if (cloned_mac) { + json_object_set_new (row, "other_config", + json_pack ("[s, [[s, s]]]", "map", + "hwaddr", cloned_mac)); + } + /* Create a new one. */ json_array_append_new (params, json_pack ("{s:s, s:s, s:o, s:s}", "op", "insert", "table", "Bridge", @@ -493,7 +542,8 @@ _inc_next_cfg (const char *db_uuid) */ static void _add_interface (NMOvsdb *self, json_t *params, - NMConnection *bridge, NMConnection *port, NMConnection *interface) + NMConnection *bridge, NMConnection *port, NMConnection *interface, + NMDevice *bridge_device, NMDevice *interface_device) { NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self); GHashTableIter iter; @@ -566,7 +616,7 @@ _add_interface (NMOvsdb *self, json_t *params, _expect_ovs_bridges (params, priv->db_uuid, bridges); json_array_append_new (new_bridges, json_pack ("[s, s]", "named-uuid", "rowBridge")); _set_ovs_bridges (params, priv->db_uuid, new_bridges); - _insert_bridge (params, bridge, new_ports); + _insert_bridge (params, bridge, bridge_device, new_ports); } else { /* Bridge already exists. */ g_return_if_fail (ovs_bridge); @@ -584,7 +634,7 @@ _add_interface (NMOvsdb *self, json_t *params, } if (!has_interface) { - _insert_interface (params, interface); + _insert_interface (params, interface, interface_device); json_array_append_new (new_interfaces, json_pack ("[s, s]", "named-uuid", "rowInterface")); } } @@ -733,7 +783,8 @@ ovsdb_next_command (NMOvsdb *self) json_array_append_new (params, json_string ("Open_vSwitch")); json_array_append_new (params, _inc_next_cfg (priv->db_uuid)); - _add_interface (self, params, call->bridge, call->port, call->interface); + _add_interface (self, params, call->bridge, call->port, call->interface, + call->bridge_device, call->interface_device); msg = json_pack ("{s:i, s:s, s:o}", "id", call->id, @@ -1385,7 +1436,7 @@ ovsdb_try_connect (NMOvsdb *self) /* Queue a monitor call before any other command, ensuring that we have an up * to date view of existing bridged that we need for add and remove ops. */ ovsdb_call_method (self, OVSDB_MONITOR, NULL, - NULL, NULL, NULL, _monitor_bridges_cb, NULL); + NULL, NULL, NULL, NULL, NULL, _monitor_bridges_cb, NULL); } /*****************************************************************************/ @@ -1426,6 +1477,7 @@ out: void nm_ovsdb_add_interface (NMOvsdb *self, NMConnection *bridge, NMConnection *port, NMConnection *interface, + NMDevice *bridge_device, NMDevice *interface_device, NMOvsdbCallback callback, gpointer user_data) { OvsdbCall *call; @@ -1435,7 +1487,9 @@ nm_ovsdb_add_interface (NMOvsdb *self, call->user_data = user_data; ovsdb_call_method (self, OVSDB_ADD_INTERFACE, NULL, - bridge, port, interface, _transact_cb, call); + bridge, port, interface, + bridge_device, interface_device, + _transact_cb, call); } void @@ -1449,7 +1503,7 @@ nm_ovsdb_del_interface (NMOvsdb *self, const char *ifname, call->user_data = user_data; ovsdb_call_method (self, OVSDB_DEL_INTERFACE, ifname, - NULL, NULL, NULL, _transact_cb, call); + NULL, NULL, NULL, NULL, NULL, _transact_cb, call); } /*****************************************************************************/ @@ -1466,6 +1520,8 @@ _clear_call (gpointer data) g_clear_object (&call->bridge); g_clear_object (&call->port); g_clear_object (&call->interface); + g_clear_object (&call->bridge_device); + g_clear_object (&call->interface_device); break; case OVSDB_DEL_INTERFACE: g_clear_pointer (&call->ifname, g_free); diff --git a/src/devices/ovs/nm-ovsdb.h b/src/devices/ovs/nm-ovsdb.h index cf9fe2a21..f9dbabbb7 100644 --- a/src/devices/ovs/nm-ovsdb.h +++ b/src/devices/ovs/nm-ovsdb.h @@ -42,6 +42,7 @@ GType nm_ovsdb_get_type (void); void nm_ovsdb_add_interface (NMOvsdb *self, NMConnection *bridge, NMConnection *port, NMConnection *interface, + NMDevice *bridge_device, NMDevice *interface_device, NMOvsdbCallback callback, gpointer user_data); void nm_ovsdb_del_interface (NMOvsdb *self, const char *ifname, -- 2.21.0