[PATCH 1/2] Fix removal of tagged interface and bridge when multiple bss share them

Michael Braun michael-dev
Mon Apr 27 00:08:03 PDT 2015


Currently, if multiple bss share are bridge and tagged vlan interface, only the
first instance of struct hostapd_vlan for this vlanid will have the
DVLAN_CLEAN_VLAN flag added.
Thus, when this instance is removed, the tagged vlan interface will be removed
from bridge, thought other bss might still need it.
Similarily, the bridge will be left over, as the does not have zero ports when
the first instance of a struct hostapd_vlan is freed.

1. This patch fixes this by having a global (per process) reference counter for
   dynamic tagged vlan and dynamically created bridge interfaces, so they are
   only removed after all local users are freed.
   (struct hapd_interfaces*)->vlan_priv is used to hold src/ap/vlan_init.c
   global per-process data like drv_priv does; right now this is only used for
   the interface reference counting, but could get extended when needed. Then
   possibly some vlan_global_init / vlan_global_deinit should be added, but
   this is not required right now.

2. Additionally, vlan->configured is checked to avoid reference counter
   decreasing before vlan_newlink increased them.

3. In order to avoid race conditions, vlan_dellink is called explicitly
   after hostapd_vlan_if_remove. Otherwise there would be a short timeframe
   between hostapd_vlan_if_remove and vlan_dellink during which the struct
   hostapd_vlan still exists, so ap_sta_bind_vlan would try to attach stations
   to it.

Signed-off-by: Michael Braun <michael-dev at fami-braun.de>
---
 src/ap/ap_config.h |   3 --
 src/ap/hostapd.h   |   3 ++
 src/ap/vlan_init.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 111 insertions(+), 12 deletions(-)

diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index 7b4a7ea..3272c8a 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -117,9 +117,6 @@ struct hostapd_vlan {
 	int dynamic_vlan;
 #ifdef CONFIG_FULL_DYNAMIC_VLAN
 
-#define DVLAN_CLEAN_BR 	0x1
-#define DVLAN_CLEAN_VLAN	0x2
-#define DVLAN_CLEAN_VLAN_PORT	0x4
 #define DVLAN_CLEAN_WLAN_PORT	0x8
 	int clean;
 #endif /* CONFIG_FULL_DYNAMIC_VLAN */
diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h
index 75cc24e..7672fd3 100644
--- a/src/ap/hostapd.h
+++ b/src/ap/hostapd.h
@@ -49,6 +49,9 @@ struct hapd_interfaces {
 	struct hostapd_iface **iface;
 
 	size_t terminate_on_error;
+#ifndef CONFIG_NO_VLAN
+	struct dynamic_iface *vlan_priv;
+#endif
 };
 
 enum hostapd_chan_status {
diff --git a/src/ap/vlan_init.c b/src/ap/vlan_init.c
index b89a1f4..ced4673 100644
--- a/src/ap/vlan_init.c
+++ b/src/ap/vlan_init.c
@@ -35,6 +35,88 @@ struct full_dynamic_vlan {
 	int s; /* socket on which to listen for new/removed interfaces. */
 };
 
+struct dynamic_iface {
+	char ifname[IFNAMSIZ+1];
+	int usage;
+
+	#define DVLAN_CLEAN_BR         0x1
+	#define DVLAN_CLEAN_VLAN       0x2
+	#define DVLAN_CLEAN_VLAN_PORT  0x4
+	int clean;
+
+	struct dynamic_iface *next;
+};
+
+
+/* Increment ref counter for ifname and add clean flag.
+ * If not in list, add it only if some flags are given.
+ */
+static void dyn_iface_get(char* ifname, int clean, struct hostapd_data *hapd) {
+	struct dynamic_iface *next, **dynamic_ifaces;
+	struct hapd_interfaces *interfaces;
+
+	interfaces = hapd->iface->interfaces;
+	dynamic_ifaces = &interfaces->vlan_priv;
+
+	for (next = *dynamic_ifaces; next; next = next->next) {
+		if (os_strcmp(ifname, next->ifname) == 0)
+			break;
+	}
+
+	if (next) {
+		next->usage++;
+		next->clean |= clean;
+		return;
+	}
+
+	if (!clean)
+		return;
+
+	next = os_zalloc(sizeof(*next));
+	if (!next)
+		return;
+	os_strlcpy(next->ifname, ifname, sizeof(next->ifname));
+	next->usage = 1;
+	next->clean = clean;
+	next->next = *dynamic_ifaces;
+	*dynamic_ifaces = next;
+}
+
+
+/* Decrement reference counter for given ifname.
+ * Return clean flag iff reference counter was decreased to zero, else zero
+ */
+static int dyn_iface_put(char* ifname, struct hostapd_data *hapd) {
+	struct dynamic_iface *next, *prev = NULL, **dynamic_ifaces;
+	struct hapd_interfaces *interfaces;
+	int clean;
+
+	interfaces = hapd->iface->interfaces;
+	dynamic_ifaces = &interfaces->vlan_priv;
+
+	for (next = *dynamic_ifaces; next; next = next->next) {
+		if (os_strcmp(ifname, next->ifname) == 0)
+			break;
+		prev = next;
+	}
+
+	if (!next)
+		return 0;
+
+	next->usage--;
+	if (next->usage)
+		return 0;
+
+	if (prev)
+		prev->next = next->next;
+	else
+		*dynamic_ifaces = next->next;
+	clean = next->clean;
+	os_free(next);
+
+	return clean;
+}
+
 
 static int ifconfig_helper(const char *if_name, int up)
 {
@@ -482,6 +564,7 @@ static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 	struct hostapd_vlan *vlan = hapd->conf->vlan;
 	char *tagged_interface = hapd->conf->ssid.vlan_tagged_interface;
 	int vlan_naming = hapd->conf->ssid.vlan_naming;
+	int clean;
 
 	wpa_printf(MSG_DEBUG, "VLAN: vlan_newlink(%s)", ifname);
 
@@ -503,7 +586,9 @@ static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 			}
 
 			if (!br_addbr(br_name))
-				vlan->clean |= DVLAN_CLEAN_BR;
+				dyn_iface_get(br_name, DVLAN_CLEAN_BR, hapd);
+			else
+				dyn_iface_get(br_name, 0, hapd);
 
 			ifconfig_up(br_name);
 
@@ -519,13 +604,16 @@ static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 						    sizeof(vlan_ifname),
 						    "vlan%d", vlan->vlan_id);
 
+				clean = 0;
 				ifconfig_up(tagged_interface);
 				if (!vlan_add(tagged_interface, vlan->vlan_id,
 					      vlan_ifname))
-					vlan->clean |= DVLAN_CLEAN_VLAN;
+					clean |= DVLAN_CLEAN_VLAN;
 
 				if (!br_addif(br_name, vlan_ifname))
-					vlan->clean |= DVLAN_CLEAN_VLAN_PORT;
+					clean |= DVLAN_CLEAN_VLAN_PORT;
+
+				dyn_iface_get(vlan_ifname, clean, hapd);
 
 				ifconfig_up(vlan_ifname);
 			}
