wireless: fix premature removal of hotplug devices due to down state

When a device is added that isn't up, status toggles can sometimes lead to a
DEV_EVENT_REMOVE event, which causes the device to be removed from an interface
or a bridge.
Fix this by toggling the dev->disabled status instead based on IFF_UP, and
adding a check to bridge/interface code to only permanently remove devices that
are actually gone.

Fixes: 516ab774cc16 ("system-linux: fix race condition on bringing up wireless devices")
Signed-off-by: Felix Fietkau <nbd@nbd.name>

diff --git a/bonding.c b/bonding.c
index 1f13148fea41c929bb5dcf5db6771de722d6f072..532308cb94468baf054c018781a8b3af699b8828 100644 (file)
--- a/bonding.c
+++ b/bonding.c
@@ -303,7 +303,7 @@ bonding_port_cb(struct device_user *dep, enum device_event ev)
                bonding_disable_port(bp, true);
                break;
        case DEV_EVENT_REMOVE:
-               if (dep->hotplug) {
+               if (dep->hotplug && !dev->sys_present) {
                        vlist_delete(&bdev->ports, &bp->node);
                        return;
                }

diff --git a/bridge.c b/bridge.c
index 7a633abe3f27965205033d0d01f9a9f567b77b4d..dd4d2c1c896e989faf90f759a9a71d6cdad84b12 100644 (file)
--- a/bridge.c
+++ b/bridge.c
@@ -794,7 +794,7 @@ bridge_member_cb(struct device_user *dep, enum device_event ev)
                bridge_disable_member(bm, true);
                break;
        case DEV_EVENT_REMOVE:
-               if (dep->hotplug) {
+               if (dep->hotplug && !dev->sys_present) {
                        vlist_delete(&bst->members, &bm->node);
                        return;
                }

diff --git a/device.c b/device.c
index e26edbbc2d223030b14d0f10a711558ffbc182c9..1370335ce3fc6ba836ad713465fcfc57a9cb88c3 100644 (file)
--- a/device.c
+++ b/device.c
@@ -880,9 +880,9 @@ void device_cleanup(struct device *dev)
        device_delete(dev);
 }
 
-static void __device_set_present(struct device *dev, bool state)
+static void __device_set_present(struct device *dev, bool state, bool force)
 {
-       if (dev->present == state)
+       if (dev->present == state && !force)
                return;
 
        dev->present = state;
@@ -897,7 +897,7 @@ device_refresh_present(struct device *dev)
        if (dev->disabled || dev->deferred)
                state = false;
 
-       __device_set_present(dev, state);
+       __device_set_present(dev, state, false);
 }
 
 void
@@ -937,7 +937,10 @@ void device_set_present(struct device *dev, bool state)
 
        D(DEVICE, "%s '%s' %s present\n", dev->type->name, dev->ifname, state ? "is now" : "is no longer" );
        dev->sys_present = state;
-       device_refresh_present(dev);
+       if (!state)
+               __device_set_present(dev, state, true);
+       else
+               device_refresh_present(dev);
        if (!state)
                safe_list_for_each(&dev->users, device_release_cb, NULL);
 }

diff --git a/interface.c b/interface.c
index b2c12300106d9b7c347a5a7b8afc1bfdcd762be2..03c781b516776cd35a2c3408c766513e0e57a4c3 100644 (file)
--- a/interface.c
+++ b/interface.c
@@ -437,7 +437,7 @@ interface_main_dev_cb(struct device_user *dep, enum device_event ev)
                break;
        case DEV_EVENT_REMOVE:
                interface_set_available(iface, false);
-               if (dep->dev && dep->dev->external)
+               if (dep->dev && dep->dev->external && !dep->dev->sys_present)
                        interface_set_main_dev(iface, NULL);
                break;
        case DEV_EVENT_UP:

diff --git a/system-linux.c b/system-linux.c
index 8efb020e78d22ef4054961b00766915f20a2821e..21110c552157268e18514fa25e1590f9e5a31af4 100644 (file)
--- a/system-linux.c
+++ b/system-linux.c
@@ -700,12 +700,10 @@ static void
 system_device_update_state(struct device *dev, unsigned int flags, unsigned int ifindex)
 {
        if (dev->type == &simple_device_type) {
-               bool present = ifindex > 0;
-
                if (dev->external)
-                       present = present && (flags & IFF_UP);
+                       device_set_disabled(dev, !(flags & IFF_UP));
 
-               device_set_present(dev, present);
+               device_set_present(dev, ifindex > 0);
        }
        device_set_link(dev, flags & IFF_LOWER_UP ? true : false);
 }