device: fix hanging port devices when controller goes down while port is not fully attached

This partly reverts 1fe8166fc9fb ('device: only deactivate when the master
we've enslaved to goes away').

If the controller fails while the port is not yet fully attached,
before this patch the following happened:

  <info>  [1664299566.1065] device (bond0): state change: ip-config -> failed (reason 'config-failed', sys-iface-state: 'managed')
  ...
  <warn>  [1664299566.1073] device (bond0): Activation: failed for connection 'bond0'
  <trace> [1664299566.1073] device[6b76ac7314eb0b53] (bond0): master: release one slave a9f10ea824bb1725/eth1 (not enslaved) (configure)
  <debug> [1664299566.1073] device[a9f10ea824bb1725] (eth1): unmanaged: flags set to [!sleeping,!by-type,!platform-init,!user-explicit,!user-settings,!user-conf=0x0/0x179/managed], forget [is-slave=0x800], reason removed)
  ...
  <info>  [1664299566.1080] device (eth1): state change: config -> ip-config (reason 'none', sys-iface-state: 'managed')

Note that now eth1 has no controller, but it lingers in "ip-config" state indefinitely.

If we look at a case where the port is already attached we see:

  <info>  [1664299540.9661] device (bond0): state change: secondaries -> failed (reason 'config-failed', sys-iface-state: 'managed')
  ...
  <warn>  [1664299540.9667] device (bond0): Activation: failed for connection 'bond0'
  <trace> [1664299540.9667] device[6b76ac7314eb0b53] (bond0): master: release one slave a9f10ea824bb1725/eth1 (enslaved) (configure)
  <debug> [1664299540.9667] platform: (eth1) link: releasing 10 from master 'bond0' (80)
  ...
  <info>  [1664299540.9740] device (bond0): detached bond port eth1
  ...
  <debug> [1664299540.9749] device[a9f10ea824bb1725] (eth1): Activation: connection 'eth1' master failed
  ...
  <warn>  [1664299540.9749] device (eth1): queue-state[secondaries, reason:none, id:520]: replace previously queued state change
  ...
  <debug> [1664299540.9750] device[a9f10ea824bb1725] (eth1): queue-state[deactivating, reason:dependency-failed, id:533]: queue state change
  <debug> [1664299540.9751] device[a9f10ea824bb1725] (eth1): unmanaged: flags set to [!sleeping,!by-type,!platform-init,!user-explicit,!user-settings,!user-conf=0x0/0x179/managed], forget [is-slave=0x800], reason removed)
  ...
  <debug> [1664299541.0201] device[a9f10ea824bb1725] (eth1): enslaved to unknown device 0 (??)
  ...
  <debug> [1664299541.0227] device[a9f10ea824bb1725] (eth1): queue-state[deactivating, reason:dependency-failed, id:533]: change state
  <info>  [1664299541.0228] device (eth1): state change: ip-check -> deactivating (reason 'dependency-failed', sys-iface-state: 'managed')

Fix that by not ignoring the nm_device_slave_notify_release() call. Now we get:

  <info>  [1664391684.9757] device (bond0): state change: ip-config -> failed (reason 'config-failed', sys-iface-state: 'managed')
  ...
  <debug> [1664391684.9759] active-connection[69c2b12d61f5b171]: set state deactivated (was activating)
  <debug> [1664391684.9760] active-connection[142bb8240f6a696d]: check-master-ready: already signalled (state activating, master 0x56116f1480a0 is in state deactivated)
  ...
  <debug> [1664391684.9762] manager: ActivatingConnection now (none)
  ...
  <warn>  [1664391684.9763] device (bond0): Activation: failed for connection 'bond0'
  <trace> [1664391684.9763] device[142828814dec6e26] (bond0): master: release one slave 720791275fe8a68c/eth1 (not enslaved) (configure)
  <debug> [1664391684.9763] device[720791275fe8a68c] (eth1): Activation: connection 'eth1' master failed
  ...
  <debug> [1664391684.9764] device[720791275fe8a68c] (eth1): queue-state[deactivating, reason:dependency-failed, id:3047]: queue state change
  <debug> [1664391684.9765] device[720791275fe8a68c] (eth1): unmanaged: flags set to [!sleeping,!by-type,!platform-init,!user-explicit,!user-settings,!user-conf=0x0/0x179/managed], forget [is-slave=0x800], reason removed)
  ...
  <debug> [1664391684.9797] device[720791275fe8a68c] (eth1): queue-state[deactivating, reason:dependency-failed, id:3047]: change state
  <info>  [1664391684.9797] device (eth1): state change: config -> deactivating (reason 'dependency-failed', sys-iface-state: 'managed')

Commit 1fe8166fc9fb ('device: only deactivate when the master we've
enslaved to goes away') added the "return", but it seems to also add it
in cases where we need to handle this. Restrict the return to cases if
we do "no-config".

https://bugzilla.redhat.com/show_bug.cgi?id=2130287

Fixes: 1fe8166fc9fb ('device: only deactivate when the master we've enslaved to goes away')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1406

1 files changed, 8 insertions, 4 deletions

diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index eedbc31f3c..f6aa60ad45 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -778,7 +778,9 @@ static void _dev_l3_cfg_commit_type_reset(NMDevice *self);
 
 static gboolean nm_device_master_add_slave(NMDevice *self, NMDevice *slave, gboolean configure);
 static void     nm_device_slave_notify_enslave(NMDevice *self, gboolean success);
-static void     nm_device_slave_notify_release(NMDevice *self, NMDeviceStateReason reason);
+static void     nm_device_slave_notify_release(NMDevice           *self,
+                                               NMDeviceStateReason reason,
+                                               ReleaseSlaveType    release_type);
 
 static void _dev_ipll6_start(NMDevice *self);
 
@@ -6238,7 +6240,7 @@ nm_device_master_release_slave(NMDevice           *self,
                                                release_type >= RELEASE_SLAVE_TYPE_CONFIG);
 
     /* raise notifications about the release, including clearing is_enslaved. */
-    nm_device_slave_notify_release(slave, reason);
+    nm_device_slave_notify_release(slave, reason, release_type);
 
     /* keep both alive until the end of the function.
      * Transfers ownership from slave_priv->master.  */
@@ -8033,7 +8035,9 @@ nm_device_slave_notify_enslave(NMDevice *self, gboolean success)
  * Notifies a slave that it has been released, and why.
  */
 static void
-nm_device_slave_notify_release(NMDevice *self, NMDeviceStateReason reason)
+nm_device_slave_notify_release(NMDevice           *self,
+                               NMDeviceStateReason reason,
+                               ReleaseSlaveType    release_type)
 {
     NMDevicePrivate *priv       = NM_DEVICE_GET_PRIVATE(self);
     NMConnection    *connection = nm_device_get_applied_connection(self);
@@ -8041,7 +8045,7 @@ nm_device_slave_notify_release(NMDevice *self, NMDeviceStateReason reason)
 
     g_return_if_fail(priv->master);
 
-    if (!priv->is_enslaved)
+    if (!priv->is_enslaved && release_type == RELEASE_SLAVE_TYPE_NO_CONFIG)
         return;
 
     if (priv->state > NM_DEVICE_STATE_DISCONNECTED && priv->state <= NM_DEVICE_STATE_ACTIVATED) {