system/NetworkManager
system/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager synced 2024-07-22 10:46:59 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

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

Browse source 
This partly reverts 1fe8166fc9 ('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 1fe8166fc9 ('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: 1fe8166fc9 ('device: only deactivate when the master we've enslaved to goes away')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1406
This commit is contained in:
Thomas Haller 2022-09-28 21:00:07 +02:00
parent 44d99e366b
commit 2be9c693d9
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
1 changed files with 8 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/core/devices/nm-device.c
View file

@ -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) {