[PATCH 0/5] mac80211: mesh beaconing

[PATCH 0/5] mac80211: mesh beaconing

[Thomas Pedersen]

Currently, mesh beacons have IEs either missing or in the wrong order. Update
the mesh beacon to the proper format specified in 802.11s draft 12.0. This
patch set will be part of a greater effort to update the mesh stack in mac80211
in anticipation of the ratification of the 802.11s standard.

Thomas Pedersen (5):
  mac80211: give if_mesh a beacon
  mac80211: correct mesh beacon head
  mac80211: Ext. Supported Rates go in mesh beacon tail
  mac80211: move RSN IE to tail of mesh beacon
  mac80211: mesh beacon includes TIM IE

 net/mac80211/cfg.c         |   31 ++++++--
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mesh.c        |  185 ++++++++++++++++++++++++++++++++------------
 net/mac80211/tx.c          |   51 ++++++------
 4 files changed, 187 insertions(+), 81 deletions(-)

-- 
1.7.6

[PATCH 1/5] mac80211: give if_mesh a beacon

[Thomas Pedersen]

Maintain a mesh beacon in the mesh interface like the AP interface does.
This will save us some work later when Probe Responses are implemented.

Based on Javier Cardona's "mac80211: Support beacon configuration via
nl80211 for mesh interfaces", which never made it upstream.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/cfg.c         |   31 +++++++++++++++++++++++------
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mesh.c        |   44 +++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/tx.c          |   45 ++++++++++++++++++-------------------------
 4 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index bfc36e9..639c287 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -466,7 +466,10 @@ static int ieee80211_config_beacon(struct ieee80211_sub_if_data *sdata,
 	int size;
 	int err = -EINVAL;
 
-	old = rtnl_dereference(sdata->u.ap.beacon);
+	if (ieee80211_vif_is_mesh(&sdata->vif))
+		old = rtnl_dereference(sdata->u.mesh.beacon);
+	else
+		old = rtnl_dereference(sdata->u.ap.beacon);
 
 	/* head must not be zero-length */
 	if (params->head && !params->head_len)
@@ -542,8 +545,10 @@ static int ieee80211_config_beacon(struct ieee80211_sub_if_data *sdata,
 
 	sdata->vif.bss_conf.dtim_period = new->dtim_period;
 
-	rcu_assign_pointer(sdata->u.ap.beacon, new);
-
+	if (ieee80211_vif_is_mesh(&sdata->vif))
+		rcu_assign_pointer(sdata->u.mesh.beacon, new);
+	else
+		rcu_assign_pointer(sdata->u.ap.beacon, new);
 	synchronize_rcu();
 
 	kfree(old);
@@ -561,7 +566,10 @@ static int ieee80211_add_beacon(struct wiphy *wiphy, struct net_device *dev,
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
-	old = rtnl_dereference(sdata->u.ap.beacon);
+	if (ieee80211_vif_is_mesh(&sdata->vif))
+		old = rtnl_dereference(sdata->u.mesh.beacon);
+	else
+		old = rtnl_dereference(sdata->u.ap.beacon);
 	if (old)
 		return -EALREADY;
 
@@ -576,7 +584,10 @@ static int ieee80211_set_beacon(struct wiphy *wiphy, struct net_device *dev,
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
-	old = rtnl_dereference(sdata->u.ap.beacon);
+	if (ieee80211_vif_is_mesh(&sdata->vif))
+		old = rtnl_dereference(sdata->u.mesh.beacon);
+	else
+		old = rtnl_dereference(sdata->u.ap.beacon);
 	if (!old)
 		return -ENOENT;
 
@@ -590,11 +601,17 @@ static int ieee80211_del_beacon(struct wiphy *wiphy, struct net_device *dev)
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
-	old = rtnl_dereference(sdata->u.ap.beacon);
+	if (ieee80211_vif_is_mesh(&sdata->vif))
+		old = rtnl_dereference(sdata->u.mesh.beacon);
+	else
+		old = rtnl_dereference(sdata->u.ap.beacon);
 	if (!old)
 		return -ENOENT;
 
-	rcu_assign_pointer(sdata->u.ap.beacon, NULL);
+	if (ieee80211_vif_is_mesh(&sdata->vif))
+		rcu_assign_pointer(sdata->u.mesh.beacon, NULL);
+	else
+		rcu_assign_pointer(sdata->u.ap.beacon, NULL);
 	synchronize_rcu();
 	kfree(old);
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index dda0d1a..2d726b7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -519,6 +519,7 @@ struct ieee80211_if_mesh {
 		IEEE80211_MESH_SEC_AUTHED = 0x1,
 		IEEE80211_MESH_SEC_SECURED = 0x2,
 	} security;
+	struct beacon_data *beacon;
 };
 
 #ifdef CONFIG_MAC80211_MESH
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 29e9980..60fd958 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <asm/unaligned.h>
 #include "ieee80211_i.h"
+#include "cfg.h"
 #include "mesh.h"
 
 #define IEEE80211_MESH_PEER_INACTIVITY_LIMIT (1800 * HZ)
@@ -461,6 +462,9 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_mgmt *bcn_head;
+	struct beacon_parameters bcn_params;
+	u8 *pos;
 
 	local->fif_other_bss++;
 	/* mesh ifaces must set allmulti to forward mcast traffic */
@@ -473,7 +477,44 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	set_bit(MESH_WORK_HOUSEKEEPING, &ifmsh->wrkq_flags);
 	ieee80211_mesh_root_setup(ifmsh);
 	ieee80211_queue_work(&local->hw, &sdata->work);
+
+	/* Build fixed part of mesh beacon. */
+	memset(&bcn_params, 0, sizeof(struct beacon_parameters));
+
+	/* header + fixed fields + null ssid */
+	bcn_params.head_len = 24 + sizeof(bcn_head->u.beacon) + 2;
+	pos = kmalloc(bcn_params.head_len, GFP_KERNEL | __GFP_ZERO);
+	if (pos == NULL) {
+		printk(KERN_ERR "Unable to allocate mesh beacon\n");
+		return;
+	}
+
+	/* Header */
+	bcn_head = (struct ieee80211_mgmt *) pos;
+	bcn_head->frame_control =
+		cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_BEACON);
+	memset(bcn_head->da, 0xff, ETH_ALEN);
+	memcpy(bcn_head->sa, sdata->vif.addr, ETH_ALEN);
+	memcpy(bcn_head->bssid, sdata->vif.addr, ETH_ALEN);
+
+	/* Default beacon interval and capabilities */
+	bcn_head->u.beacon.beacon_int = MESH_DEFAULT_BEACON_INTERVAL;
+	bcn_head->u.beacon.capab_info = 0x0; /* 0x0 for MPs */
+
+	/* Mesh uses null SSID */
+	pos += 24 + sizeof(bcn_head->u.beacon);
+	*pos++ = WLAN_EID_SSID;
+	*pos++ = 0x0;
+
+	bcn_params.head = (char *) bcn_head;
+	bcn_params.dtim_period = 1;	/* unused for now */
+
+	if (mac80211_config_ops.add_beacon(sdata->wdev.wiphy, sdata->dev,
+				&bcn_params))
+		printk(KERN_ERR "Unable to configure mesh beacon\n");
+
 	sdata->vif.bss_conf.beacon_int = MESH_DEFAULT_BEACON_INTERVAL;
+	sdata->vif.bss_conf.enable_beacon = 1;
 	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON |
 						BSS_CHANGED_BEACON_ENABLED |
 						BSS_CHANGED_BEACON_INT);
@@ -485,6 +526,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 
 	ifmsh->mesh_id_len = 0;
+	sdata->vif.bss_conf.enable_beacon = 0;
 	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
 	sta_info_flush(local, NULL);
 
@@ -498,6 +540,8 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
 	 * it no longer is.
 	 */
 	cancel_work_sync(&sdata->work);
+	if (sdata->u.mesh.beacon)
+		mac80211_config_ops.del_beacon(sdata->wdev.wiphy, sdata->dev);
 
 	local->fif_other_bss--;
 	atomic_dec(&local->iff_allmultis);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 8cb0d2d..c406b13 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2250,38 +2250,31 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
 		hdr->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
 						 IEEE80211_STYPE_BEACON);
 	} else if (ieee80211_vif_is_mesh(&sdata->vif)) {
-		struct ieee80211_mgmt *mgmt;
-		u8 *pos;
-
+		struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 #ifdef CONFIG_MAC80211_MESH
 		if (!sdata->u.mesh.mesh_id_len)
 			goto out;
 #endif
+		beacon = rcu_dereference(ifmsh->beacon);
+		if (beacon) {
+			/* headroom, head length, tail length, mesh_ies and
+			 * maximum TIM length */
+			skb = dev_alloc_skb(local->tx_headroom +
+					    beacon->head_len +
+					    beacon->tail_len + 400);
+			if (!skb)
+				goto out;
 
-		/* headroom, head length, tail length and maximum TIM length */
-		skb = dev_alloc_skb(local->tx_headroom + 400 +
-				sdata->u.mesh.ie_len);
-		if (!skb)
-			goto out;
+			skb_reserve(skb, local->hw.extra_tx_headroom);
+			memcpy(skb_put(skb, beacon->head_len), beacon->head,
+			       beacon->head_len);
+
+			mesh_mgmt_ies_add(skb, sdata);
 
-		skb_reserve(skb, local->hw.extra_tx_headroom);
-		mgmt = (struct ieee80211_mgmt *)
-			skb_put(skb, 24 + sizeof(mgmt->u.beacon));
-		memset(mgmt, 0, 24 + sizeof(mgmt->u.beacon));
-		mgmt->frame_control =
-		    cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_BEACON);
-		memset(mgmt->da, 0xff, ETH_ALEN);
-		memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
-		memcpy(mgmt->bssid, sdata->vif.addr, ETH_ALEN);
-		mgmt->u.beacon.beacon_int =
-			cpu_to_le16(sdata->vif.bss_conf.beacon_int);
-		mgmt->u.beacon.capab_info = 0x0; /* 0x0 for MPs */
-
-		pos = skb_put(skb, 2);
-		*pos++ = WLAN_EID_SSID;
-		*pos++ = 0x0;
-
-		mesh_mgmt_ies_add(skb, sdata);
+			if (beacon->tail && beacon->tail_len)
+				memcpy(skb_put(skb, beacon->tail_len),
+				       beacon->tail, beacon->tail_len);
+		}
 	} else {
 		WARN_ON(1);
 		goto out;
-- 
1.7.6

[PATCH 2/5] mac80211: correct mesh beacon head

[Thomas Pedersen]

Supported Rates and DS Parameter elements go in the beacon head. Add
these IEs when we initially construct the beacon.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/mesh.c |   47 ++++++++++++++++++++++++++++++-----------------
 1 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 60fd958..70bfa96 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -217,14 +217,6 @@ void mesh_mgmt_ies_add(struct sk_buff *skb, struct ieee80211_sub_if_data *sdata)
 	len = sband->n_bitrates;
 	if (len > 8)
 		len = 8;
-	pos = skb_put(skb, len + 2);
-	*pos++ = WLAN_EID_SUPP_RATES;
-	*pos++ = len;
-	for (i = 0; i < len; i++) {
-		rate = sband->bitrates[i].bitrate;
-		*pos++ = (u8) (rate / 5);
-	}
-
 	if (sband->n_bitrates > len) {
 		pos = skb_put(skb, sband->n_bitrates - len + 2);
 		*pos++ = WLAN_EID_EXT_SUPP_RATES;
@@ -235,13 +227,6 @@ void mesh_mgmt_ies_add(struct sk_buff *skb, struct ieee80211_sub_if_data *sdata)
 		}
 	}
 