@@ -549,13 +637,15 @@ static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
 	struct hostapd_vlan *first, *prev, *vlan = hapd->conf->vlan;
 	char *tagged_interface = hapd->conf->ssid.vlan_tagged_interface;
 	int vlan_naming = hapd->conf->ssid.vlan_naming;
+	int clean;
 
 	wpa_printf(MSG_DEBUG, "VLAN: vlan_dellink(%s)", ifname);
 
 	first = prev = vlan;
 
 	while (vlan) {
-		if (os_strcmp(ifname, vlan->ifname) == 0) {
+		if (os_strcmp(ifname, vlan->ifname) == 0 &&
+		    vlan->configured) {
 			if (hapd->conf->vlan_bridge[0]) {
 				os_snprintf(br_name, sizeof(br_name), "%s%d",
 					    hapd->conf->vlan_bridge,
@@ -583,20 +673,27 @@ static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
 					os_snprintf(vlan_ifname,
 						    sizeof(vlan_ifname),
 						    "vlan%d", vlan->vlan_id);
-				if (vlan->clean & DVLAN_CLEAN_VLAN_PORT)
+
+				clean = dyn_iface_put(vlan_ifname, hapd);
+
+				if (clean & DVLAN_CLEAN_VLAN_PORT)
 					br_delif(br_name, vlan_ifname);
-				ifconfig_down(vlan_ifname);
 
-				if (vlan->clean & DVLAN_CLEAN_VLAN)
+				if (clean & DVLAN_CLEAN_VLAN) {
+					ifconfig_down(vlan_ifname);
 					vlan_rem(vlan_ifname);
+				}
 			}
 
-			if ((vlan->clean & DVLAN_CLEAN_BR) &&
+			clean = dyn_iface_put(br_name, hapd);
+			if ((clean & DVLAN_CLEAN_BR) &&
 			    br_getnumports(br_name) == 0) {
 				ifconfig_down(br_name);
 				br_delbr(br_name);
 			}
+		}
 
+		if (os_strcmp(ifname, vlan->ifname) == 0) {
 			if (vlan == first) {
 				hapd->conf->vlan = vlan->next;
 			} else {
@@ -975,8 +1072,10 @@ int vlan_remove_dynamic(struct hostapd_data *hapd, int vlan_id)
 	if (vlan == NULL)
 		return 1;
 
-	if (vlan->dynamic_vlan == 0)
+	if (vlan->dynamic_vlan == 0) {
 		hostapd_vlan_if_remove(hapd, vlan->ifname);
+		vlan_dellink(vlan->ifname, hapd);
+	}
 
 	return 0;
 }
-- 
1.9.1




More information about the Hostap mailing list