device: only deactivate when the master we've enslaved to goes away

Sometimes weird things happen.

Let dummy0 be an externally created device that has a master. We decide
to activate a connection that has no master on it:

  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.

  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.

  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 stage1 cleans the device up, removing it from the master.

  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'

We decide to deal with this by enqueuing a deactivation. That is not
great -- we shouldn't even have had this master!

This patch takes the deactivation path only if we were willingly
enslaved to the master in question.
This commit is contained in:
Lubomir Rintel 2022-06-02 13:26:10 +02:00
parent 0fa8c5f94c
commit 1fe8166fc9

View file

@ -7920,6 +7920,9 @@ nm_device_slave_notify_release(NMDevice *self, NMDeviceStateReason reason)
g_return_if_fail(priv->master);
if (!priv->is_enslaved)
return;
if (priv->state > NM_DEVICE_STATE_DISCONNECTED && priv->state <= NM_DEVICE_STATE_ACTIVATED) {
switch (nm_device_state_reason_check(reason)) {
case NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED:
@ -7949,14 +7952,12 @@ nm_device_slave_notify_release(NMDevice *self, NMDeviceStateReason reason)
} else
_LOGI(LOGD_DEVICE, "released from master device %s", nm_device_get_iface(priv->master));
if (priv->is_enslaved) {
priv->is_enslaved = FALSE;
priv->is_enslaved = FALSE;
_notify(self, PROP_MASTER);
_notify(self, PROP_MASTER);
nm_clear_pointer(&NM_DEVICE_GET_PRIVATE(priv->master)->ports_variant, g_variant_unref);
nm_gobject_notify_together(priv->master, PROP_PORTS, PROP_SLAVES);
}
nm_clear_pointer(&NM_DEVICE_GET_PRIVATE(priv->master)->ports_variant, g_variant_unref);
nm_gobject_notify_together(priv->master, PROP_PORTS, PROP_SLAVES);
}
/**