Patchwork [v4,23/25] VLAN: ensure destroying interfaces goes reverse as creating

login
register
mail settings
Submitter michael-dev@fami-braun.de
Date July 27, 2013, 7:56 p.m.
Message ID <20130727195622.17627.56148.stgit@ray-controller>
Download mbox | patch
Permalink /patch/262525/
State Superseded
Headers show

Comments

michael-dev@fami-braun.de - July 27, 2013, 7:56 p.m.
I currently see
[kernel] unregister_netdevice: waiting for wl0_1.73 to become free. Usage count = 1
messages, thought wl0_1.73 is not in ip -f link a s or /sys/class/net .
iw dev wl0_1 station dump hangs

This patch tries to avoid this by not plugging the wl device out in the middle at first
but using the reverse procedure of creating for destruction.

Further, it fixes a race condition. Imagine
 1. STA removal triggers dynamic interface removal
 2. STA reconnects, finds VLAN still present but fails to set key
    (detecting the VLAN has been removed here would generate
     a NEWLINK messages after 3. but that would be pointless,
     due to 3. removing the vlan from hapd->conf->vlan, which
     is required for manual vlan interface removal)
 3. DELLINK is processed, removes the VLAN although in use

Signed-hostap: Michael Braun <michael-dev@fami-braun.de>
---
 src/ap/ap_config.h |    3 +++
 src/ap/vlan_init.c |   57 ++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 43 insertions(+), 17 deletions(-)

Patch

diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index deae005..d58a3d1 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -95,6 +95,9 @@  struct hostapd_vlan {
 	 * freed.
 	 */
 	int newlink_seen;
+	/* Make sure DELLINK processing does not affect newly created
+	 * interfaces. */
+	int ifidx;
 	int dynamic_vlan;
 #ifdef CONFIG_FULL_DYNAMIC_VLAN
 
diff --git a/src/ap/vlan_init.c b/src/ap/vlan_init.c
index 24663c4..3b7cdfb 100644
--- a/src/ap/vlan_init.c
+++ b/src/ap/vlan_init.c
@@ -653,14 +653,16 @@  static void vlan_newlink_vlan(const int vlan_id, char *ifname, int *clean,
 }
 
 
-static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
+static void vlan_newlink(char *ifname, int ifidx, struct hostapd_data *hapd)
 {
 	struct hostapd_vlan *vlan;
 	char vlan_ifname[IFNAMSIZ+1];
 	int i, ret;
 
 	for (vlan = hapd->conf->vlan; vlan; vlan = vlan->next) {
-		if (os_strcmp(ifname, vlan->ifname) != 0 || vlan->newlink_seen)
+		if (os_strcmp(ifname, vlan->ifname) != 0 ||
+		    vlan->newlink_seen ||
+		    (vlan->ifidx > 0 && ifidx > 0 && vlan->ifidx != ifidx))
 			continue;
 
 		wpa_printf(MSG_DEBUG, "VLAN: vlan_newlink(%s)", ifname);
@@ -698,7 +700,7 @@  static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 		}
 
 		vlan->newlink_seen = 1;
-
+		vlan->ifidx = ifidx;
 		break;
 	}
 }
@@ -760,7 +762,7 @@  static void vlan_dellink_vlan(int vlan_id, char* ifname, int *clean,
 }
 
 
