device: release slaves when an external device is going managedlr/cancel-ip-state-with-activation

When we're deactivating an externally created device that has a master
because we're activating a connection on it, actually remove the device
from the master. Otherwise unpleasant things happen:

  active-connection[0x55ed7ba78400]: constructed (NMActRequest, version-id 4, type managed)
  device[0a458361f9fed8f5] (dummy0): sys-iface-state: external -> managed
  device[0a458361f9fed8f5] (dummy0): queue activation request waiting for currently active connection to disconnect
  device (dummy0): disconnecting for new activation request.
  device (dummy0): state change: activated -> deactivating (reason 'new-activation', sys-iface-state: 'managed')
  device (br0): master: release one slave 0a458361f9fed8f5/dummy0 (enslaved)(no-config)

Note the "no-config" above. We'set priv->master = NULL, but didn't
communicate the change to the platform. I believe this is not good.
This patch changes that.

  device (br0): bridge port dummy0 was detached
  device (dummy0): released from master device br0
  active-connection[0x55ed7ba782e0]: set state deactivating (was activated)
  device (dummy0): ip4: set state none (was done, reason: ip-state-clear)
  device (dummy0): ip6: set state none (was done, reason: ip-state-clear)
  device (dummy0): state change: deactivating -> disconnected (reason 'new-activation', sys-iface-state: 'managed')

  platform: (dummy0) emit signal link-changed changed: 102: dummy0
      <NOARP,UP,LOWER_UP;broadcast,noarp,up,running,lowerup> mtu 1500 master 101 arp 1 dummy* init
       addrgenmode none addr EA:8D:DD:DF:1F:B7 brd FF:FF:FF:FF:FF:FF driver dummy rx:0,0 tx:39,4746

Now the platform sent us a new link, the "master" property is still set.

  device[0a458361f9fed8f5] (dummy0): queued link change for ifindex 102
  device[0a458361f9fed8f5] (dummy0): deactivating device (reason 'new-activation') [60]
  device (dummy0): ip: set (combined) state none (was done, reason: ip-state-clear)
  config: device-state: write #102 (/run/NetworkManager/devices/102); managed=managed, perm-hw-addr-fake=EA:8D:DD:DF:1F:B7, route-metric-default=0-0
  active-connection[0x55ed7ba782e0]: set state deactivated (was deactivating)
  active-connection[0x55ed7ba782e0]: check-master-ready: already signalled (state deactivated, master 0x55ed7ba781c0 is in state activated)
  device (dummy0): Activation: starting connection 'dummy1' (ec6fca51-84e6-4a5b-a297-f602252c9f69)
  device[0a458361f9fed8f5] (dummy0): activation-stage: schedule activate_stage1_device_prepare

  l3cfg[ae290b5c1f585d6c,ifindex=102]: emit signal (platform-change-on-idle, obj-type-flags=0x2a)
  device (br0): master: add one slave 0a458361f9fed8f5/dummy0

Amidst the new activation we're processing the netlink message we got.
We set priv->master back, effectively nullifying the release above. Sad.

  device (dummy0): state change: disconnected -> prepare (reason 'none', sys-iface-state: 'managed')
  device[0a458361f9fed8f5] (dummy0): add_pending_action (2): 'in-state-change'
  active-connection[0x55ed7ba78400]: set state activating (was unknown)
  manager: NetworkManager state is now CONNECTING
  active-connection[0x55ed7ba78400]: check-master-ready: not signalling (state activating, no master)
  device[8fff58d61c7686ce] (br0): slave dummy0 state change 30 (disconnected) -> 40 (prepare)
  device[0a458361f9fed8f5] (dummy0): remove_pending_action (1): 'in-state-change'
  device (br0): master: release one slave 0a458361f9fed8f5/dummy0 (not enslaved) (force-configure)
  platform: (dummy0) link: releasing 102 from master 'br0' (101)
  device (br0): detached bridge port dummy0

Now things go south. The stage1 cleans the device up, removing it from
the master and the device itself decides it should deactivate itself
because it lots its master regardless of the fact that it should not
have one and it's in fact an unwanted carryover from previous activation.
I believe this is also wrong.

  device[0a458361f9fed8f5] (dummy0): Activation: connection 'dummy1' master deactivated
  device (dummy0): ip4: set state none (was pending, reason: ip-state-clear)
  device (dummy0): ip6: set state none (was pending, reason: ip-state-clear)
  device[0a458361f9fed8f5] (dummy0): add_pending_action (2): 'queued-state-change-deactivating'
  device[0a458361f9fed8f5] (dummy0): queue-state[deactivating, reason:connection-assumed, id:298]: queue state change
  device[0a458361f9fed8f5] (dummy0): activation-stage: synchronously invoke activate_stage2_device_config
  device (dummy0): state change: prepare -> config (reason 'none', sys-iface-state: 'managed')

Now things are really weird. We synchronously go to config, effectively
overriding the queued deactivation. We've really messed up.

1 files changed, 3 insertions, 2 deletions

diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index fb8d6f177f..3795e98d06 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -7656,8 +7656,9 @@ slave_state_changed(NMDevice           *slave,
     }
 
     if (release) {
-        configure = priv->sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_MANAGED
-                    && nm_device_sys_iface_state_get(slave) != NM_DEVICE_SYS_IFACE_STATE_EXTERNAL;
+        configure = (priv->sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_MANAGED
+                     && nm_device_sys_iface_state_get(slave) != NM_DEVICE_SYS_IFACE_STATE_EXTERNAL)
+                    || nm_device_sys_iface_state_get(slave) == NM_DEVICE_SYS_IFACE_STATE_MANAGED;
 
         nm_device_master_release_slave(self,
                                        slave,