Wifi bug
Felix Fietkau
nbd at nbd.name
Mon Sep 27 09:17:40 PDT 2021
On 2021-09-27 17:45, Hannu Nyman wrote:
> Felix Fietkau kirjoitti 27.9.2021 klo 13.59:
>> On a crash, it should drop a .core file to /tmp. Please copy that to
>> your build host and use ./scripts/remote-gdb to obtain a backtrace from
>> it. I'd like to know, which line of code in netifd it crashes on, so I
>> can fix it. So far the bug has not shown up in my own tests...
>>
>> - Felix
>
>
> This is probably what you are looking for...
> To me it looks like it might actually be a list handling bug in libubox.Can you please try this netifd patch?
Thanks,
- Felix
---
diff --git a/alias.c b/alias.c
index 951e046bb3f1..98d54100fef9 100644
--- a/alias.c
+++ b/alias.c
@@ -178,13 +178,9 @@ alias_notify_device(const char *name, struct device *dev)
{
struct alias_device *alias;
- device_lock();
-
alias = avl_find_element(&aliases, name, alias, avl);
if (alias)
alias_set_device(alias, dev);
-
- device_unlock();
}
struct device *
diff --git a/bonding.c b/bonding.c
index 0bf4f9a331ef..457fe5159899 100644
--- a/bonding.c
+++ b/bonding.c
@@ -566,8 +566,6 @@ bonding_free_port(struct bonding_port *bp)
bonding_remove_port(bp);
- device_lock();
-
device_remove_user(&bp->dev);
/*
@@ -582,8 +580,6 @@ bonding_free_port(struct bonding_port *bp)
device_set_present(dev, true);
}
- device_unlock();
-
free(bp);
}
diff --git a/bridge.c b/bridge.c
index 2ce5c2b11b49..7e61b9df8326 100644
--- a/bridge.c
+++ b/bridge.c
@@ -512,8 +512,6 @@ restart:
goto restart;
}
- device_lock();
-
device_remove_user(&bm->dev);
uloop_timeout_cancel(&bm->check_timer);
@@ -529,8 +527,6 @@ restart:
device_set_present(dev, true);
}
- device_unlock();
-
free(bm);
}
diff --git a/config.c b/config.c
index d83ea9cb6b6c..9bbda39d3fb5 100644
--- a/config.c
+++ b/config.c
@@ -762,7 +762,6 @@ config_init_all(void)
vlist_update(&interfaces);
config_init = true;
- device_lock();
device_reset_config();
config_init_devices(true);
@@ -775,12 +774,10 @@ config_init_all(void)
config_init_wireless();
config_init = false;
- device_unlock();
device_reset_old();
device_init_pending();
vlist_flush(&interfaces);
- device_free_unused(NULL);
interface_refresh_assignments(false);
interface_start_pending();
wireless_start_pending();
diff --git a/device.c b/device.c
index bb39ea7f8d71..b3d0e85f8550 100644
--- a/device.c
+++ b/device.c
@@ -99,18 +99,6 @@ device_type_get(const char *tname)
return NULL;
}
-void device_lock(void)
-{
- __devlock++;
-}
-
-void device_unlock(void)
-{
- __devlock--;
- if (!__devlock)
- device_free_unused(NULL);
-}
-
static int device_vlan_len(struct kvlist *kv, const void *data)
{
return sizeof(unsigned int);
@@ -895,14 +883,27 @@ device_free(struct device *dev)
}
static void
-__device_free_unused(struct device *dev)
+__device_free_unused(struct uloop_timeout *timeout)
{
- if (!safe_list_empty(&dev->users) ||
- !safe_list_empty(&dev->aliases) ||
- dev->current_config || __devlock)
- return;
+ struct device *dev, *tmp;
+
+ avl_for_each_element_safe(&devices, dev, avl, tmp) {
+ if (!safe_list_empty(&dev->users) ||
+ !safe_list_empty(&dev->aliases) ||
+ dev->current_config)
+ continue;
+
+ device_free(dev);
+ }
+}
+
+void device_free_unused(void)
+{
+ static struct uloop_timeout free_timer = {
+ .cb = __device_free_unused,
+ };
- device_free(dev);
+ uloop_timeout_set(&free_timer, 1);
}
void device_remove_user(struct device_user *dep)
@@ -919,19 +920,7 @@ void device_remove_user(struct device_user *dep)
safe_list_del(&dep->list);
dep->dev = NULL;
D(DEVICE, "Remove user for device '%s', refcount=%d\n", dev->ifname, device_refcount(dev));
- __device_free_unused(dev);
-}
-
-void
-device_free_unused(struct device *dev)
-{
- struct device *tmp;
-
- if (dev)
- return __device_free_unused(dev);
-
- avl_for_each_element_safe(&devices, dev, avl, tmp)
- __device_free_unused(dev);
+ device_free_unused();
}
void
diff --git a/device.h b/device.h
index 0496e893cbc9..37f8c37c58a3 100644
--- a/device.h
+++ b/device.h
@@ -300,9 +300,6 @@ extern const struct uci_blob_param_list device_attr_list;
extern struct device_type simple_device_type;
extern struct device_type tunnel_device_type;
-void device_lock(void);
-void device_unlock(void);
-
void device_vlan_update(bool done);
void device_stp_init(void);
@@ -346,7 +343,7 @@ void device_release(struct device_user *dep);
int device_check_state(struct device *dev);
void device_dump_status(struct blob_buf *b, struct device *dev);
-void device_free_unused(struct device *dev);
+void device_free_unused(void);
struct device *get_vlan_device_chain(const char *ifname, int create);
void alias_notify_device(const char *name, struct device *dev);
diff --git a/extdev.c b/extdev.c
index 977d9c2fc7f9..5c5e76901d81 100644
--- a/extdev.c
+++ b/extdev.c
@@ -942,11 +942,9 @@ __create(const char *name, struct device_type *type, struct blob_attr *config)
inv_error:
extdev_invocation_error(ret, __extdev_methods[METHOD_CREATE], name);
error:
- device_lock();
free(edev->dev.config);
device_cleanup(&edev->dev);
free(edev);
- device_unlock();
netifd_log_message(L_WARNING, "Failed to create %s %s\n", type->name, name);
return NULL;
}
diff --git a/interface.c b/interface.c
index 2391e121badf..6cf0d309d5f5 100644
--- a/interface.c
+++ b/interface.c
@@ -637,8 +637,6 @@ interface_claim_device(struct interface *iface)
if (iface->parent_iface.iface)
interface_remove_user(&iface->parent_iface);
- device_lock();
-
if (iface->parent_ifname) {
parent = vlist_find(&interfaces, iface->parent_ifname, parent, node);
iface->parent_iface.cb = interface_alias_cb;
@@ -654,8 +652,6 @@ interface_claim_device(struct interface *iface)
if (dev)
interface_set_main_dev(iface, dev);
- device_unlock();
-
if (iface->proto_handler->flags & PROTO_FLAG_INIT_AVAILABLE)
interface_set_available(iface, true);
}
@@ -1087,30 +1083,19 @@ interface_handle_link(struct interface *iface, const char *name,
struct blob_attr *vlan, bool add, bool link_ext)
{
struct device *dev;
- int ret;
-
- device_lock();
dev = device_get(name, add ? (link_ext ? 2 : 1) : 0);
- if (!dev) {
- ret = UBUS_STATUS_NOT_FOUND;
- goto out;
- }
+ if (!dev)
+ return UBUS_STATUS_NOT_FOUND;
- if (add) {
- interface_set_device_config(iface, dev);
- if (!link_ext)
- device_set_present(dev, true);
+ if (!add)
+ return interface_remove_link(iface, dev, vlan);
- ret = interface_add_link(iface, dev, vlan, link_ext);
- } else {
- ret = interface_remove_link(iface, dev, vlan);
- }
+ interface_set_device_config(iface, dev);
+ if (!link_ext)
+ device_set_present(dev, true);
-out:
- device_unlock();
-
- return ret;
+ return interface_add_link(iface, dev, vlan, link_ext);
}
void
diff --git a/ubus.c b/ubus.c
index 56cce8163cef..4770cb686534 100644
--- a/ubus.c
+++ b/ubus.c
@@ -291,7 +291,7 @@ netifd_handle_alias(struct ubus_context *ctx, struct ubus_object *obj,
return 0;
error:
- device_free_unused(dev);
+ device_free_unused();
return UBUS_STATUS_INVALID_ARGUMENT;
}
More information about the openwrt-devel
mailing list