From 35facc8306f590a7330789ab6d5785c0d43073ef Mon Sep 17 00:00:00 2001 From: Felix Fietkau Date: Fri, 10 Nov 2023 15:34:23 +0100 Subject: [PATCH] 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 --- bonding.c | 2 +- bridge.c | 2 +- device.c | 11 +++++++---- interface.c | 2 +- system-linux.c | 6 ++---- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/bonding.c b/bonding.c index 1f13148..532308c 100644 --- 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 7a633ab..dd4d2c1 100644 --- 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 e26edbb..1370335 100644 --- 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 b2c1230..03c781b 100644 --- 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 8efb020..21110c5 100644 --- 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); } -- 2.30.2