-	if (sband->band == IEEE80211_BAND_2GHZ) {
-		pos = skb_put(skb, 2 + 1);
-		*pos++ = WLAN_EID_DS_PARAMS;
-		*pos++ = 1;
-		*pos++ = ieee80211_frequency_to_channel(local->hw.conf.channel->center_freq);
-	}
-
 	pos = skb_put(skb, 2 + sdata->u.mesh.mesh_id_len);
 	*pos++ = WLAN_EID_MESH_ID;
 	*pos++ = sdata->u.mesh.mesh_id_len;
@@ -464,6 +449,10 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_mgmt *bcn_head;
 	struct beacon_parameters bcn_params;
+	struct ieee80211_supported_band *sband;
+	struct ieee80211_channel *channel = local->hw.conf.channel;
+	u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
+	int rates, i;
 	u8 *pos;
 
 	local->fif_other_bss++;
@@ -478,11 +467,22 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	ieee80211_mesh_root_setup(ifmsh);
 	ieee80211_queue_work(&local->hw, &sdata->work);
 
+	/* build supported rates array */
+	sband = local->hw.wiphy->bands[channel->band];
+	rates = sband->n_bitrates;
+	if (rates > 8)
+		rates = 8;
+	for (i = 0; i < rates; i++) {
+		int rate = sband->bitrates[i].bitrate;
+		supp_rates[i] = (u8) (rate / 5);
+	}
+
 	/* Build fixed part of mesh beacon. */
 	memset(&bcn_params, 0, sizeof(struct beacon_parameters));
 