-static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
+static void vlan_dellink(char *ifname, int ifidx, struct hostapd_data *hapd)
 {
 	struct hostapd_vlan *first, *prev, *vlan;
 	char vlan_ifname[IFNAMSIZ];
@@ -768,7 +770,8 @@  static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
 
 	for (first = prev = vlan = hapd->conf->vlan; vlan;
 	     prev = vlan, vlan = vlan->next) {
-		if (os_strcmp(ifname, vlan->ifname) != 0)
+		if (os_strcmp(ifname, vlan->ifname) != 0 ||
+		    (vlan->ifidx > 0 && ifidx > 0 && vlan->ifidx != ifidx))
 			continue;
 
 		wpa_printf(MSG_DEBUG, "VLAN:%s: vlan_dellink(%s)",
@@ -820,7 +823,7 @@  vlan_read_ifnames(struct nlmsghdr *h, size_t len, int del,
 		  struct hostapd_data *hapd)
 {
 	struct ifinfomsg *ifi;
-	int attrlen, nlmsg_len, rta_len;
+	int attrlen, nlmsg_len, rta_len, ifidx;
 	struct rtattr *attr;
 
 	if (len < sizeof(*ifi))
@@ -834,6 +837,7 @@  vlan_read_ifnames(struct nlmsghdr *h, size_t len, int del,
 	if (attrlen < 0)
 		return;
 
+	ifidx = ifi->ifi_index;
 	attr = (struct rtattr *) (((char *) ifi) + nlmsg_len);
 
 	rta_len = RTA_ALIGN(sizeof(struct rtattr));
@@ -852,9 +856,9 @@  vlan_read_ifnames(struct nlmsghdr *h, size_t len, int del,
 			os_memcpy(ifname, ((char *) attr) + rta_len, n);
 
 			if (del)
-				vlan_dellink(ifname, hapd);
+				vlan_dellink(ifname, ifidx, hapd);
 			else
-				vlan_newlink(ifname, hapd);
+				vlan_newlink(ifname, ifidx, hapd);
 		}
 
 		attr = RTA_NEXT(attr, attrlen);
@@ -1030,21 +1034,29 @@  static void vlan_dynamic_remove(struct hostapd_data *hapd,
 				struct hostapd_vlan *vlan)
 {
 	struct hostapd_vlan *next;
+	char buf[IFNAMSIZ+1];
+	vlan_t vlan_id = VLAN_NULL;
 
 	while (vlan) {
 		next = vlan->next;
 
-		if (vlan_untagged(&vlan->vlan_id) != VLAN_ID_WILDCARD &&
-		    hostapd_vlan_if_remove(hapd, vlan->ifname)) {
-			wpa_printf(MSG_ERROR, "VLAN: Could not remove VLAN "
-				   "iface: %s: %s",
-				   vlan->ifname, strerror(errno));
-		}
+		vlan_alloc_copy(&vlan_id, &vlan->vlan_id);
+		os_strncpy(buf, vlan->ifname, sizeof(buf));
+
 #ifdef CONFIG_FULL_DYNAMIC_VLAN
 		if (vlan->clean)
-			vlan_dellink(vlan->ifname, hapd);
+			vlan_dellink(buf, vlan->ifidx, hapd);
 #endif /* CONFIG_FULL_DYNAMIC_VLAN */
 
+		if (vlan_untagged(&vlan_id) != VLAN_ID_WILDCARD &&
+		    hostapd_vlan_if_remove(hapd, buf)) {
+			wpa_printf(MSG_ERROR, "VLAN: Could not remove VLAN "
+				   "iface: %s: %s",
+				   buf, strerror(errno));
+		}
+
+		vlan_free(&vlan_id);
+
 		vlan = next;
 	}
 }
@@ -1160,6 +1172,10 @@  struct hostapd_vlan * vlan_add_dynamic(struct hostapd_data *hapd,
 		os_free(n);
 		return NULL;
 	}
+#ifdef CONFIG_FULL_DYNAMIC_VLAN
+	// if id_add returned the ifidx, it could be reused here
+	n->ifidx = if_nametoindex(n->ifname);
+#endif
 
 	/* insert into list */
 	if (!prev) {
@@ -1181,6 +1197,7 @@  struct hostapd_vlan * vlan_add_dynamic(struct hostapd_data *hapd,
 int vlan_remove_dynamic(struct hostapd_data *hapd, vlan_t vlan_id)
 {
 	struct hostapd_vlan *vlan;
+	char buf[IFNAMSIZ+1];
 
 	if (vlan_untagged(&vlan->vlan_id) == VLAN_ID_WILDCARD)
 		return 1;
@@ -1201,8 +1218,14 @@  int vlan_remove_dynamic(struct hostapd_data *hapd, vlan_t vlan_id)
 	if (vlan == NULL)
 		return 1;
 
-	if (vlan->dynamic_vlan == 0)
-		hostapd_vlan_if_remove(hapd, vlan->ifname);
+	if (vlan->dynamic_vlan == 0) {
+		os_strncpy(buf, vlan->ifname, sizeof(buf));
+#ifdef CONFIG_FULL_DYNAMIC_VLAN
+		vlan_dellink(buf, vlan->ifidx, hapd);
+#endif
+		vlan = NULL; /* freed by vlan_dellink */
+		hostapd_vlan_if_remove(hapd, buf);
+	}
 
 	return 0;
 }