[OpenWrt-Devel] [PATCH v2 netifd 1/3] device: Fix device find failure in avl list due to device name change

[Hans Dedecker]

As device name is used as key in avl list a device name change will break the avl find logic.
Function device_set_ifname offers api to set the device name and re-inserts the avl node in the list
when the avl key value is changed.

Signed-off-by: Hans Dedecker <dedeckeh at gmail.com>
---
v1 -> v2 : Fix debug print in device_init_virtual

 alias.c  |  9 +++------
 device.c | 29 +++++++++++++++++++++++++----
 device.h |  1 +
 vlan.c   | 11 +++++++----
 4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/alias.c b/alias.c
index e4bb700..6b938ca 100644
--- a/alias.c
+++ b/alias.c
@@ -59,13 +59,10 @@ static void alias_set_device(struct alias_device *alias, struct device *dev)
 	alias->dev.hidden = !dev;
 	if (dev) {
 		device_set_ifindex(&alias->dev, dev->ifindex);
-		strcpy(alias->dev.ifname, dev->ifname);
-		device_broadcast_event(&alias->dev, DEV_EVENT_UPDATE_IFNAME);
+		device_set_ifname(&alias->dev, dev->ifname);
 		device_add_user(&alias->dep, dev);
-	} else {
-		alias->dev.ifname[0] = 0;
-		device_broadcast_event(&alias->dev, DEV_EVENT_UPDATE_IFNAME);
-	}
+	} else
+		device_set_ifname(&alias->dev, "");
 }
 
 static int
diff --git a/device.c b/device.c
index 06c4820..a4d804e 100644
--- a/device.c
+++ b/device.c
@@ -422,14 +422,14 @@ void device_init_virtual(struct device *dev, const struct device_type *type, con
 	assert(dev);
 	assert(type);
 
-	if (name)
-		strncpy(dev->ifname, name, IFNAMSIZ);
-
-	D(DEVICE, "Initialize device '%s'\n", dev->ifname);
+	D(DEVICE, "Initialize device '%s'\n", name ? name : "");
 	INIT_SAFE_LIST(&dev->users);
 	INIT_SAFE_LIST(&dev->aliases);
 	dev->type = type;
 
+	if (name)
+		device_set_ifname(dev, name);
+
 	if (!dev->set_state)
 		dev->set_state = set_device_state;
 }
@@ -582,6 +582,27 @@ void device_set_ifindex(struct device *dev, int ifindex)
 	device_broadcast_event(dev, DEV_EVENT_UPDATE_IFINDEX);
 }
 
+int device_set_ifname(struct device *dev, const char *name)
+{
+	int ret = 0;
+
+	if (!strcmp(dev->ifname, name))
+		return 0;
+
+	if (dev->avl.key)
+		avl_delete(&devices, &dev->avl);
+
+	strncpy(dev->ifname, name, IFNAMSIZ);
+
+	if (dev->avl.key)
+		ret = avl_insert(&devices, &dev->avl);
+
+	if (ret == 0)
+		device_broadcast_event(dev, DEV_EVENT_UPDATE_IFNAME);
+
+	return ret;
+}
+
 static int device_refcount(struct device *dev)
 {
 	struct list_head *list;
diff --git a/device.h b/device.h
index 77a2fef..b069a2b 100644
--- a/device.h
+++ b/device.h
@@ -250,6 +250,7 @@ void device_broadcast_event(struct device *dev, enum device_event ev);
 void device_set_present(struct device *dev, bool state);
 void device_set_link(struct device *dev, bool state);
 void device_set_ifindex(struct device *dev, int ifindex);
+int device_set_ifname(struct device *dev, const char *name);
 void device_refresh_present(struct device *dev);
 int device_claim(struct device_user *dep);
 void device_release(struct device_user *dep);
diff --git a/vlan.c b/vlan.c
index ac434ce..7f8697b 100644
--- a/vlan.c
+++ b/vlan.c
@@ -63,8 +63,11 @@ static int vlan_set_device_state(struct device *dev, bool up)
 
 static void vlan_dev_set_name(struct vlan_device *vldev, struct device *dev)
 {
+	char name[IFNAMSIZ];
+
 	vldev->dev.hidden = dev->hidden;
-	snprintf(vldev->dev.ifname, IFNAMSIZ, "%s.%d", dev->ifname, vldev->id);
+	snprintf(name, IFNAMSIZ, "%s.%d", dev->ifname, vldev->id);
+	device_set_ifname(&vldev->dev, name);
 }
 
 static void vlan_dev_cb(struct device_user *dep, enum device_event ev)
@@ -81,7 +84,6 @@ static void vlan_dev_cb(struct device_user *dep, enum device_event ev)
 		break;
 	case DEV_EVENT_UPDATE_IFNAME:
 		vlan_dev_set_name(vldev, dep->dev);
-		device_broadcast_event(&vldev->dev, ev);
 		break;
 	case DEV_EVENT_TOPO_CHANGE:
 		/* Propagate topo changes */
@@ -124,9 +126,10 @@ static struct device *get_vlan_device(struct device *dev, int id, bool create)
 		return NULL;
 
 	vldev->id = id;
-	vlan_dev_set_name(vldev, dev);
 
-	device_init(&vldev->dev, &vlan_type, vldev->dev.ifname);
+	device_init(&vldev->dev, &vlan_type, NULL);
+
+	vlan_dev_set_name(vldev, dev);
 	vldev->dev.default_config = true;
 
 	vldev->set_state = vldev->dev.set_state;
-- 
1.9.1
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel