diff mbox

[PATCHv2,19/21] VLAN: ensure destroying interfaces goes reverse as creating

Message ID 81d9b7877bde304d14cf18ece25972e780e8ea31.1370512966.git.michael-dev@fami-braun.de
State Superseded
Headers show

Commit Message

michael-dev May 30, 2013, 8:59 a.m. UTC 
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>
diff mbox

Patch

diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index 504e8f3..a648aab 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -103,6 +103,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 36e74e1..5e0b536 100644
--- a/src/ap/vlan_init.c
+++ b/src/ap/vlan_init.c
@@ -649,14 +649,16 @@  static void vlan_newlink_vlan(const int vlan_id, char *ifname, int *clean,
 			*clean |= DVLAN_CLEAN_WLAN_PORT;
 }
 
-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);
@@ -697,6 +699,7 @@  static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 		 * vlan entry is freed uplink RTM_DELLINK
 		 */
 		vlan->newlink_seen = 1;
+		vlan->ifidx = ifidx;
 		break;
 	}
 }
@@ -753,7 +756,7 @@  static void vlan_dellink_vlan(int vlan_id, char* ifname, int *clean,
 		*clean = 0;
 }
 
-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];
@@ -761,7 +764,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)",
@@ -813,7 +817,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))
@@ -827,6 +831,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));
@@ -845,9 +850,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);
@@ -1023,21 +1028,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;
 	}
 }
@@ -1153,6 +1166,8 @@  struct hostapd_vlan * vlan_add_dynamic(struct hostapd_data *hapd,
 		os_free(n);
 		return NULL;
 	}
+	// if id_add returned the ifidx, it could be reused here
+	n->ifidx = if_nametoindex(n->ifname);
 
 	/* insert into list */
 	if (!prev) {
@@ -1174,6 +1189,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;
@@ -1194,8 +1210,12 @@  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));
+		vlan_dellink(buf, vlan->ifidx, hapd);
+		vlan = NULL; /* freed by vlan_dellink */
+		hostapd_vlan_if_remove(hapd, buf);
+	}
 
 	return 0;
 }