From 6f6c044739a04e1a3b59274853ca869bcbfd30d8 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 28 Apr 2022 10:03:56 +0200 Subject: [PATCH 1/5] ovsdb: fix memory leak @error was leaked when created inside the function. While at it, remove the goto. Fixes: 830a5a14cb29 ('device: add support for OpenVSwitch devices') --- src/core/devices/ovs/nm-ovsdb.c | 36 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/core/devices/ovs/nm-ovsdb.c b/src/core/devices/ovs/nm-ovsdb.c index 3c91323d78..44e16cb78f 100644 --- a/src/core/devices/ovs/nm-ovsdb.c +++ b/src/core/devices/ovs/nm-ovsdb.c @@ -2486,29 +2486,27 @@ typedef struct { static void _transact_cb(NMOvsdb *self, json_t *result, GError *error, gpointer user_data) { - OvsdbCall *call = user_data; - const char *err; - const char *err_details; - size_t index; - json_t *value; + OvsdbCall *call = user_data; + gs_free_error GError *local = NULL; + const char *err; + const char *err_details; + size_t index; + json_t *value; - if (error) - goto out; - - json_array_foreach (result, index, value) { - if (json_unpack(value, "{s:s, s:s}", "error", &err, "details", &err_details) == 0) { - g_set_error(&error, - G_IO_ERROR, - G_IO_ERROR_FAILED, - "Error running the transaction: %s: %s", - err, - err_details); - goto out; + if (!error) { + json_array_foreach (result, index, value) { + if (json_unpack(value, "{s:s, s:s}", "error", &err, "details", &err_details) == 0) { + local = g_error_new(G_IO_ERROR, + G_IO_ERROR_FAILED, + "Error running the transaction: %s: %s", + err, + err_details); + break; + } } } -out: - call->callback(error, call->user_data); + call->callback(local ?: error, call->user_data); nm_g_slice_free(call); } From bcc958c411287ccc650cd04bc77ac22c8fecb08c Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 2 May 2022 13:58:04 +0200 Subject: [PATCH 2/5] device: rename {enslave,release}_slave() to {attach,detach}_port() Rename the enslave_slave() and release_slave() device methods to attach_port() and detach_port(). --- src/core/devices/nm-device-bond.c | 28 +++++----- src/core/devices/nm-device-bridge.c | 30 +++++----- src/core/devices/nm-device-vrf.c | 46 +++++++-------- src/core/devices/nm-device.c | 16 +++--- src/core/devices/nm-device.h | 10 ++-- src/core/devices/ovs/nm-device-ovs-bridge.c | 10 ++-- src/core/devices/ovs/nm-device-ovs-port.c | 48 ++++++++-------- src/core/devices/team/nm-device-team.c | 62 ++++++++++----------- 8 files changed, 125 insertions(+), 125 deletions(-) diff --git a/src/core/devices/nm-device-bond.c b/src/core/devices/nm-device-bond.c index 16896d5753..80400afcc4 100644 --- a/src/core/devices/nm-device-bond.c +++ b/src/core/devices/nm-device-bond.c @@ -425,7 +425,7 @@ commit_port_options(NMDevice *bond_device, NMDevice *port, NMSettingBondPort *s_ } static gboolean -enslave_slave(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) +attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) { NMDeviceBond *self = NM_DEVICE_BOND(device); NMSettingBondPort *s_port; @@ -442,7 +442,7 @@ enslave_slave(NMDevice *device, NMDevice *port, NMConnection *connection, gboole nm_device_bring_up(port, TRUE, NULL); if (!success) { - _LOGI(LOGD_BOND, "assigning bond port %s: failed", nm_device_get_ip_iface(port)); + _LOGI(LOGD_BOND, "attaching bond port %s: failed", nm_device_get_ip_iface(port)); return FALSE; } @@ -450,15 +450,15 @@ enslave_slave(NMDevice *device, NMDevice *port, NMConnection *connection, gboole commit_port_options(device, port, s_port); - _LOGI(LOGD_BOND, "assigned bond port %s", nm_device_get_ip_iface(port)); + _LOGI(LOGD_BOND, "attached bond port %s", nm_device_get_ip_iface(port)); } else - _LOGI(LOGD_BOND, "bond port %s was assigned", nm_device_get_ip_iface(port)); + _LOGI(LOGD_BOND, "bond port %s was attached", nm_device_get_ip_iface(port)); return TRUE; } static void -release_slave(NMDevice *device, NMDevice *slave, gboolean configure) +detach_port(NMDevice *device, NMDevice *port, gboolean configure) { NMDeviceBond *self = NM_DEVICE_BOND(device); gboolean success; @@ -472,10 +472,10 @@ release_slave(NMDevice *device, NMDevice *slave, gboolean configure) configure = FALSE; } - ifindex_slave = nm_device_get_ip_ifindex(slave); + ifindex_slave = nm_device_get_ip_ifindex(port); if (ifindex_slave <= 0) - _LOGD(LOGD_BOND, "bond slave %s is already released", nm_device_get_ip_iface(slave)); + _LOGD(LOGD_BOND, "bond port %s is already detached", nm_device_get_ip_iface(port)); if (configure) { NMConnection *applied; @@ -490,9 +490,9 @@ release_slave(NMDevice *device, NMDevice *slave, gboolean configure) ifindex_slave); if (success) { - _LOGI(LOGD_BOND, "released bond slave %s", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_BOND, "detached bond port %s", nm_device_get_ip_iface(port)); } else { - _LOGW(LOGD_BOND, "failed to release bond slave %s", nm_device_get_ip_iface(slave)); + _LOGW(LOGD_BOND, "failed to detach bond port %s", nm_device_get_ip_iface(port)); } } @@ -512,12 +512,12 @@ release_slave(NMDevice *device, NMDevice *slave, gboolean configure) * other state is noticed by the now-released slave. */ if (ifindex_slave > 0) { - if (!nm_device_bring_up(slave, TRUE, NULL)) - _LOGW(LOGD_BOND, "released bond slave could not be brought up."); + if (!nm_device_bring_up(port, TRUE, NULL)) + _LOGW(LOGD_BOND, "detached bond port could not be brought up."); } } else { if (ifindex_slave > 0) { - _LOGI(LOGD_BOND, "bond slave %s was released", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_BOND, "bond port %s was detached", nm_device_get_ip_iface(port)); } } } @@ -663,8 +663,8 @@ nm_device_bond_class_init(NMDeviceBondClass *klass) device_class->create_and_realize = create_and_realize; device_class->act_stage1_prepare = act_stage1_prepare; device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired; - device_class->enslave_slave = enslave_slave; - device_class->release_slave = release_slave; + device_class->attach_port = attach_port; + device_class->detach_port = detach_port; device_class->can_reapply_change = can_reapply_change; device_class->reapply_connection = reapply_connection; } diff --git a/src/core/devices/nm-device-bridge.c b/src/core/devices/nm-device-bridge.c index f11c172abb..796e58c650 100644 --- a/src/core/devices/nm-device-bridge.c +++ b/src/core/devices/nm-device-bridge.c @@ -975,7 +975,7 @@ deactivate(NMDevice *device) } static gboolean -enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gboolean configure) +attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) { NMDeviceBridge *self = NM_DEVICE_BRIDGE(device); NMConnection *master_connection; @@ -985,7 +985,7 @@ enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gbool if (configure) { if (!nm_platform_link_enslave(nm_device_get_platform(device), nm_device_get_ip_ifindex(device), - nm_device_get_ip_ifindex(slave))) + nm_device_get_ip_ifindex(port))) return FALSE; master_connection = nm_device_get_applied_connection(device); @@ -1009,25 +1009,25 @@ enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gbool * (except for the default one) and so there's no need to flush. */ if (plat_vlans - && !nm_platform_link_set_bridge_vlans(nm_device_get_platform(slave), - nm_device_get_ifindex(slave), + && !nm_platform_link_set_bridge_vlans(nm_device_get_platform(port), + nm_device_get_ifindex(port), TRUE, plat_vlans)) return FALSE; } - commit_slave_options(slave, s_port); + commit_slave_options(port, s_port); - _LOGI(LOGD_BRIDGE, "attached bridge port %s", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_BRIDGE, "attached bridge port %s", nm_device_get_ip_iface(port)); } else { - _LOGI(LOGD_BRIDGE, "bridge port %s was attached", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_BRIDGE, "bridge port %s was attached", nm_device_get_ip_iface(port)); } return TRUE; } static void -release_slave(NMDevice *device, NMDevice *slave, gboolean configure) +detach_port(NMDevice *device, NMDevice *port, gboolean configure) { NMDeviceBridge *self = NM_DEVICE_BRIDGE(device); gboolean success; @@ -1040,10 +1040,10 @@ release_slave(NMDevice *device, NMDevice *slave, gboolean configure) configure = FALSE; } - ifindex_slave = nm_device_get_ip_ifindex(slave); + ifindex_slave = nm_device_get_ip_ifindex(port); if (ifindex_slave <= 0) { - _LOGD(LOGD_TEAM, "bond slave %s is already released", nm_device_get_ip_iface(slave)); + _LOGD(LOGD_TEAM, "bridge port %s is already detached", nm_device_get_ip_iface(port)); return; } @@ -1053,12 +1053,12 @@ release_slave(NMDevice *device, NMDevice *slave, gboolean configure) ifindex_slave); if (success) { - _LOGI(LOGD_BRIDGE, "detached bridge port %s", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_BRIDGE, "detached bridge port %s", nm_device_get_ip_iface(port)); } else { - _LOGW(LOGD_BRIDGE, "failed to detach bridge port %s", nm_device_get_ip_iface(slave)); + _LOGW(LOGD_BRIDGE, "failed to detach bridge port %s", nm_device_get_ip_iface(port)); } } else { - _LOGI(LOGD_BRIDGE, "bridge port %s was detached", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_BRIDGE, "bridge port %s was detached", nm_device_get_ip_iface(port)); } } @@ -1182,8 +1182,8 @@ nm_device_bridge_class_init(NMDeviceBridgeClass *klass) device_class->act_stage1_prepare = act_stage1_prepare; device_class->act_stage2_config = act_stage2_config; device_class->deactivate = deactivate; - device_class->enslave_slave = enslave_slave; - device_class->release_slave = release_slave; + device_class->attach_port = attach_port; + device_class->detach_port = detach_port; device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired; } diff --git a/src/core/devices/nm-device-vrf.c b/src/core/devices/nm-device-vrf.c index ae80e1d462..6454d4d30d 100644 --- a/src/core/devices/nm-device-vrf.c +++ b/src/core/devices/nm-device-vrf.c @@ -207,37 +207,37 @@ update_connection(NMDevice *device, NMConnection *connection) } static gboolean -enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gboolean configure) +attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) { - NMDeviceVrf *self = NM_DEVICE_VRF(device); - gboolean success = TRUE; - const char *slave_iface = nm_device_get_ip_iface(slave); + NMDeviceVrf *self = NM_DEVICE_VRF(device); + gboolean success = TRUE; + const char *port_iface = nm_device_get_ip_iface(port); - nm_device_master_check_slave_physical_port(device, slave, LOGD_DEVICE); + nm_device_master_check_slave_physical_port(device, port, LOGD_DEVICE); if (configure) { - nm_device_take_down(slave, TRUE); + nm_device_take_down(port, TRUE); success = nm_platform_link_enslave(nm_device_get_platform(device), nm_device_get_ip_ifindex(device), - nm_device_get_ip_ifindex(slave)); - nm_device_bring_up(slave, TRUE, NULL); + nm_device_get_ip_ifindex(port)); + nm_device_bring_up(port, TRUE, NULL); if (!success) return FALSE; - _LOGI(LOGD_DEVICE, "enslaved VRF slave %s", slave_iface); + _LOGI(LOGD_DEVICE, "attached VRF port %s", port_iface); } else - _LOGI(LOGD_BOND, "VRF slave %s was enslaved", slave_iface); + _LOGI(LOGD_BOND, "VRF port %s was attached", port_iface); return TRUE; } static void -release_slave(NMDevice *device, NMDevice *slave, gboolean configure) +detach_port(NMDevice *device, NMDevice *port, gboolean configure) { NMDeviceVrf *self = NM_DEVICE_VRF(device); gboolean success; - int ifindex_slave; + int ifindex_port; int ifindex; if (configure) { @@ -246,26 +246,26 @@ release_slave(NMDevice *device, NMDevice *slave, gboolean configure) configure = FALSE; } - ifindex_slave = nm_device_get_ip_ifindex(slave); + ifindex_port = nm_device_get_ip_ifindex(port); - if (ifindex_slave <= 0) - _LOGD(LOGD_DEVICE, "VRF slave %s is already released", nm_device_get_ip_iface(slave)); + if (ifindex_port <= 0) + _LOGD(LOGD_DEVICE, "VRF port %s is already detached", nm_device_get_ip_iface(port)); if (configure) { - if (ifindex_slave > 0) { + if (ifindex_port > 0) { success = nm_platform_link_release(nm_device_get_platform(device), nm_device_get_ip_ifindex(device), - ifindex_slave); + ifindex_port); if (success) { - _LOGI(LOGD_DEVICE, "released VRF slave %s", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_DEVICE, "detached VRF port %s", nm_device_get_ip_iface(port)); } else { - _LOGW(LOGD_DEVICE, "failed to release VRF slave %s", nm_device_get_ip_iface(slave)); + _LOGW(LOGD_DEVICE, "failed to detach VRF port %s", nm_device_get_ip_iface(port)); } } } else { - if (ifindex_slave > 0) { - _LOGI(LOGD_DEVICE, "VRF slave %s was released", nm_device_get_ip_iface(slave)); + if (ifindex_port > 0) { + _LOGI(LOGD_DEVICE, "VRF port %s was detached", nm_device_get_ip_iface(port)); } } } @@ -316,8 +316,8 @@ nm_device_vrf_class_init(NMDeviceVrfClass *klass) device_class->is_master = TRUE; device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES(NM_LINK_TYPE_VRF); - device_class->enslave_slave = enslave_slave; - device_class->release_slave = release_slave; + device_class->attach_port = attach_port; + device_class->detach_port = detach_port; device_class->link_changed = link_changed; device_class->unrealize_notify = unrealize_notify; device_class->create_and_realize = create_and_realize; diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index d77f3fa501..93f9e7fd2f 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -5948,7 +5948,7 @@ nm_device_master_enslave_slave(NMDevice *self, NMDevice *slave, NMConnection *co g_return_val_if_fail(self != NULL, FALSE); g_return_val_if_fail(slave != NULL, FALSE); - g_return_val_if_fail(NM_DEVICE_GET_CLASS(self)->enslave_slave != NULL, FALSE); + g_return_val_if_fail(NM_DEVICE_GET_CLASS(self)->attach_port != NULL, FALSE); info = find_slave_info(self, slave); if (!info) @@ -5961,7 +5961,7 @@ nm_device_master_enslave_slave(NMDevice *self, NMDevice *slave, NMConnection *co if (configure) g_return_val_if_fail(nm_device_get_state(slave) >= NM_DEVICE_STATE_DISCONNECTED, FALSE); - success = NM_DEVICE_GET_CLASS(self)->enslave_slave(self, slave, connection, configure); + success = NM_DEVICE_GET_CLASS(self)->attach_port(self, slave, connection, configure); info->slave_is_enslaved = success; } @@ -6017,7 +6017,7 @@ nm_device_master_release_slave(NMDevice *self, RELEASE_SLAVE_TYPE_NO_CONFIG, RELEASE_SLAVE_TYPE_CONFIG, RELEASE_SLAVE_TYPE_CONFIG_FORCE)); - g_return_if_fail(NM_DEVICE_GET_CLASS(self)->release_slave != NULL); + g_return_if_fail(NM_DEVICE_GET_CLASS(self)->detach_port != NULL); info = find_slave_info(self, slave); @@ -6042,9 +6042,9 @@ nm_device_master_release_slave(NMDevice *self, /* first, let subclasses handle the release ... */ if (info->slave_is_enslaved || nm_device_sys_iface_state_is_external(slave) || release_type >= RELEASE_SLAVE_TYPE_CONFIG_FORCE) - NM_DEVICE_GET_CLASS(self)->release_slave(self, - slave, - release_type >= RELEASE_SLAVE_TYPE_CONFIG); + NM_DEVICE_GET_CLASS(self)->detach_port(self, + slave, + release_type >= RELEASE_SLAVE_TYPE_CONFIG); /* raise notifications about the release, including clearing is_enslaved. */ nm_device_slave_notify_release(slave, reason); @@ -6385,7 +6385,7 @@ device_recheck_slave_status(NMDevice *self, const NMPlatformLink *plink) NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); } - if (master && NM_DEVICE_GET_CLASS(master)->enslave_slave) { + if (master && NM_DEVICE_GET_CLASS(master)->attach_port) { nm_device_master_add_slave(master, self, FALSE); goto out; } @@ -7609,7 +7609,7 @@ nm_device_master_add_slave(NMDevice *self, NMDevice *slave, gboolean configure) g_return_val_if_fail(NM_IS_DEVICE(self), FALSE); g_return_val_if_fail(NM_IS_DEVICE(slave), FALSE); - g_return_val_if_fail(NM_DEVICE_GET_CLASS(self)->enslave_slave != NULL, FALSE); + g_return_val_if_fail(NM_DEVICE_GET_CLASS(self)->attach_port != NULL, FALSE); priv = NM_DEVICE_GET_PRIVATE(self); slave_priv = NM_DEVICE_GET_PRIVATE(slave); diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index 80def12511..bfa274d69e 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -373,12 +373,12 @@ typedef struct _NMDeviceClass { NMConnection *connection, GError **error); - gboolean (*enslave_slave)(NMDevice *self, - NMDevice *slave, - NMConnection *connection, - gboolean configure); + gboolean (*attach_port)(NMDevice *self, + NMDevice *port, + NMConnection *connection, + gboolean configure); - void (*release_slave)(NMDevice *self, NMDevice *slave, gboolean configure); + void (*detach_port)(NMDevice *self, NMDevice *port, gboolean configure); void (*parent_changed_notify)(NMDevice *self, int old_ifindex, diff --git a/src/core/devices/ovs/nm-device-ovs-bridge.c b/src/core/devices/ovs/nm-device-ovs-bridge.c index 683ada1339..da0a830e5d 100644 --- a/src/core/devices/ovs/nm-device-ovs-bridge.c +++ b/src/core/devices/ovs/nm-device-ovs-bridge.c @@ -79,19 +79,19 @@ act_stage3_ip_config(NMDevice *device, int addr_family) } static gboolean -enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gboolean configure) +attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) { if (!configure) return TRUE; - if (!NM_IS_DEVICE_OVS_PORT(slave)) + if (!NM_IS_DEVICE_OVS_PORT(port)) return FALSE; return TRUE; } static void -release_slave(NMDevice *device, NMDevice *slave, gboolean configure) +detach_port(NMDevice *device, NMDevice *port, gboolean configure) {} void @@ -159,8 +159,8 @@ nm_device_ovs_bridge_class_init(NMDeviceOvsBridgeClass *klass) device_class->get_generic_capabilities = get_generic_capabilities; device_class->act_stage3_ip_config = act_stage3_ip_config; device_class->ready_for_ip_config = ready_for_ip_config; - device_class->enslave_slave = enslave_slave; - device_class->release_slave = release_slave; + device_class->attach_port = attach_port; + device_class->detach_port = detach_port; device_class->can_reapply_change_ovs_external_ids = TRUE; device_class->reapply_connection = nm_device_ovs_reapply_connection; } diff --git a/src/core/devices/ovs/nm-device-ovs-port.c b/src/core/devices/ovs/nm-device-ovs-port.c index 116f58c43a..72c534ebf0 100644 --- a/src/core/devices/ovs/nm-device-ovs-port.c +++ b/src/core/devices/ovs/nm-device-ovs-port.c @@ -116,7 +116,7 @@ set_mtu_cb(GError *error, gpointer user_data) } static gboolean -enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gboolean configure) +attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) { NMDeviceOvsPort *self = NM_DEVICE_OVS_PORT(device); NMActiveConnection *ac_port = NULL; @@ -131,39 +131,39 @@ enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gbool ac_bridge = nm_active_connection_get_master(ac_port); if (!ac_bridge) { _LOGW(LOGD_DEVICE, - "can't enslave %s: bridge active-connection not found", - nm_device_get_iface(slave)); + "can't attach %s: bridge active-connection not found", + nm_device_get_iface(port)); return FALSE; } bridge_device = nm_active_connection_get_device(ac_bridge); if (!bridge_device) { - _LOGW(LOGD_DEVICE, "can't enslave %s: bridge device not found", nm_device_get_iface(slave)); + _LOGW(LOGD_DEVICE, "can't attach %s: bridge device not found", nm_device_get_iface(port)); return FALSE; } 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), + nm_device_get_applied_connection(port), bridge_device, - slave, + port, add_iface_cb, - g_object_ref(slave)); + g_object_ref(port)); /* DPDK ports does not have a link after the devbind, so the MTU must be * set on ovsdb after adding the interface. */ - if (NM_IS_DEVICE_OVS_INTERFACE(slave) && _ovs_interface_is_dpdk(slave)) { - s_wired = nm_device_get_applied_setting(slave, NM_TYPE_SETTING_WIRED); + if (NM_IS_DEVICE_OVS_INTERFACE(port) && _ovs_interface_is_dpdk(port)) { + s_wired = nm_device_get_applied_setting(port, NM_TYPE_SETTING_WIRED); if (!s_wired || !nm_setting_wired_get_mtu(s_wired)) return TRUE; nm_ovsdb_set_interface_mtu(nm_ovsdb_get(), - nm_device_get_ip_iface(slave), + nm_device_get_ip_iface(port), nm_setting_wired_get_mtu(s_wired), set_mtu_cb, - g_object_ref(slave)); + g_object_ref(port)); } return TRUE; @@ -186,31 +186,31 @@ del_iface_cb(GError *error, gpointer user_data) } static void -release_slave(NMDevice *device, NMDevice *slave, gboolean configure) +detach_port(NMDevice *device, NMDevice *port, gboolean configure) { - NMDeviceOvsPort *self = NM_DEVICE_OVS_PORT(device); - bool slave_not_managed = !NM_IN_SET(nm_device_sys_iface_state_get(slave), - NM_DEVICE_SYS_IFACE_STATE_MANAGED, - NM_DEVICE_SYS_IFACE_STATE_ASSUME); + NMDeviceOvsPort *self = NM_DEVICE_OVS_PORT(device); + bool port_not_managed = !NM_IN_SET(nm_device_sys_iface_state_get(port), + NM_DEVICE_SYS_IFACE_STATE_MANAGED, + NM_DEVICE_SYS_IFACE_STATE_ASSUME); - _LOGI(LOGD_DEVICE, "releasing ovs interface %s", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_DEVICE, "detaching ovs interface %s", nm_device_get_ip_iface(port)); /* Even if the an interface's device has gone away (e.g. externally * removed and thus we're called with configure=FALSE), we still need * to make sure its OVSDB entry is gone. */ - if (configure || slave_not_managed) { + if (configure || port_not_managed) { nm_ovsdb_del_interface(nm_ovsdb_get(), - nm_device_get_iface(slave), + nm_device_get_iface(port), del_iface_cb, - g_object_ref(slave)); + g_object_ref(port)); } if (configure) { /* Open VSwitch is going to delete this one. We must ignore what happens * next with the interface. */ - if (NM_IS_DEVICE_OVS_INTERFACE(slave)) - nm_device_update_from_platform_link(slave, NULL); + if (NM_IS_DEVICE_OVS_INTERFACE(port)) + nm_device_update_from_platform_link(port, NULL); } } @@ -245,8 +245,8 @@ nm_device_ovs_port_class_init(NMDeviceOvsPortClass *klass) device_class->get_generic_capabilities = get_generic_capabilities; device_class->act_stage3_ip_config = act_stage3_ip_config; device_class->ready_for_ip_config = ready_for_ip_config; - device_class->enslave_slave = enslave_slave; - device_class->release_slave = release_slave; + device_class->attach_port = attach_port; + device_class->detach_port = detach_port; device_class->can_reapply_change_ovs_external_ids = TRUE; device_class->reapply_connection = nm_device_ovs_reapply_connection; } diff --git a/src/core/devices/team/nm-device-team.c b/src/core/devices/team/nm-device-team.c index b67c710020..77a259d413 100644 --- a/src/core/devices/team/nm-device-team.c +++ b/src/core/devices/team/nm-device-team.c @@ -791,18 +791,18 @@ deactivate(NMDevice *device) } static gboolean -enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gboolean configure) +attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) { - NMDeviceTeam *self = NM_DEVICE_TEAM(device); - NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE(self); - gboolean success = TRUE; - const char *slave_iface = nm_device_get_ip_iface(slave); + NMDeviceTeam *self = NM_DEVICE_TEAM(device); + NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE(self); + gboolean success = TRUE; + const char *port_iface = nm_device_get_ip_iface(port); NMSettingTeamPort *s_team_port; - nm_device_master_check_slave_physical_port(device, slave, LOGD_TEAM); + nm_device_master_check_slave_physical_port(device, port, LOGD_TEAM); if (configure) { - nm_device_take_down(slave, TRUE); + nm_device_take_down(port, TRUE); s_team_port = nm_connection_get_setting_team_port(connection); if (s_team_port) { @@ -811,19 +811,19 @@ enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gbool if (config) { if (!priv->tdc) { _LOGW(LOGD_TEAM, - "enslaved team port %s config not changed, not connected to teamd", - slave_iface); + "attached team port %s config not changed, not connected to teamd", + port_iface); } else { gs_free char *sanitized_config = NULL; int err; sanitized_config = g_strdup(config); g_strdelimit(sanitized_config, "\r\n", ' '); - err = teamdctl_port_config_update_raw(priv->tdc, slave_iface, sanitized_config); + err = teamdctl_port_config_update_raw(priv->tdc, port_iface, sanitized_config); if (err != 0) { _LOGE(LOGD_TEAM, "failed to update config for port %s (err=%d)", - slave_iface, + port_iface, err); return FALSE; } @@ -832,8 +832,8 @@ enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gbool } success = nm_platform_link_enslave(nm_device_get_platform(device), nm_device_get_ip_ifindex(device), - nm_device_get_ip_ifindex(slave)); - nm_device_bring_up(slave, TRUE, NULL); + nm_device_get_ip_ifindex(port)); + nm_device_bring_up(port, TRUE, NULL); if (!success) return FALSE; @@ -841,21 +841,21 @@ enslave_slave(NMDevice *device, NMDevice *slave, NMConnection *connection, gbool nm_clear_g_source(&priv->teamd_read_timeout); priv->teamd_read_timeout = g_timeout_add_seconds(5, teamd_read_timeout_cb, self); - _LOGI(LOGD_TEAM, "enslaved team port %s", slave_iface); + _LOGI(LOGD_TEAM, "attached team port %s", port_iface); } else - _LOGI(LOGD_TEAM, "team port %s was enslaved", slave_iface); + _LOGI(LOGD_TEAM, "team port %s was attached", port_iface); return TRUE; } static void -release_slave(NMDevice *device, NMDevice *slave, gboolean configure) +detach_port(NMDevice *device, NMDevice *port, gboolean configure) { NMDeviceTeam *self = NM_DEVICE_TEAM(device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE(self); gboolean do_release, success; NMSettingTeamPort *s_port; - int ifindex_slave; + int ifindex_port; int ifindex; do_release = configure; @@ -865,39 +865,39 @@ release_slave(NMDevice *device, NMDevice *slave, gboolean configure) do_release = FALSE; } - ifindex_slave = nm_device_get_ip_ifindex(slave); + ifindex_port = nm_device_get_ip_ifindex(port); - if (ifindex_slave <= 0) { - _LOGD(LOGD_TEAM, "team port %s is already released", nm_device_get_ip_iface(slave)); + if (ifindex_port <= 0) { + _LOGD(LOGD_TEAM, "team port %s is already detached", nm_device_get_ip_iface(port)); } else if (do_release) { success = nm_platform_link_release(nm_device_get_platform(device), nm_device_get_ip_ifindex(device), - ifindex_slave); + ifindex_port); if (success) - _LOGI(LOGD_TEAM, "released team port %s", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_TEAM, "detached team port %s", nm_device_get_ip_iface(port)); else - _LOGW(LOGD_TEAM, "failed to release team port %s", nm_device_get_ip_iface(slave)); + _LOGW(LOGD_TEAM, "failed to detach team port %s", nm_device_get_ip_iface(port)); /* Kernel team code "closes" the port when releasing it, (which clears * IFF_UP), so we must bring it back up here to ensure carrier changes and * other state is noticed by the now-released port. */ - if (!nm_device_bring_up(slave, TRUE, NULL)) { + if (!nm_device_bring_up(port, TRUE, NULL)) { _LOGW(LOGD_TEAM, - "released team port %s could not be brought up", - nm_device_get_ip_iface(slave)); + "detached team port %s could not be brought up", + nm_device_get_ip_iface(port)); } nm_clear_g_source(&priv->teamd_read_timeout); priv->teamd_read_timeout = g_timeout_add_seconds(5, teamd_read_timeout_cb, self); } else - _LOGI(LOGD_TEAM, "team port %s was released", nm_device_get_ip_iface(slave)); + _LOGI(LOGD_TEAM, "team port %s was detached", nm_device_get_ip_iface(port)); /* Delete any port configuration we previously set */ if (configure && priv->tdc - && (s_port = nm_device_get_applied_setting(slave, NM_TYPE_SETTING_TEAM_PORT)) + && (s_port = nm_device_get_applied_setting(port, NM_TYPE_SETTING_TEAM_PORT)) && (nm_setting_team_port_get_config(s_port))) - teamdctl_port_config_update_raw(priv->tdc, nm_device_get_ip_iface(slave), "{}"); + teamdctl_port_config_update_raw(priv->tdc, nm_device_get_ip_iface(port), "{}"); } static gboolean @@ -1064,8 +1064,8 @@ nm_device_team_class_init(NMDeviceTeamClass *klass) device_class->act_stage1_prepare = act_stage1_prepare; device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired; device_class->deactivate = deactivate; - device_class->enslave_slave = enslave_slave; - device_class->release_slave = release_slave; + device_class->attach_port = attach_port; + device_class->detach_port = detach_port; obj_properties[PROP_CONFIG] = g_param_spec_string(NM_DEVICE_TEAM_CONFIG, "", From 9fcbc6b37deccaa0bfcc9bddec726e41d5df5ced Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 27 Apr 2022 16:27:24 +0200 Subject: [PATCH 3/5] device: make attach_port() asynchronous For some device types the attach-port operation doesn't complete immediately. NMDevice needs to wait that the operation completes before proceeding (for example, before starting stage3 for the port). Change attach_port() so that it can return TERNARY_DEFAULT to indicate that the operation will complete asynchronously. Most of devices are not affected by this and can continue returning TRUE/FALSE as before without callback. --- src/core/devices/nm-device-bond.c | 10 +- src/core/devices/nm-device-bridge.c | 10 +- src/core/devices/nm-device-vrf.c | 10 +- src/core/devices/nm-device.c | 114 +++++++++++++------- src/core/devices/nm-device.h | 17 ++- src/core/devices/ovs/nm-device-ovs-bridge.c | 10 +- src/core/devices/ovs/nm-device-ovs-port.c | 10 +- src/core/devices/team/nm-device-team.c | 10 +- 8 files changed, 136 insertions(+), 55 deletions(-) diff --git a/src/core/devices/nm-device-bond.c b/src/core/devices/nm-device-bond.c index 80400afcc4..d5cb158a1b 100644 --- a/src/core/devices/nm-device-bond.c +++ b/src/core/devices/nm-device-bond.c @@ -424,8 +424,14 @@ commit_port_options(NMDevice *bond_device, NMDevice *port, NMSettingBondPort *s_ queue_id_str); } -static gboolean -attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) +static NMTernary +attach_port(NMDevice *device, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceAttachPortCallback callback, + gpointer user_data) { NMDeviceBond *self = NM_DEVICE_BOND(device); NMSettingBondPort *s_port; diff --git a/src/core/devices/nm-device-bridge.c b/src/core/devices/nm-device-bridge.c index 796e58c650..7b7053296c 100644 --- a/src/core/devices/nm-device-bridge.c +++ b/src/core/devices/nm-device-bridge.c @@ -974,8 +974,14 @@ deactivate(NMDevice *device) } } -static gboolean -attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) +static NMTernary +attach_port(NMDevice *device, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceAttachPortCallback callback, + gpointer user_data) { NMDeviceBridge *self = NM_DEVICE_BRIDGE(device); NMConnection *master_connection; diff --git a/src/core/devices/nm-device-vrf.c b/src/core/devices/nm-device-vrf.c index 6454d4d30d..2aef0e3d1e 100644 --- a/src/core/devices/nm-device-vrf.c +++ b/src/core/devices/nm-device-vrf.c @@ -206,8 +206,14 @@ update_connection(NMDevice *device, NMConnection *connection) g_object_set(G_OBJECT(s_vrf), NM_SETTING_VRF_TABLE, priv->props.table, NULL); } -static gboolean -attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) +static NMTernary +attach_port(NMDevice *device, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceAttachPortCallback callback, + gpointer user_data) { NMDeviceVrf *self = NM_DEVICE_VRF(device); gboolean success = TRUE; diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 93f9e7fd2f..a1a024cef3 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -120,11 +120,12 @@ typedef enum _nm_packed { } AddrMethodState; typedef struct { - CList lst_slave; - NMDevice *slave; - gulong watch_id; - bool slave_is_enslaved; - bool configure; + CList lst_slave; + NMDevice *slave; + GCancellable *cancellable; + gulong watch_id; + bool slave_is_enslaved; + bool configure; } SlaveInfo; typedef struct { @@ -5927,43 +5928,16 @@ find_slave_info(NMDevice *self, NMDevice *slave) return NULL; } -/** - * nm_device_master_enslave_slave: - * @self: the master device - * @slave: the slave device to enslave - * @connection: (allow-none): the slave device's connection - * - * If @self is capable of enslaving other devices (ie it's a bridge, bond, team, - * etc) then this function enslaves @slave. - * - * Returns: %TRUE on success, %FALSE on failure or if this device cannot enslave - * other devices. - */ -static gboolean -nm_device_master_enslave_slave(NMDevice *self, NMDevice *slave, NMConnection *connection) +static void +attach_port_done(NMDevice *self, NMDevice *slave, gboolean success) { SlaveInfo *info; - gboolean success = FALSE; - gboolean configure; - - g_return_val_if_fail(self != NULL, FALSE); - g_return_val_if_fail(slave != NULL, FALSE); - g_return_val_if_fail(NM_DEVICE_GET_CLASS(self)->attach_port != NULL, FALSE); info = find_slave_info(self, slave); if (!info) - return FALSE; + return; - if (info->slave_is_enslaved) - success = TRUE; - else { - configure = (info->configure && connection != NULL); - if (configure) - g_return_val_if_fail(nm_device_get_state(slave) >= NM_DEVICE_STATE_DISCONNECTED, FALSE); - - success = NM_DEVICE_GET_CLASS(self)->attach_port(self, slave, connection, configure); - info->slave_is_enslaved = success; - } + info->slave_is_enslaved = success; nm_device_slave_notify_enslave(info->slave, success); @@ -5983,8 +5957,71 @@ nm_device_master_enslave_slave(NMDevice *self, NMDevice *slave, NMConnection *co */ if (success) nm_device_activate_schedule_stage3_ip_config(self, FALSE); +} - return success; +static void +attach_port_cb(NMDevice *self, GError *error, gpointer user_data) +{ + NMDevice *slave = user_data; + SlaveInfo *info; + + if (nm_utils_error_is_cancelled(error)) + return; + + info = find_slave_info(self, slave); + if (!info) + return; + + nm_clear_g_cancellable(&info->cancellable); + attach_port_done(self, slave, !error); +} + +/** + * nm_device_master_enslave_slave: + * @self: the master device + * @slave: the slave device to enslave + * @connection: (allow-none): the slave device's connection + * + * If @self is capable of enslaving other devices (ie it's a bridge, bond, team, + * etc) then this function enslaves @slave. + */ +static void +nm_device_master_enslave_slave(NMDevice *self, NMDevice *slave, NMConnection *connection) +{ + SlaveInfo *info; + NMTernary success; + gboolean configure; + + g_return_if_fail(self); + g_return_if_fail(slave); + g_return_if_fail(NM_DEVICE_GET_CLASS(self)->attach_port); + + info = find_slave_info(self, slave); + if (!info) + return; + + if (info->slave_is_enslaved) + success = TRUE; + else { + configure = (info->configure && connection != NULL); + if (configure) + g_return_if_fail(nm_device_get_state(slave) >= NM_DEVICE_STATE_DISCONNECTED); + + nm_clear_g_cancellable(&info->cancellable); + info->cancellable = g_cancellable_new(); + success = NM_DEVICE_GET_CLASS(self)->attach_port(self, + slave, + connection, + configure, + info->cancellable, + attach_port_cb, + slave); + + if (success == NM_TERNARY_DEFAULT) + return; + } + + attach_port_done(self, slave, success); } /** @@ -6038,6 +6075,7 @@ nm_device_master_release_slave(NMDevice *self, g_return_if_fail(self == slave_priv->master); nm_assert(slave == info->slave); + nm_clear_g_cancellable(&info->cancellable); /* first, let subclasses handle the release ... */ if (info->slave_is_enslaved || nm_device_sys_iface_state_is_external(slave) @@ -7609,7 +7647,7 @@ nm_device_master_add_slave(NMDevice *self, NMDevice *slave, gboolean configure) g_return_val_if_fail(NM_IS_DEVICE(self), FALSE); g_return_val_if_fail(NM_IS_DEVICE(slave), FALSE); - g_return_val_if_fail(NM_DEVICE_GET_CLASS(self)->attach_port != NULL, FALSE); + g_return_val_if_fail(NM_DEVICE_GET_CLASS(self)->attach_port, FALSE); priv = NM_DEVICE_GET_PRIVATE(self); slave_priv = NM_DEVICE_GET_PRIVATE(slave); diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index bfa274d69e..377c2fb6b8 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -163,6 +163,7 @@ typedef enum { } NMDeviceCheckDevAvailableFlags; typedef void (*NMDeviceDeactivateCallback)(NMDevice *self, GError *error, gpointer user_data); +typedef void (*NMDeviceAttachPortCallback)(NMDevice *self, GError *error, gpointer user_data); typedef struct _NMDeviceClass { NMDBusObjectClass parent; @@ -373,11 +374,17 @@ typedef struct _NMDeviceClass { NMConnection *connection, GError **error); - gboolean (*attach_port)(NMDevice *self, - NMDevice *port, - NMConnection *connection, - gboolean configure); - + /* Attachs a port asynchronously. Returns TRUE/FALSE on immediate + * success/error; in such cases, the callback is not invoked. If the + * action couldn't be completed immediately, DEFAULT is returned and + * the callback will always be invoked asynchronously. */ + NMTernary (*attach_port)(NMDevice *self, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceAttachPortCallback callback, + gpointer user_data); void (*detach_port)(NMDevice *self, NMDevice *port, gboolean configure); void (*parent_changed_notify)(NMDevice *self, diff --git a/src/core/devices/ovs/nm-device-ovs-bridge.c b/src/core/devices/ovs/nm-device-ovs-bridge.c index da0a830e5d..bea6e77bc8 100644 --- a/src/core/devices/ovs/nm-device-ovs-bridge.c +++ b/src/core/devices/ovs/nm-device-ovs-bridge.c @@ -78,8 +78,14 @@ act_stage3_ip_config(NMDevice *device, int addr_family) nm_device_devip_set_state(device, addr_family, NM_DEVICE_IP_STATE_READY, NULL); } -static gboolean -attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) +static NMTernary +attach_port(NMDevice *device, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceAttachPortCallback callback, + gpointer user_data) { if (!configure) return TRUE; diff --git a/src/core/devices/ovs/nm-device-ovs-port.c b/src/core/devices/ovs/nm-device-ovs-port.c index 72c534ebf0..86f5e6bec6 100644 --- a/src/core/devices/ovs/nm-device-ovs-port.c +++ b/src/core/devices/ovs/nm-device-ovs-port.c @@ -115,8 +115,14 @@ set_mtu_cb(GError *error, gpointer user_data) g_object_unref(self); } -static gboolean -attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) +static NMTernary +attach_port(NMDevice *device, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceEnslaveSlaveCallback callback, + gpointer user_data) { NMDeviceOvsPort *self = NM_DEVICE_OVS_PORT(device); NMActiveConnection *ac_port = NULL; diff --git a/src/core/devices/team/nm-device-team.c b/src/core/devices/team/nm-device-team.c index 77a259d413..1f098806d3 100644 --- a/src/core/devices/team/nm-device-team.c +++ b/src/core/devices/team/nm-device-team.c @@ -790,8 +790,14 @@ deactivate(NMDevice *device) teamd_cleanup(self, TRUE); } -static gboolean -attach_port(NMDevice *device, NMDevice *port, NMConnection *connection, gboolean configure) +static NMTernary +attach_port(NMDevice *device, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceAttachPortCallback callback, + gpointer user_data) { NMDeviceTeam *self = NM_DEVICE_TEAM(device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE(self); From c503f5b2144478cd9449d407c94274a2ab523988 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 2 May 2022 15:13:22 +0200 Subject: [PATCH 4/5] ovs: attach port asynchronously The attach operation needs to be asynchronous as we should wait the result from ovsdb. https://bugzilla.redhat.com/show_bug.cgi?id=2052441 --- src/core/devices/ovs/nm-device-ovs-port.c | 82 ++++++++++++++++------- 1 file changed, 56 insertions(+), 26 deletions(-) diff --git a/src/core/devices/ovs/nm-device-ovs-port.c b/src/core/devices/ovs/nm-device-ovs-port.c index 86f5e6bec6..a64314dd0c 100644 --- a/src/core/devices/ovs/nm-device-ovs-port.c +++ b/src/core/devices/ovs/nm-device-ovs-port.c @@ -72,20 +72,42 @@ act_stage3_ip_config(NMDevice *device, int addr_family) nm_device_devip_set_state(device, addr_family, NM_DEVICE_IP_STATE_READY, NULL); } +typedef struct { + NMDevice *device; + NMDevice *port; + GCancellable *cancellable; + NMDeviceAttachPortCallback callback; + gpointer callback_user_data; +} AttachPortData; + static void add_iface_cb(GError *error, gpointer user_data) { - NMDevice *slave = user_data; + AttachPortData *data = user_data; + NMDeviceOvsPort *self; + gs_free_error GError *local = NULL; - if (error && !g_error_matches(error, NM_UTILS_ERROR, NM_UTILS_ERROR_CANCELLED_DISPOSING)) { - nm_log_warn(LOGD_DEVICE, - "device %s could not be added to a ovs port: %s", - nm_device_get_iface(slave), - error->message); - nm_device_state_changed(slave, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_OVSDB_FAILED); + if (g_cancellable_is_cancelled(data->cancellable)) { + local = nm_utils_error_new_cancelled(FALSE, NULL); + error = local; + } else if (error && !nm_utils_error_is_cancelled_or_disposing(error)) { + self = NM_DEVICE_OVS_PORT(data->device); + _LOGW(LOGD_DEVICE, + "device %s could not be added to a ovs port: %s", + nm_device_get_iface(data->port), + error->message); + nm_device_state_changed(data->port, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_OVSDB_FAILED); } - g_object_unref(slave); + data->callback(data->device, error, data->callback_user_data); + + g_object_unref(data->device); + g_object_unref(data->port); + nm_clear_g_cancellable(&data->cancellable); + + nm_g_slice_free(data); } static gboolean @@ -116,19 +138,20 @@ set_mtu_cb(GError *error, gpointer user_data) } static NMTernary -attach_port(NMDevice *device, - NMDevice *port, - NMConnection *connection, - gboolean configure, - GCancellable *cancellable, - NMDeviceEnslaveSlaveCallback callback, - gpointer user_data) +attach_port(NMDevice *device, + NMDevice *port, + NMConnection *connection, + gboolean configure, + GCancellable *cancellable, + NMDeviceAttachPortCallback callback, + gpointer user_data) { NMDeviceOvsPort *self = NM_DEVICE_OVS_PORT(device); NMActiveConnection *ac_port = NULL; NMActiveConnection *ac_bridge = NULL; NMDevice *bridge_device; NMSettingWired *s_wired; + AttachPortData *data; if (!configure) return TRUE; @@ -148,6 +171,15 @@ attach_port(NMDevice *device, return FALSE; } + data = g_slice_new(AttachPortData); + *data = (AttachPortData){ + .device = g_object_ref(device), + .port = g_object_ref(port), + .cancellable = g_object_ref(cancellable), + .callback = callback, + .callback_user_data = user_data, + }; + nm_ovsdb_add_interface(nm_ovsdb_get(), nm_active_connection_get_applied_connection(ac_bridge), nm_device_get_applied_connection(device), @@ -155,24 +187,22 @@ attach_port(NMDevice *device, bridge_device, port, add_iface_cb, - g_object_ref(port)); + data); /* DPDK ports does not have a link after the devbind, so the MTU must be * set on ovsdb after adding the interface. */ if (NM_IS_DEVICE_OVS_INTERFACE(port) && _ovs_interface_is_dpdk(port)) { s_wired = nm_device_get_applied_setting(port, NM_TYPE_SETTING_WIRED); - - if (!s_wired || !nm_setting_wired_get_mtu(s_wired)) - return TRUE; - - nm_ovsdb_set_interface_mtu(nm_ovsdb_get(), - nm_device_get_ip_iface(port), - nm_setting_wired_get_mtu(s_wired), - set_mtu_cb, - g_object_ref(port)); + if (s_wired && nm_setting_wired_get_mtu(s_wired)) { + nm_ovsdb_set_interface_mtu(nm_ovsdb_get(), + nm_device_get_ip_iface(port), + nm_setting_wired_get_mtu(s_wired), + set_mtu_cb, + g_object_ref(port)); + } } - return TRUE; + return NM_TERNARY_DEFAULT; } static void From af9ed3eb2feffefeab40ae706aa4c6f3a3caee86 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 3 May 2022 12:59:39 +0200 Subject: [PATCH 5/5] ovs: add FIXME about cancellable operations --- src/core/devices/ovs/nm-ovsdb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/devices/ovs/nm-ovsdb.c b/src/core/devices/ovs/nm-ovsdb.c index 44e16cb78f..7fa394d67d 100644 --- a/src/core/devices/ovs/nm-ovsdb.c +++ b/src/core/devices/ovs/nm-ovsdb.c @@ -376,6 +376,9 @@ ovsdb_call_method(NMOvsdb *self, NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE(self); OvsdbMethodCall *call; + /* FIXME(shutdown): this function should accept a cancellable to + * interrupt the operation. */ + /* Ensure we're not unsynchronized before we queue the method call. */ ovsdb_try_connect(self);