From 3d18c9c841aeaad831f609d2623d9e6482f45f65 Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Tue, 24 Jan 2023 12:30:45 +0100 Subject: [PATCH 1/4] device: do cleanup type removed if sys-iface-state is REMOVED When the state is DISCONNECTED is being set from a configuring/configured state we might want to always DECONFIGURE the interface (ifindex, ip addresses, routes..) except if the sys-iface-state is REMOVED in that case we would like to remove it. --- src/core/devices/nm-device.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 300c958743..4046fc2bfc 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -16105,8 +16105,11 @@ _set_state_full(NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, * userspace IPv6LL enabled. */ _dev_addrgenmode6_set(self, NM_IN6_ADDR_GEN_MODE_NONE); + if (priv->sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_REMOVED) { + nm_device_cleanup(self, reason, CLEANUP_TYPE_REMOVED); + } else + nm_device_cleanup(self, reason, CLEANUP_TYPE_DECONFIGURE); - nm_device_cleanup(self, reason, CLEANUP_TYPE_DECONFIGURE); } else if (old_state < NM_DEVICE_STATE_DISCONNECTED) { if (priv->sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_MANAGED) { /* Ensure IPv6 is set up as it may not have been done when From f00db8e15de589114fb1a68d16205a6d7dbc34e9 Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Tue, 24 Jan 2023 12:33:09 +0100 Subject: [PATCH 2/4] device: always queue recheck_assume before making device managed There were a few places where we did already this but there was one place where we missed it, in nm-manager.c:do_sleep_wake(). Therefore, the device end in DISCONNECTED state and did not assume the connection. --- src/core/devices/nm-device.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 4046fc2bfc..11f9001395 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -6495,13 +6495,6 @@ _dev_unmanaged_check_external_down(NMDevice *self, gboolean only_if_unmanaged, g } ext_flags = _dev_unmanaged_is_external_down(self, FALSE); - if (ext_flags != NM_UNMAN_FLAG_OP_SET_UNMANAGED) { - /* Ensure the assume check is queued before any queued state changes - * from the transition to UNAVAILABLE. - */ - nm_device_queue_recheck_assume(self); - } - if (now) { nm_device_set_unmanaged_by_flags(self, NM_UNMANAGED_EXTERNAL_DOWN, @@ -6966,12 +6959,6 @@ device_link_changed(gpointer user_data) } } - /* The assume check should happen before the device transitions to - * UNAVAILABLE, because in UNAVAILABLE we already clean up the IP - * configuration. Therefore, this function should never trigger a - * sync state transition. - */ - nm_device_queue_recheck_assume(self); nm_device_set_unmanaged_by_flags_queue(self, NM_UNMANAGED_PLATFORM_INIT, NM_UNMAN_FLAG_OP_SET_MANAGED, @@ -14701,6 +14688,15 @@ _set_unmanaged_flags(NMDevice *self, if (transition_state) { new_state = was_managed ? NM_DEVICE_STATE_UNMANAGED : NM_DEVICE_STATE_UNAVAILABLE; + if (new_state != NM_DEVICE_STATE_UNMANAGED) { + /* The assume check should happen before the device transitions to + * UNAVAILABLE, because in UNAVAILABLE we already clean up the IP + * configuration. Therefore, this function should never trigger a + * sync state transition. + */ + nm_device_queue_recheck_assume(self); + } + if (now) nm_device_state_changed(self, new_state, reason); else From 5a9a7623c5a4bae2001b30eb99e640e2e3285c3e Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Tue, 24 Jan 2023 12:37:14 +0100 Subject: [PATCH 3/4] core: set STATE_REASON_CONNECTION_ASSUMED when waking up Otherwise, external devices become managed and we clear IP addresses. --- src/core/nm-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 5e83a0c53e..b58079b59b 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -7077,7 +7077,7 @@ do_sleep_wake(NMManager *self, gboolean sleeping_changed) nm_device_set_unmanaged_by_flags(device, NM_UNMANAGED_SLEEPING, NM_UNMAN_FLAG_OP_SET_MANAGED, - NM_DEVICE_STATE_REASON_NOW_MANAGED); + NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); } /* Give the connections a chance to recreate the virtual devices. From 7f96d4d2cd9caa9f3f3a804919b31ce49f9dfdfa Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Mon, 6 Mar 2023 17:57:16 +0100 Subject: [PATCH 4/4] devices: drop wrong assertion on parent when ifindex is not present When creating a parent dependent device it can have software device as parent without an ifindex. In that case, it will fail on an ssertion on parent being missing. In order to avoid this, we are handling the situation similar to what we do for VLAN devices. NetworkManager will raise different error and block the autoconnection instead of asserting. This solves the assert error for the following commands: ``` nmcli connection add type macvlan ifname mv1 con-name mv1+ macvlan.parent dummy0 mode vepa nmcli connection add type dummy ifname dummy0 con-name dummy0+ autoconnect no ``` --- src/core/devices/nm-device-6lowpan.c | 14 +++++++++++--- src/core/devices/nm-device-macvlan.c | 14 +++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/core/devices/nm-device-6lowpan.c b/src/core/devices/nm-device-6lowpan.c index 870a1c14ef..f3386ddb58 100644 --- a/src/core/devices/nm-device-6lowpan.c +++ b/src/core/devices/nm-device-6lowpan.c @@ -72,14 +72,22 @@ create_and_realize(NMDevice *device, s_6lowpan = NM_SETTING_6LOWPAN(nm_connection_get_setting(connection, NM_TYPE_SETTING_6LOWPAN)); g_return_val_if_fail(s_6lowpan, FALSE); - parent_ifindex = parent ? nm_device_get_ifindex(parent) : 0; + if (!parent) { + g_set_error(error, + NM_DEVICE_ERROR, + NM_DEVICE_ERROR_MISSING_DEPENDENCIES, + "6LoWPAN device can not be created without a parent interface"); + return FALSE; + } + parent_ifindex = nm_device_get_ifindex(parent); if (parent_ifindex <= 0) { g_set_error(error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_MISSING_DEPENDENCIES, - "6LoWPAN devices can not be created without a parent interface"); - g_return_val_if_fail(!parent, FALSE); + "cannot retrieve ifindex of interface %s (%s)", + nm_device_get_iface(parent), + nm_device_get_type_desc(parent)); return FALSE; } diff --git a/src/core/devices/nm-device-macvlan.c b/src/core/devices/nm-device-macvlan.c index 3f57bfb1fc..06c0df07e9 100644 --- a/src/core/devices/nm-device-macvlan.c +++ b/src/core/devices/nm-device-macvlan.c @@ -208,14 +208,22 @@ create_and_realize(NMDevice *device, s_macvlan = nm_connection_get_setting_macvlan(connection); g_return_val_if_fail(s_macvlan, FALSE); - parent_ifindex = parent ? nm_device_get_ifindex(parent) : 0; + if (!parent) { + g_set_error(error, + NM_DEVICE_ERROR, + NM_DEVICE_ERROR_MISSING_DEPENDENCIES, + "MACVLAN device can not be created without a parent interface"); + return FALSE; + } + parent_ifindex = nm_device_get_ifindex(parent); if (parent_ifindex <= 0) { g_set_error(error, NM_DEVICE_ERROR, NM_DEVICE_ERROR_MISSING_DEPENDENCIES, - "MACVLAN devices can not be created without a parent interface"); - g_return_val_if_fail(!parent, FALSE); + "cannot retrieve ifindex of interface %s (%s)", + nm_device_get_iface(parent), + nm_device_get_type_desc(parent)); return FALSE; }