-	/* header + fixed fields + null ssid */
-	bcn_params.head_len = 24 + sizeof(bcn_head->u.beacon) + 2;
+	/* header + fixed fields + null ssid + supp rates + DS params */
+	bcn_params.head_len = 24 + sizeof(bcn_head->u.beacon) + 2 +
+			      (2 + rates) + 3;
 	pos = kmalloc(bcn_params.head_len, GFP_KERNEL | __GFP_ZERO);
 	if (pos == NULL) {
 		printk(KERN_ERR "Unable to allocate mesh beacon\n");
@@ -506,6 +506,19 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	*pos++ = WLAN_EID_SSID;
 	*pos++ = 0x0;
 
+	/* supported rates */
+	*pos++ = WLAN_EID_SUPP_RATES;
+	*pos++ = rates;
+	memcpy(pos, supp_rates, rates);
+	pos += rates;
+
+	/* DS parameter set */
+	if (sband->band == IEEE80211_BAND_2GHZ) {
+		*pos++ = WLAN_EID_DS_PARAMS;
+		*pos++ = 1;
+		*pos++ = ieee80211_frequency_to_channel(channel->center_freq);
+	}
+
 	bcn_params.head = (char *) bcn_head;
 	bcn_params.dtim_period = 1;	/* unused for now */
 
-- 
1.7.6

[PATCH 3/5] mac80211: Ext. Supported Rates go in mesh beacon tail

[Thomas Pedersen]

Give mesh beacon a tail and maintain Extended Supported Rates IE here.
Reorder construction of mesh beacon frame so it looks like:

head | tail | mesh IEs | vendor IEs

mesh IEs include dynamic elements which must be updated on each beacon.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/mesh.c |   41 ++++++++++++++++++++++-------------------
 net/mac80211/tx.c   |    4 ++--
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 70bfa96..7c10b97 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -207,26 +207,9 @@ int mesh_rmc_check(u8 *sa, struct ieee80211s_hdr *mesh_hdr,
 
 void mesh_mgmt_ies_add(struct sk_buff *skb, struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_supported_band *sband;
 	u8 *pos;
-	int len, i, rate;
 	u8 neighbors;
 
-	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
-	len = sband->n_bitrates;
-	if (len > 8)
-		len = 8;
-	if (sband->n_bitrates > len) {
-		pos = skb_put(skb, sband->n_bitrates - len + 2);
-		*pos++ = WLAN_EID_EXT_SUPP_RATES;
-		*pos++ = sband->n_bitrates - len;
-		for (i = len; i < sband->n_bitrates; i++) {
-			rate = sband->bitrates[i].bitrate;
-			*pos++ = (u8) (rate / 5);
-		}
-	}
-
 	pos = skb_put(skb, 2 + sdata->u.mesh.mesh_id_len);
 	*pos++ = WLAN_EID_MESH_ID;
 	*pos++ = sdata->u.mesh.mesh_id_len;
@@ -452,8 +435,8 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *channel = local->hw.conf.channel;
 	u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
-	int rates, i;
-	u8 *pos;
+	int rates, exrates, i;
+	u8 *pos, *bcn_tail;
 
 	local->fif_other_bss++;
 	/* mesh ifaces must set allmulti to forward mcast traffic */
@@ -472,6 +455,7 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	rates = sband->n_bitrates;
 	if (rates > 8)
 		rates = 8;
+	exrates = sband->n_bitrates - rates;
 	for (i = 0; i < rates; i++) {
 		int rate = sband->bitrates[i].bitrate;
 		supp_rates[i] = (u8) (rate / 5);
@@ -519,7 +503,26 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		*pos++ = ieee80211_frequency_to_channel(channel->center_freq);
 	}
 
+	/* Build beacon tail */
+	bcn_params.tail_len = 2 + exrates; /* extended rates */
+	pos = kmalloc(bcn_params.tail_len, GFP_KERNEL | __GFP_ZERO);
+	if (pos == NULL) {
+		printk(KERN_ERR "Unable to allocate mesh beacon tail\n");
+		return;
+	}
+	bcn_tail = pos;
+
+	if (exrates) {
+		*pos++ = WLAN_EID_EXT_SUPP_RATES;
+		*pos++ = exrates;
+		for (i = rates; i < sband->n_bitrates; i++) {
+			int rate = sband->bitrates[i].bitrate;
+			*pos++ = (u8) (rate / 5);
+		}
+	}
+
 	bcn_params.head = (char *) bcn_head;
+	bcn_params.tail = bcn_tail;
 	bcn_params.dtim_period = 1;	/* unused for now */
 
 	if (mac80211_config_ops.add_beacon(sdata->wdev.wiphy, sdata->dev,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index c406b13..e1389e8 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2269,11 +2269,11 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
 			memcpy(skb_put(skb, beacon->head_len), beacon->head,
 			       beacon->head_len);
 
-			mesh_mgmt_ies_add(skb, sdata);
-
 			if (beacon->tail && beacon->tail_len)
 				memcpy(skb_put(skb, beacon->tail_len),
 				       beacon->tail, beacon->tail_len);
+
+			mesh_mgmt_ies_add(skb, sdata);
 		}
 	} else {
 		WARN_ON(1);
-- 
1.7.6

[PATCH 4/5] mac80211: move RSN IE to tail of mesh beacon

[Thomas Pedersen]

The mesh specific beacon IEs go right before the vendor IEs. Add a
filter that checks for custom IEs (currently just the RSN IE), which
belong after the TIM, but before HT. Also, ensure we don't add any
custom IEs twice in mesh_mgmt_ies_add().

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/mesh.c |   68 ++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 7c10b97..d77268f 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -207,52 +207,64 @@ int mesh_rmc_check(u8 *sa, struct ieee80211s_hdr *mesh_hdr,
 
 void mesh_mgmt_ies_add(struct sk_buff *skb, struct ieee80211_sub_if_data *sdata)
 {
+	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 	u8 *pos;
 	u8 neighbors;
+	u8 offset = 0, noffset;
 
-	pos = skb_put(skb, 2 + sdata->u.mesh.mesh_id_len);
+	pos = skb_put(skb, 2 + ifmsh->mesh_id_len);
 	*pos++ = WLAN_EID_MESH_ID;
-	*pos++ = sdata->u.mesh.mesh_id_len;
-	if (sdata->u.mesh.mesh_id_len)
-		memcpy(pos, sdata->u.mesh.mesh_id, sdata->u.mesh.mesh_id_len);
+	*pos++ = ifmsh->mesh_id_len;
+	if (ifmsh->mesh_id_len)
+		memcpy(pos, ifmsh->mesh_id, ifmsh->mesh_id_len);
 
 	pos = skb_put(skb, 2 + sizeof(struct ieee80211_meshconf_ie));
 	*pos++ = WLAN_EID_MESH_CONFIG;
 	*pos++ = sizeof(struct ieee80211_meshconf_ie);
 
 	/* Active path selection protocol ID */
-	*pos++ = sdata->u.mesh.mesh_pp_id;
+	*pos++ = ifmsh->mesh_pp_id;
 
 	/* Active path selection metric ID   */
-	*pos++ = sdata->u.mesh.mesh_pm_id;
+	*pos++ = ifmsh->mesh_pm_id;
 
 	/* Congestion control mode identifier */
-	*pos++ = sdata->u.mesh.mesh_cc_id;
+	*pos++ = ifmsh->mesh_cc_id;
 
 	/* Synchronization protocol identifier */
-	*pos++ = sdata->u.mesh.mesh_sp_id;
+	*pos++ = ifmsh->mesh_sp_id;
 
 	/* Authentication Protocol identifier */
-	*pos++ = sdata->u.mesh.mesh_auth_id;
+	*pos++ = ifmsh->mesh_auth_id;
 
 	/* Mesh Formation Info - number of neighbors */
-	neighbors = atomic_read(&sdata->u.mesh.mshstats.estab_plinks);
+	neighbors = atomic_read(&ifmsh->mshstats.estab_plinks);
 	/* Number of neighbor mesh STAs or 15 whichever is smaller */
 	neighbors = (neighbors > 15) ? 15 : neighbors;
 	*pos++ = neighbors << 1;
 
 	/* Mesh capability */
-	sdata->u.mesh.accepting_plinks = mesh_plink_availables(sdata);
+	ifmsh->accepting_plinks = mesh_plink_availables(sdata);
 	*pos = MESHCONF_CAPAB_FORWARDING;
-	*pos++ |= sdata->u.mesh.accepting_plinks ?
+	*pos++ |= ifmsh->accepting_plinks ?
 	    MESHCONF_CAPAB_ACCEPT_PLINKS : 0x00;
 	*pos++ = 0x00;
 
-	if (sdata->u.mesh.ie) {
-		int len = sdata->u.mesh.ie_len;
-		const u8 *data = sdata->u.mesh.ie;
-		if (skb_tailroom(skb) > len)
-			memcpy(skb_put(skb, len), data, len);
+	if (ifmsh->ie) {
+		int len;
+		const u8 *data;
+		/* fast-forward to vendor IEs and add remaining */
+		noffset = ieee80211_ie_split_vendor(ifmsh->ie, ifmsh->ie_len,
+						    offset);
+		if (noffset != offset) {
+			len = ifmsh->ie_len - noffset;
+			data = ifmsh->ie + noffset;
+			if (skb_tailroom(skb) > len)
+				memcpy(skb_put(skb, len), data, len);
+			else
+				printk(KERN_ERR "o11s: not enough tailroom\n");
+			offset = noffset;
+		}
 	}
 }
 
@@ -435,8 +447,8 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *channel = local->hw.conf.channel;
 	u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
-	int rates, exrates, i;
-	u8 *pos, *bcn_tail;
+	int rates, exrates, i, len = 0;
+	u8 *pos, *bcn_tail, offset = 0, noffset;
 
 	local->fif_other_bss++;
 	/* mesh ifaces must set allmulti to forward mcast traffic */
@@ -503,8 +515,20 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		*pos++ = ieee80211_frequency_to_channel(channel->center_freq);
 	}
 
+	/* Check for custom IEs before HT */
+	if (ifmsh->ie_len && ifmsh->ie) {
+		static const u8 tail_ies[] = {
+			WLAN_EID_RSN,
+		};
+		noffset = ieee80211_ie_split(ifmsh->ie, ifmsh->ie_len,
+					     tail_ies, ARRAY_SIZE(tail_ies),
+					     offset);
+		len = noffset - offset;
+	}
+
 	/* Build beacon tail */
-	bcn_params.tail_len = 2 + exrates; /* extended rates */
+	bcn_params.tail_len = 2 + exrates + /* extended rates */
+			      len; /* extra IEs */
 	pos = kmalloc(bcn_params.tail_len, GFP_KERNEL | __GFP_ZERO);
 	if (pos == NULL) {
 		printk(KERN_ERR "Unable to allocate mesh beacon tail\n");
@@ -521,6 +545,10 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		}
 	}
 
+	memcpy(pos, ifmsh->ie + offset, len);
+	pos += len;
+	offset = noffset;
+
 	bcn_params.head = (char *) bcn_head;
 	bcn_params.tail = bcn_tail;
 	bcn_params.dtim_period = 1;	/* unused for now */
-- 
1.7.6

[PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Thomas Pedersen]

Even though we don't currently implement PS for mesh points, the
standard asks us to include a TIM element in beacons. Include an empty
element for now.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/tx.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index e1389e8..174db68 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2251,6 +2251,7 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
 						 IEEE80211_STYPE_BEACON);
 	} else if (ieee80211_vif_is_mesh(&sdata->vif)) {
 		struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+		u8 *pos;
 #ifdef CONFIG_MAC80211_MESH
 		if (!sdata->u.mesh.mesh_id_len)
 			goto out;
@@ -2269,6 +2270,14 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
 			memcpy(skb_put(skb, beacon->head_len), beacon->head,
 			       beacon->head_len);
 
+			pos = skb_put(skb, 6);
+			*pos++ = WLAN_EID_TIM;
+			*pos++ = 4;
+			*pos++ = 0;	/* DTIM count */
+			*pos++ = 0;	/* DTIM period */
+			*pos++ = 0;	/* Bitmap control */
+			*pos++ = 0;	/* Part Virt Bitmap */
+
 			if (beacon->tail && beacon->tail_len)
 				memcpy(skb_put(skb, beacon->tail_len),
 				       beacon->tail, beacon->tail_len);
-- 
1.7.6

Re: [PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Jouni Malinen]

On Wed, Jul 13, 2011 at 04:45:47PM -0700, Thomas Pedersen wrote:
> Even though we don't currently implement PS for mesh points, the
> standard asks us to include a TIM element in beacons. Include an empty
> element for now.

Is it allowed to not support PS in this type of case? AP does not have
such option..

> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> @@ -2269,6 +2270,14 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
> +			pos = skb_put(skb, 6);
> +			*pos++ = WLAN_EID_TIM;
> +			*pos++ = 4;
> +			*pos++ = 0;	/* DTIM count */
> +			*pos++ = 0;	/* DTIM period */

The DTIM Period value 0 is reserved and does not really make much
sense.. Would it be better to set this to 1?

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Johannes Berg]

On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
> Maintain a mesh beacon in the mesh interface like the AP interface does.
> This will save us some work later when Probe Responses are implemented.
> 
> Based on Javier Cardona's "mac80211: Support beacon configuration via
> nl80211 for mesh interfaces", which never made it upstream.

That makes little sense, was there a discussion, or did it just get
lost? Maybe you should resend it.

johannes

Re: [PATCH 4/5] mac80211: move RSN IE to tail of mesh beacon

[Johannes Berg]

On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:

> +			if (skb_tailroom(skb) > len)
> +				memcpy(skb_put(skb, len), data, len);
> +			else
> +				printk(KERN_ERR "o11s: not enough tailroom\n");

Can this printk happen? If not, how about WARN(1, ...);, and if, I think
there should be better handling than a printk.

johannes

Re: [PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Johannes Berg]

On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:

>  			goto out;
> @@ -2269,6 +2270,14 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
>  			memcpy(skb_put(skb, beacon->head_len), beacon->head,
>  			       beacon->head_len);
>  
> +			pos = skb_put(skb, 6);

You just keep adding here -- how is the length of the skb determined?

johannes

Re: [PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Thomas Pedersen]

On Thu, Jul 14, 2011 at 2:52 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
>
>>                       goto out;
>> @@ -2269,6 +2270,14 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
>>                       memcpy(skb_put(skb, beacon->head_len), beacon->head,
>>                              beacon->head_len);
>>
>> +                     pos = skb_put(skb, 6);
>
> You just keep adding here -- how is the length of the skb determined?

A little further up:

                        /* headroom, head length, tail length, custom IEs, and
                         * mesh IEs + maximum TIM length */
                        skb = dev_alloc_skb(local->tx_headroom +
                                            beacon->head_len +
                                            beacon->tail_len +
                                            ifmsh->ie_len + 400);

we made room for the max TIM element.
>
> johannes
>
>

Re: [PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Thomas Pedersen]

On Wed, Jul 13, 2011 at 11:25 PM, Jouni Malinen <j@w1.fi> wrote:
> On Wed, Jul 13, 2011 at 04:45:47PM -0700, Thomas Pedersen wrote:
>> Even though we don't currently implement PS for mesh points, the
>> standard asks us to include a TIM element in beacons. Include an empty
>> element for now.
>
> Is it allowed to not support PS in this type of case? AP does not have
> such option..
>

You're right, it doesn't look like PS support is optional. The
standard requires us to respect the PS states of peers. Support for
this will require some work, but in the meantime, we can include a TIM
element indicating no frames buffered here, as well as a Mesh Awake
Window which never expires.

Otherwise, until we implement proper PS, might it be better not to
include the TIM IE at all?

>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>> @@ -2269,6 +2270,14 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
>> +                     pos = skb_put(skb, 6);
>> +                     *pos++ = WLAN_EID_TIM;
>> +                     *pos++ = 4;
>> +                     *pos++ = 0;     /* DTIM count */
>> +                     *pos++ = 0;     /* DTIM period */
>
> The DTIM Period value 0 is reserved and does not really make much
> sense.. Would it be better to set this to 1?

That makes sense, thanks!

>
> --
> Jouni Malinen                                            PGP id EFC895FA
>

[PATCH v2 4/5] mac80211: move RSN IE to tail of mesh beacon

[Thomas Pedersen]

The mesh specific beacon IEs go right before the vendor IEs. Add a
filter that checks for custom IEs (currently just the RSN IE), which
belong after the TIM, but before HT. Also, ensure we don't add any
custom IEs twice in mesh_mgmt_ies_add().

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---

v2:
	Ensure room for custom IEs in transmitted beacon (Johannes).
	We'll be allocating twice as much space as needed for some IEs
	(in tail_len and again ie_len), but it shouldn't be a big deal.

 net/mac80211/mesh.c |   63 +++++++++++++++++++++++++++++++++++---------------
 net/mac80211/tx.c   |    7 +++--
 2 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 7c10b97..b4bddf2 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -207,52 +207,61 @@ int mesh_rmc_check(u8 *sa, struct ieee80211s_hdr *mesh_hdr,
 
 void mesh_mgmt_ies_add(struct sk_buff *skb, struct ieee80211_sub_if_data *sdata)
 {
+	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 	u8 *pos;
 	u8 neighbors;
+	u8 offset = 0, noffset;
 
-	pos = skb_put(skb, 2 + sdata->u.mesh.mesh_id_len);
+	pos = skb_put(skb, 2 + ifmsh->mesh_id_len);
 	*pos++ = WLAN_EID_MESH_ID;
-	*pos++ = sdata->u.mesh.mesh_id_len;
-	if (sdata->u.mesh.mesh_id_len)
-		memcpy(pos, sdata->u.mesh.mesh_id, sdata->u.mesh.mesh_id_len);
+	*pos++ = ifmsh->mesh_id_len;
+	if (ifmsh->mesh_id_len)
+		memcpy(pos, ifmsh->mesh_id, ifmsh->mesh_id_len);
 
 	pos = skb_put(skb, 2 + sizeof(struct ieee80211_meshconf_ie));
 	*pos++ = WLAN_EID_MESH_CONFIG;
 	*pos++ = sizeof(struct ieee80211_meshconf_ie);
 
 	/* Active path selection protocol ID */
-	*pos++ = sdata->u.mesh.mesh_pp_id;
+	*pos++ = ifmsh->mesh_pp_id;
 
 	/* Active path selection metric ID   */
-	*pos++ = sdata->u.mesh.mesh_pm_id;
+	*pos++ = ifmsh->mesh_pm_id;
 
 	/* Congestion control mode identifier */
-	*pos++ = sdata->u.mesh.mesh_cc_id;
+	*pos++ = ifmsh->mesh_cc_id;
 
 	/* Synchronization protocol identifier */
-	*pos++ = sdata->u.mesh.mesh_sp_id;
+	*pos++ = ifmsh->mesh_sp_id;
 
 	/* Authentication Protocol identifier */
-	*pos++ = sdata->u.mesh.mesh_auth_id;
+	*pos++ = ifmsh->mesh_auth_id;
 
 	/* Mesh Formation Info - number of neighbors */
-	neighbors = atomic_read(&sdata->u.mesh.mshstats.estab_plinks);
+	neighbors = atomic_read(&ifmsh->mshstats.estab_plinks);
 	/* Number of neighbor mesh STAs or 15 whichever is smaller */
 	neighbors = (neighbors > 15) ? 15 : neighbors;
 	*pos++ = neighbors << 1;
 
 	/* Mesh capability */
-	sdata->u.mesh.accepting_plinks = mesh_plink_availables(sdata);
+	ifmsh->accepting_plinks = mesh_plink_availables(sdata);
 	*pos = MESHCONF_CAPAB_FORWARDING;
-	*pos++ |= sdata->u.mesh.accepting_plinks ?
+	*pos++ |= ifmsh->accepting_plinks ?
 	    MESHCONF_CAPAB_ACCEPT_PLINKS : 0x00;
 	*pos++ = 0x00;
 
-	if (sdata->u.mesh.ie) {
-		int len = sdata->u.mesh.ie_len;
-		const u8 *data = sdata->u.mesh.ie;
-		if (skb_tailroom(skb) > len)
+	if (ifmsh->ie) {
+		int len;
+		const u8 *data;
+		/* fast-forward to vendor IEs and add remaining */
+		noffset = ieee80211_ie_split_vendor(ifmsh->ie, ifmsh->ie_len,
+						    offset);
+		if (noffset != offset) {
+			len = ifmsh->ie_len - noffset;
+			data = ifmsh->ie + noffset;
 			memcpy(skb_put(skb, len), data, len);
+			offset = noffset;
+		}
 	}
 }
 
@@ -435,8 +444,8 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *channel = local->hw.conf.channel;
 	u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
-	int rates, exrates, i;
-	u8 *pos, *bcn_tail;
+	int rates, exrates, i, len = 0;
+	u8 *pos, *bcn_tail, offset = 0, noffset;
 
 	local->fif_other_bss++;
 	/* mesh ifaces must set allmulti to forward mcast traffic */
@@ -503,8 +512,20 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		*pos++ = ieee80211_frequency_to_channel(channel->center_freq);
 	}
 
+	/* Check for custom IEs before HT */
+	if (ifmsh->ie_len && ifmsh->ie) {
+		static const u8 tail_ies[] = {
+			WLAN_EID_RSN,
+		};
+		noffset = ieee80211_ie_split(ifmsh->ie, ifmsh->ie_len,
+					     tail_ies, ARRAY_SIZE(tail_ies),
+					     offset);
+		len = noffset - offset;
+	}
+
 	/* Build beacon tail */
-	bcn_params.tail_len = 2 + exrates; /* extended rates */
+	bcn_params.tail_len = 2 + exrates + /* extended rates */
+			      len; /* extra IEs */
 	pos = kmalloc(bcn_params.tail_len, GFP_KERNEL | __GFP_ZERO);
 	if (pos == NULL) {
 		printk(KERN_ERR "Unable to allocate mesh beacon tail\n");
@@ -521,6 +542,10 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		}
 	}
 
+	memcpy(pos, ifmsh->ie + offset, len);
+	pos += len;
+	offset = noffset;
+
 	bcn_params.head = (char *) bcn_head;
 	bcn_params.tail = bcn_tail;
 	bcn_params.dtim_period = 1;	/* unused for now */
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index e1389e8..c692ced 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2257,11 +2257,12 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
 #endif
 		beacon = rcu_dereference(ifmsh->beacon);
 		if (beacon) {
-			/* headroom, head length, tail length, mesh_ies and
-			 * maximum TIM length */
+			/* headroom, head length, tail length, custom IEs, and
+			 * mesh IEs + maximum TIM length */
 			skb = dev_alloc_skb(local->tx_headroom +
 					    beacon->head_len +
-					    beacon->tail_len + 400);
+					    beacon->tail_len +
+					    ifmsh->ie_len + 400);
 			if (!skb)
 				goto out;
 
-- 
1.7.6

Re: [PATCH v2 4/5] mac80211: move RSN IE to tail of mesh beacon

[Johannes Berg]

On Thu, 2011-07-14 at 17:50 -0700, Thomas Pedersen wrote:
> The mesh specific beacon IEs go right before the vendor IEs. Add a
> filter that checks for custom IEs (currently just the RSN IE), which
> belong after the TIM, but before HT. Also, ensure we don't add any
> custom IEs twice in mesh_mgmt_ies_add().
> 
> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
> ---
> 
> v2:
> 	Ensure room for custom IEs in transmitted beacon (Johannes).
> 	We'll be allocating twice as much space as needed for some IEs
> 	(in tail_len and again ie_len), but it shouldn't be a big deal.

Not that I like allocating twice as much, but I don't really care
either.

Maybe you should be doing the cleanups in mesh_mgmt_ies_add()
separately? I'm having a hard time following these changes.

> +               noffset = ieee80211_ie_split_vendor(ifmsh->ie, ifmsh->ie_len,
> +                                                   offset);
> +               if (noffset != offset) {

isn't offset always 0 here? So wouldn't it make more sense to put 0
explicitly?

johannes

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Johannes Berg]

On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:

> Based on Javier Cardona's "mac80211: Support beacon configuration via
> nl80211 for mesh interfaces", which never made it upstream.

Ok I don't get it -- this patchset seems to apply fine, so what part is
dependent on Javier's patches?

johannes

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Johannes Berg]

On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:

> +	if (ieee80211_vif_is_mesh(&sdata->vif))
> +		rcu_assign_pointer(sdata->u.mesh.beacon, new);

I don't think this compiles if MESH isn't configured into mac80211. Not
sure adding lots of ifdefs is good, but it's probably the only choice
right now.

> @@ -461,6 +462,9 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>  {
>  	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>  	struct ieee80211_local *local = sdata->local;
> +	struct ieee80211_mgmt *bcn_head;
> +	struct beacon_parameters bcn_params;
> +	u8 *pos;
>  
>  	local->fif_other_bss++;
>  	/* mesh ifaces must set allmulti to forward mcast traffic */
> @@ -473,7 +477,44 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>  	set_bit(MESH_WORK_HOUSEKEEPING, &ifmsh->wrkq_flags);
>  	ieee80211_mesh_root_setup(ifmsh);
>  	ieee80211_queue_work(&local->hw, &sdata->work);
> +
> +	/* Build fixed part of mesh beacon. */
> +	memset(&bcn_params, 0, sizeof(struct beacon_parameters));
> +
> +	/* header + fixed fields + null ssid */
> +	bcn_params.head_len = 24 + sizeof(bcn_head->u.beacon) + 2;
> +	pos = kmalloc(bcn_params.head_len, GFP_KERNEL | __GFP_ZERO);

Err, kzalloc, don't use __GFP_ZERO.

> +	if (mac80211_config_ops.add_beacon(sdata->wdev.wiphy, sdata->dev,
> +				&bcn_params))
> +		printk(KERN_ERR "Unable to configure mesh beacon\n");

That's horrible. Don't do that. Just do whatever is necessary inline. If
you restructure, you only need like 10% of the code from
ieee80211_config_beacon() anyway.

> @@ -498,6 +540,8 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
>  	 * it no longer is.
>  	 */
>  	cancel_work_sync(&sdata->work);
> +	if (sdata->u.mesh.beacon)
> +		mac80211_config_ops.del_beacon(sdata->wdev.wiphy, sdata->dev);

Ditto.

>  	} else if (ieee80211_vif_is_mesh(&sdata->vif)) {
> -		struct ieee80211_mgmt *mgmt;
> -		u8 *pos;
> -
> +		struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>  #ifdef CONFIG_MAC80211_MESH
>  		if (!sdata->u.mesh.mesh_id_len)
>  			goto out;
>  #endif
> +		beacon = rcu_dereference(ifmsh->beacon);

not going to compile w/o CONFIG_MAC80211_MESH

> +		if (beacon) {
> +			/* headroom, head length, tail length, mesh_ies and
> +			 * maximum TIM length */
> +			skb = dev_alloc_skb(local->tx_headroom +
> +					    beacon->head_len +
> +					    beacon->tail_len + 400);

what's + 400? You can't really think the TIM IE is that long?

> -		/* headroom, head length, tail length and maximum TIM length */
> -		skb = dev_alloc_skb(local->tx_headroom + 400 +
> -				sdata->u.mesh.ie_len);

I guess it was even there before though ... huh?

Couldn't more code here be shared between mesh and AP? I don't see any
difference here between mesh and AP.

Also ... since you'll need PS support eventually anyway, maybe you
should refactor "struct ieee80211_if_ap" into something AP-specific
(currently only VLANs) and the rest, and then use "the rest" in mesh as
well to make all this code more unified.

johannes

Re: [PATCH 2/5] mac80211: correct mesh beacon head

[Johannes Berg]

On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
> Supported Rates and DS Parameter elements go in the beacon head. Add
> these IEs when we initially construct the beacon.

This breaks mesh_plink_frame_tx() afaict.

johannes

Re: [PATCH 3/5] mac80211: Ext. Supported Rates go in mesh beacon tail

[Johannes Berg]

On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
> Give mesh beacon a tail and maintain Extended Supported Rates IE here.
> Reorder construction of mesh beacon frame so it looks like:
> 
> head | tail | mesh IEs | vendor IEs
> 
> mesh IEs include dynamic elements which must be updated on each beacon.

This is stupid, you're introducing a bug you fix in patch 4/5.

johannes

Re: [PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Johannes Berg]

On Thu, 2011-07-14 at 13:16 -0700, Thomas Pedersen wrote:
> On Wed, Jul 13, 2011 at 11:25 PM, Jouni Malinen <j@w1.fi> wrote:
> > On Wed, Jul 13, 2011 at 04:45:47PM -0700, Thomas Pedersen wrote:
> >> Even though we don't currently implement PS for mesh points, the
> >> standard asks us to include a TIM element in beacons. Include an empty
> >> element for now.
> >
> > Is it allowed to not support PS in this type of case? AP does not have
> > such option..
> >
> 
> You're right, it doesn't look like PS support is optional. The
> standard requires us to respect the PS states of peers. Support for
> this will require some work, but in the meantime, we can include a TIM
> element indicating no frames buffered here, as well as a Mesh Awake
> Window which never expires.
> 
> Otherwise, until we implement proper PS, might it be better not to
> include the TIM IE at all?

Or just not pretend to support the latest mesh draft ...

I don't think PS support can be hard. We have all of it in AP mode
anyway, so you just need to generalise that a bit, maybe refactor like I
said in my other email, and you should be able to get it up & running
pretty quickly?

johannes

Re: [PATCH 0/5] mac80211: mesh beaconing

[Max Filippov]

> Currently, mesh beacons have IEs either missing or in the wrong order. Update
> the mesh beacon to the proper format specified in 802.11s draft 12.0. This

Thomas, what is the exact draft version number?

> patch set will be part of a greater effort to update the mesh stack in mac80211
> in anticipation of the ratification of the 802.11s standard.

-- 
Thanks.
-- Max

Re: [PATCH 0/5] mac80211: mesh beaconing

[Thomas Pedersen]

On Fri, Jul 15, 2011 at 3:50 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> Currently, mesh beacons have IEs either missing or in the wrong order. Update
>> the mesh beacon to the proper format specified in 802.11s draft 12.0. This
>
> Thomas, what is the exact draft version number?

D12.0 from 5/20/2011 is the latest version I currently have access to.

>
>> patch set will be part of a greater effort to update the mesh stack in mac80211
>> in anticipation of the ratification of the 802.11s standard.
>
> --
> Thanks.
> -- Max
>

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Thomas Pedersen]

On Fri, Jul 15, 2011 at 2:45 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
>
>> Based on Javier Cardona's "mac80211: Support beacon configuration via
>> nl80211 for mesh interfaces", which never made it upstream.
>
> Ok I don't get it -- this patchset seems to apply fine, so what part is
> dependent on Javier's patches?

"derived from" would have been much more accurate wording.

>
> johannes
>
>

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Johannes Berg]

On Fri, 2011-07-15 at 11:36 -0700, Thomas Pedersen wrote:
> On Fri, Jul 15, 2011 at 2:45 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
> >
> >> Based on Javier Cardona's "mac80211: Support beacon configuration via
> >> nl80211 for mesh interfaces", which never made it upstream.
> >
> > Ok I don't get it -- this patchset seems to apply fine, so what part is
> > dependent on Javier's patches?
> 
> "derived from" would have been much more accurate wording.

Oh, I misunderstood, thanks.

johannes

Re: [PATCH 2/5] mac80211: correct mesh beacon head

[Thomas Pedersen]

On Fri, Jul 15, 2011 at 2:57 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
>> Supported Rates and DS Parameter elements go in the beacon head. Add
>> these IEs when we initially construct the beacon.
>
> This breaks mesh_plink_frame_tx() afaict.

Yes, although DS parameters didn't belong there anyway. v2 will fix
this. Is there a better way in mac80211 to add these IEs than just
hardcoding them into the frame construction?

> johannes
>
>

Thanks,
Thomas

Re: [PATCH v2 4/5] mac80211: move RSN IE to tail of mesh beacon

[Thomas Pedersen]

On Fri, Jul 15, 2011 at 2:07 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2011-07-14 at 17:50 -0700, Thomas Pedersen wrote:
>> The mesh specific beacon IEs go right before the vendor IEs. Add a
>> filter that checks for custom IEs (currently just the RSN IE), which
>> belong after the TIM, but before HT. Also, ensure we don't add any
>> custom IEs twice in mesh_mgmt_ies_add().
>>
>> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
>> ---
>>
>> v2:
>>       Ensure room for custom IEs in transmitted beacon (Johannes).
>>       We'll be allocating twice as much space as needed for some IEs
>>       (in tail_len and again ie_len), but it shouldn't be a big deal.
>
> Not that I like allocating twice as much, but I don't really care
> either.
>
> Maybe you should be doing the cleanups in mesh_mgmt_ies_add()
> separately? I'm having a hard time following these changes.

OK, I can factor the changes in this into a single patch.
>
>> +               noffset = ieee80211_ie_split_vendor(ifmsh->ie, ifmsh->ie_len,
>> +                                                   offset);
>> +               if (noffset != offset) {
>
> isn't offset always 0 here? So wouldn't it make more sense to put 0
> explicitly?
>

Sure.

> johannes
>
>

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Thomas Pedersen]

On Fri, Jul 15, 2011 at 2:54 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
>
>> +     if (ieee80211_vif_is_mesh(&sdata->vif))
>> +             rcu_assign_pointer(sdata->u.mesh.beacon, new);
>
> I don't think this compiles if MESH isn't configured into mac80211. Not
> sure adding lots of ifdefs is good, but it's probably the only choice
> right now.

The ifdef is already in ieee80211_vif_is_mesh()

>
>> @@ -461,6 +462,9 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>>  {
>>       struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>>       struct ieee80211_local *local = sdata->local;
>> +     struct ieee80211_mgmt *bcn_head;
>> +     struct beacon_parameters bcn_params;
>> +     u8 *pos;
>>
>>       local->fif_other_bss++;
>>       /* mesh ifaces must set allmulti to forward mcast traffic */
>> @@ -473,7 +477,44 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>>       set_bit(MESH_WORK_HOUSEKEEPING, &ifmsh->wrkq_flags);
>>       ieee80211_mesh_root_setup(ifmsh);
>>       ieee80211_queue_work(&local->hw, &sdata->work);
>> +
>> +     /* Build fixed part of mesh beacon. */
>> +     memset(&bcn_params, 0, sizeof(struct beacon_parameters));
>> +
>> +     /* header + fixed fields + null ssid */
>> +     bcn_params.head_len = 24 + sizeof(bcn_head->u.beacon) + 2;
>> +     pos = kmalloc(bcn_params.head_len, GFP_KERNEL | __GFP_ZERO);
>
> Err, kzalloc, don't use __GFP_ZERO.
>
>> +     if (mac80211_config_ops.add_beacon(sdata->wdev.wiphy, sdata->dev,
>> +                             &bcn_params))
>> +             printk(KERN_ERR "Unable to configure mesh beacon\n");
>
> That's horrible. Don't do that. Just do whatever is necessary inline. If
> you restructure, you only need like 10% of the code from
> ieee80211_config_beacon() anyway.
>
>> @@ -498,6 +540,8 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
>>        * it no longer is.
>>        */
>>       cancel_work_sync(&sdata->work);
>> +     if (sdata->u.mesh.beacon)
>> +             mac80211_config_ops.del_beacon(sdata->wdev.wiphy, sdata->dev);
>
> Ditto.
>
>>       } else if (ieee80211_vif_is_mesh(&sdata->vif)) {
>> -             struct ieee80211_mgmt *mgmt;
>> -             u8 *pos;
>> -
>> +             struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>>  #ifdef CONFIG_MAC80211_MESH
>>               if (!sdata->u.mesh.mesh_id_len)
>>                       goto out;
>>  #endif
>> +             beacon = rcu_dereference(ifmsh->beacon);
>
> not going to compile w/o CONFIG_MAC80211_MESH

I'll look into this.

>
>> +             if (beacon) {
>> +                     /* headroom, head length, tail length, mesh_ies and
>> +                      * maximum TIM length */
>> +                     skb = dev_alloc_skb(local->tx_headroom +
>> +                                         beacon->head_len +
>> +                                         beacon->tail_len + 400);
>
> what's + 400? You can't really think the TIM IE is that long?

No, but you're right, that number is pretty arbitrary. Maximum length
of mesh IEs currently (Mesh ID + Mesh Conf + Mesh Awake) is only 47.
Since mesh IEs have to be added during beacon time, might it make
sense to track the needed length in the if_mesh?

>
>> -             /* headroom, head length, tail length and maximum TIM length */
>> -             skb = dev_alloc_skb(local->tx_headroom + 400 +
>> -                             sdata->u.mesh.ie_len);
>
> I guess it was even there before though ... huh?
>
> Couldn't more code here be shared between mesh and AP? I don't see any
> difference here between mesh and AP.
>
> Also ... since you'll need PS support eventually anyway, maybe you
> should refactor "struct ieee80211_if_ap" into something AP-specific
> (currently only VLANs) and the rest, and then use "the rest" in mesh as
> well to make all this code more unified.

I like this approach much better. Are you suggesting just factoring
out the PS data, or taking it further and creating a generic "if which
beacons"?
> johannes
>
>

Thanks,
Thomas

Re: [PATCH 3/5] mac80211: Ext. Supported Rates go in mesh beacon tail

[Thomas Pedersen]

On Fri, Jul 15, 2011 at 2:58 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
>> Give mesh beacon a tail and maintain Extended Supported Rates IE here.
>> Reorder construction of mesh beacon frame so it looks like:
>>
>> head | tail | mesh IEs | vendor IEs
>>
>> mesh IEs include dynamic elements which must be updated on each beacon.
>
> This is stupid, you're introducing a bug you fix in patch 4/5.
>

True.
> johannes
>
>

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Johannes Berg]

On Fri, 2011-07-15 at 12:30 -0700, Thomas Pedersen wrote:

> >> +     if (ieee80211_vif_is_mesh(&sdata->vif))
> >> +             rcu_assign_pointer(sdata->u.mesh.beacon, new);

> The ifdef is already in ieee80211_vif_is_mesh()

Yeah but sdata->u.mesh doesn't exist without mesh configured.

> > what's + 400? You can't really think the TIM IE is that long?
> 
> No, but you're right, that number is pretty arbitrary. Maximum length
> of mesh IEs currently (Mesh ID + Mesh Conf + Mesh Awake) is only 47.
> Since mesh IEs have to be added during beacon time, might it make
> sense to track the needed length in the if_mesh?

Maybe just add a comment what's included in the 400?

> > Also ... since you'll need PS support eventually anyway, maybe you
> > should refactor "struct ieee80211_if_ap" into something AP-specific
> > (currently only VLANs) and the rest, and then use "the rest" in mesh as
> > well to make all this code more unified.
> 
> I like this approach much better. Are you suggesting just factoring
> out the PS data, or taking it further and creating a generic "if which
> beacons"?

Well, I don't think "iface which beacons" is really what we need here --
IBSS sends beacons as well but we don't really have PS support there and
it'd be different anyway. But on the other hand, if you also factor out
the beacon pointer, you can embed that into both. Like I said, I think
from struct ..._if_ap you only have the vlan list that you don't want?

johannes

Re: [PATCH 2/5] mac80211: correct mesh beacon head

[Johannes Berg]

On Fri, 2011-07-15 at 11:44 -0700, Thomas Pedersen wrote:
> On Fri, Jul 15, 2011 at 2:57 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
> >> Supported Rates and DS Parameter elements go in the beacon head. Add
> >> these IEs when we initially construct the beacon.
> >
> > This breaks mesh_plink_frame_tx() afaict.
> 
> Yes, although DS parameters didn't belong there anyway. v2 will fix
> this. Is there a better way in mac80211 to add these IEs than just
> hardcoding them into the frame construction?

Not sure what you mean, but I guess not.

johannes

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Thomas Pedersen]

On Fri, Jul 15, 2011 at 2:00 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2011-07-15 at 12:30 -0700, Thomas Pedersen wrote:
>
>> >> +     if (ieee80211_vif_is_mesh(&sdata->vif))
>> >> +             rcu_assign_pointer(sdata->u.mesh.beacon, new);
>
>> The ifdef is already in ieee80211_vif_is_mesh()
>
> Yeah but sdata->u.mesh doesn't exist without mesh configured.
>
>> > what's + 400? You can't really think the TIM IE is that long?
>>
>> No, but you're right, that number is pretty arbitrary. Maximum length
>> of mesh IEs currently (Mesh ID + Mesh Conf + Mesh Awake) is only 47.
>> Since mesh IEs have to be added during beacon time, might it make
>> sense to track the needed length in the if_mesh?
>
> Maybe just add a comment what's included in the 400?
>
>> > Also ... since you'll need PS support eventually anyway, maybe you
>> > should refactor "struct ieee80211_if_ap" into something AP-specific
>> > (currently only VLANs) and the rest, and then use "the rest" in mesh as
>> > well to make all this code more unified.
>>
>> I like this approach much better. Are you suggesting just factoring
>> out the PS data, or taking it further and creating a generic "if which
>> beacons"?
>
> Well, I don't think "iface which beacons" is really what we need here --
> IBSS sends beacons as well but we don't really have PS support there and
> it'd be different anyway. But on the other hand, if you also factor out
> the beacon pointer, you can embed that into both. Like I said, I think
> from struct ..._if_ap you only have the vlan list that you don't want?

I'd rather just factor all the PS data out for now, and maintain the
beacon separately. Is there Is there any good reason we'd want to lump
these two together? ieee80211_beacon_add_tim() for example already
treats these as separate.

>
> johannes
>
>

Re: [PATCH 1/5] mac80211: give if_mesh a beacon

[Johannes Berg]

On Sun, 2011-07-17 at 12:36 -0500, Thomas Pedersen wrote:

> >> > Also ... since you'll need PS support eventually anyway, maybe you
> >> > should refactor "struct ieee80211_if_ap" into something AP-specific
> >> > (currently only VLANs) and the rest, and then use "the rest" in mesh as
> >> > well to make all this code more unified.
> >>
> >> I like this approach much better. Are you suggesting just factoring
> >> out the PS data, or taking it further and creating a generic "if which
> >> beacons"?
> >
> > Well, I don't think "iface which beacons" is really what we need here --
> > IBSS sends beacons as well but we don't really have PS support there and
> > it'd be different anyway. But on the other hand, if you also factor out
> > the beacon pointer, you can embed that into both. Like I said, I think
> > from struct ..._if_ap you only have the vlan list that you don't want?
> 
> I'd rather just factor all the PS data out for now, and maintain the
> beacon separately. Is there Is there any good reason we'd want to lump
> these two together? ieee80211_beacon_add_tim() for example already
> treats these as separate.

No that's fine, I see no particular reason to do one or the other. Or
maybe actually I do -- beacon handling requires TX PS handling for the
TIM IE?

johannes

Re: [PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Javier Cardona]

On Fri, Jul 15, 2011 at 3:00 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2011-07-14 at 13:16 -0700, Thomas Pedersen wrote:
>> On Wed, Jul 13, 2011 at 11:25 PM, Jouni Malinen <j@w1.fi> wrote:
>> > On Wed, Jul 13, 2011 at 04:45:47PM -0700, Thomas Pedersen wrote:
>> >> Even though we don't currently implement PS for mesh points, the
>> >> standard asks us to include a TIM element in beacons. Include an empty
>> >> element for now.
>> >
>> > Is it allowed to not support PS in this type of case? AP does not have
>> > such option..
>> >
>>
>> You're right, it doesn't look like PS support is optional. The
>> standard requires us to respect the PS states of peers. Support for
>> this will require some work, but in the meantime, we can include a TIM
>> element indicating no frames buffered here, as well as a Mesh Awake
>> Window which never expires.
>>
>> Otherwise, until we implement proper PS, might it be better not to
>> include the TIM IE at all?
>
> Or just not pretend to support the latest mesh draft ...
>
> I don't think PS support can be hard. We have all of it in AP mode
> anyway, so you just need to generalise that a bit, maybe refactor like I
> said in my other email, and you should be able to get it up & running
> pretty quickly?

Mesh frame buffering can probably reuse most of the AP mode, yes.  The
difficult part is on the receiving STA as it must track the beacon
transmit times of each mesh peer.  I don't know how/if we can
implement this in a vendor-neutral mac80211ish sort of way.

That said, only buffering seems to be mandatory: "A mesh STA SHALL
have the capability to buffer frames and to perform mesh power mode
tracking" but "A mesh STA MAY use mesh power modes to reduce its power
consumption".

Cheers,

Javier


-- 
Javier Cardona
cozybit Inc.
http://www.cozybit.com

Re: [PATCH 5/5] mac80211: mesh beacon includes TIM IE

[Johannes Berg]

On Mon, 2011-07-18 at 07:11 -0700, Javier Cardona wrote:

> > I don't think PS support can be hard. We have all of it in AP mode
> > anyway, so you just need to generalise that a bit, maybe refactor like I
> > said in my other email, and you should be able to get it up & running
> > pretty quickly?
> 
> Mesh frame buffering can probably reuse most of the AP mode, yes.  The
> difficult part is on the receiving STA as it must track the beacon
> transmit times of each mesh peer.  I don't know how/if we can
> implement this in a vendor-neutral mac80211ish sort of way.
> 
> That said, only buffering seems to be mandatory: "A mesh STA SHALL
> have the capability to buffer frames and to perform mesh power mode
> tracking" but "A mesh STA MAY use mesh power modes to reduce its power
> consumption".

Right, if you don't want to go to sleep yourself you shouldn't really
have big issues different from the current PS code. Of course, you might
not have all the nice HW help we have for normal station mode, but
mac80211 can already deal with that (filtered frames etc.)

Implementing actual powersaving will likely be a bigger issue.

johannes