All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] mac80211: consolidate MBSS change notification
@ 2013-02-11 21:07 Thomas Pedersen
  2013-02-11 21:07 ` [PATCH 2/3] mac80211: cache mesh beacon Thomas Pedersen
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Thomas Pedersen @ 2013-02-11 21:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, devel, Thomas Pedersen

A few mesh utility functions will call
ieee80211_bss_info_change_notify(), and then the caller
might notify the driver of the same change again. Avoid
this redundancy by propagating the BSS changes and
generally calling bss_info_change_notify() once per
change.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/cfg.c        |   19 +++++++++++--------
 net/mac80211/mesh.c       |    2 +-
 net/mac80211/mesh.h       |   10 +++++-----
 net/mac80211/mesh_plink.c |   33 ++++++++++++++++++---------------
 net/mac80211/mesh_ps.c    |   24 ++++++++++++++++++------
 5 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 8f6b593..c5a6704 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1261,8 +1261,8 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 
 	if (ieee80211_vif_is_mesh(&sdata->vif)) {
 #ifdef CONFIG_MAC80211_MESH
+		u32 changed = 0;
 		if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED) {
-			u32 changed = 0;
 
 			switch (params->plink_state) {
 			case NL80211_PLINK_ESTAB:
@@ -1272,8 +1272,8 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 				sta->plink_state = params->plink_state;
 
 				ieee80211_mps_sta_status_update(sta);
-				ieee80211_mps_set_sta_local_pm(sta,
-					sdata->u.mesh.mshcfg.power_mode);
+				changed |= ieee80211_mps_set_sta_local_pm(sta,
+					      sdata->u.mesh.mshcfg.power_mode);
 				break;
 			case NL80211_PLINK_LISTEN:
 			case NL80211_PLINK_BLOCKED:
@@ -1287,26 +1287,29 @@ static int sta_apply_parameters(struct ieee80211_local *local,
 				sta->plink_state = params->plink_state;
 
 				ieee80211_mps_sta_status_update(sta);
-				ieee80211_mps_local_status_update(sdata);
+				changed |=
+				      ieee80211_mps_local_status_update(sdata);
 				break;
 			default:
 				/*  nothing  */
 				break;
 			}
-			ieee80211_bss_info_change_notify(sdata, changed);
 		} else {
 			switch (params->plink_action) {
 			case PLINK_ACTION_OPEN:
-				mesh_plink_open(sta);
+				changed |= mesh_plink_open(sta);
 				break;
 			case PLINK_ACTION_BLOCK:
-				mesh_plink_block(sta);
+				changed |= mesh_plink_block(sta);
 				break;
 			}
 		}
 
 		if (params->local_pm)
-			ieee80211_mps_set_sta_local_pm(sta, params->local_pm);
+			changed |=
+			      ieee80211_mps_set_sta_local_pm(sta,
+							     params->local_pm);
+		ieee80211_bss_info_change_notify(sdata, changed);
 #endif
 	}
 
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 2bf0158..0adec3d 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -675,7 +675,7 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	sdata->vif.bss_conf.basic_rates =
 		ieee80211_mandatory_rates(local, band);
 
-	ieee80211_mps_local_status_update(sdata);
+	changed |= ieee80211_mps_local_status_update(sdata);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 3b9d862..7ad035f 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -245,9 +245,9 @@ void ieee80211_mesh_root_setup(struct ieee80211_if_mesh *ifmsh);
 const struct ieee80211_mesh_sync_ops *ieee80211_mesh_sync_ops_get(u8 method);
 
 /* mesh power save */
-void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata);
-void ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
-				    enum nl80211_mesh_power_mode pm);
+u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata);
+u32 ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
+				   enum nl80211_mesh_power_mode pm);
 void ieee80211_mps_set_frame_flags(struct ieee80211_sub_if_data *sdata,
 				   struct sta_info *sta,
 				   struct ieee80211_hdr *hdr);
@@ -289,8 +289,8 @@ bool mesh_peer_accepts_plinks(struct ieee802_11_elems *ie);
 u32 mesh_accept_plinks_update(struct ieee80211_sub_if_data *sdata);
 void mesh_plink_broken(struct sta_info *sta);
 u32 mesh_plink_deactivate(struct sta_info *sta);
-int mesh_plink_open(struct sta_info *sta);
-void mesh_plink_block(struct sta_info *sta);
+u32 mesh_plink_open(struct sta_info *sta);
+u32 mesh_plink_block(struct sta_info *sta);
 void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata,
 			 struct ieee80211_mgmt *mgmt, size_t len,
 			 struct ieee80211_rx_status *rx_status);
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index 56c9b31..a4c7a7e 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -202,7 +202,7 @@ static u32 __mesh_plink_deactivate(struct sta_info *sta)
 	mesh_path_flush_by_nexthop(sta);
 
 	ieee80211_mps_sta_status_update(sta);
-	ieee80211_mps_local_status_update(sdata);
+	changed |= ieee80211_mps_local_status_update(sdata);
 
 	return changed;
 }
@@ -494,6 +494,7 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
 			   struct ieee802_11_elems *elems)
 {
 	struct sta_info *sta;
+	u32 changed = 0;
 
 	sta = mesh_sta_info_get(sdata, hw_addr, elems);
 	if (!sta)
@@ -504,11 +505,12 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
 	    sdata->u.mesh.accepting_plinks &&
 	    sdata->u.mesh.mshcfg.auto_open_plinks &&
 	    rssi_threshold_check(sta, sdata))
-		mesh_plink_open(sta);
+		changed = mesh_plink_open(sta);
 
 	ieee80211_mps_frame_release(sta, elems);
 out:
 	rcu_read_unlock();
+	ieee80211_bss_info_change_notify(sdata, changed);
 }
 
 static void mesh_plink_timer(unsigned long data)
@@ -621,13 +623,14 @@ static inline void mesh_plink_timer_set(struct sta_info *sta, int timeout)
 	add_timer(&sta->plink_timer);
 }
 
-int mesh_plink_open(struct sta_info *sta)
+u32 mesh_plink_open(struct sta_info *sta)
 {
 	__le16 llid;
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
+	u32 changed;
 
 	if (!test_sta_flag(sta, WLAN_STA_AUTH))
-		return -EPERM;
+		return 0;
 
 	spin_lock_bh(&sta->lock);
 	get_random_bytes(&llid, 2);
@@ -635,7 +638,7 @@ int mesh_plink_open(struct sta_info *sta)
 	if (sta->plink_state != NL80211_PLINK_LISTEN &&
 	    sta->plink_state != NL80211_PLINK_BLOCKED) {
 		spin_unlock_bh(&sta->lock);
-		return -EBUSY;
+		return 0;
 	}
 	sta->plink_state = NL80211_PLINK_OPN_SNT;
 	mesh_plink_timer_set(sta, sdata->u.mesh.mshcfg.dot11MeshRetryTimeout);
@@ -645,15 +648,15 @@ int mesh_plink_open(struct sta_info *sta)
 		sta->sta.addr);
 
 	/* set the non-peer mode to active during peering */
-	ieee80211_mps_local_status_update(sdata);
+	changed = ieee80211_mps_local_status_update(sdata);
 
-	return mesh_plink_frame_tx(sdata, WLAN_SP_MESH_PEERING_OPEN,
-				   sta->sta.addr, llid, 0, 0);
+	mesh_plink_frame_tx(sdata, WLAN_SP_MESH_PEERING_OPEN,
+			    sta->sta.addr, llid, 0, 0);
+	return changed;
 }
 
-void mesh_plink_block(struct sta_info *sta)
+u32 mesh_plink_block(struct sta_info *sta)
 {
-	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	u32 changed;
 
 	spin_lock_bh(&sta->lock);
@@ -661,7 +664,7 @@ void mesh_plink_block(struct sta_info *sta)
 	sta->plink_state = NL80211_PLINK_BLOCKED;
 	spin_unlock_bh(&sta->lock);
 
-	ieee80211_bss_info_change_notify(sdata, changed);
+	return changed;
 }
 
 
@@ -882,7 +885,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
 					     mshcfg->dot11MeshRetryTimeout);
 
 			/* set the non-peer mode to active during peering */
-			ieee80211_mps_local_status_update(sdata);
+			changed |= ieee80211_mps_local_status_update(sdata);
 
 			spin_unlock_bh(&sta->lock);
 			mesh_plink_frame_tx(sdata,
@@ -978,7 +981,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
 			mpl_dbg(sdata, "Mesh plink with %pM ESTABLISHED\n",
 				sta->sta.addr);
 			ieee80211_mps_sta_status_update(sta);
-			ieee80211_mps_set_sta_local_pm(sta,
+			changed |= ieee80211_mps_set_sta_local_pm(sta,
 						       mshcfg->power_mode);
 			break;
 		default:
@@ -1020,8 +1023,8 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
 					    WLAN_SP_MESH_PEERING_CONFIRM,
 					    sta->sta.addr, llid, plid, 0);
 			ieee80211_mps_sta_status_update(sta);
-			ieee80211_mps_set_sta_local_pm(sta,
-						       mshcfg->power_mode);
+			changed |= ieee80211_mps_set_sta_local_pm(sta,
+							mshcfg->power_mode);
 			break;
 		default:
 			spin_unlock_bh(&sta->lock);
diff --git a/net/mac80211/mesh_ps.c b/net/mac80211/mesh_ps.c
index b677962..aa83eac 100644
--- a/net/mac80211/mesh_ps.c
+++ b/net/mac80211/mesh_ps.c
@@ -74,14 +74,17 @@ static void mps_qos_null_tx(struct sta_info *sta)
  * @sdata: local mesh subif
  *
  * sets the non-peer power mode and triggers the driver PS (re-)configuration
+ * Return BSS_CHANGED_BEACON if a beacon update is necessary.
  */
-void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
+u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 	struct sta_info *sta;
 	bool peering = false;
 	int light_sleep_cnt = 0;
 	int deep_sleep_cnt = 0;
+	u32 changed = 0;
+	enum nl80211_mesh_power_mode nonpeer_pm;
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(sta, &sdata->local->sta_list, list) {
@@ -115,17 +118,25 @@ void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
 	 */
 	if (peering) {
 		mps_dbg(sdata, "setting non-peer PM to active for peering\n");
-		ifmsh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
+		nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
 	} else if (light_sleep_cnt || deep_sleep_cnt) {
 		mps_dbg(sdata, "setting non-peer PM to deep sleep\n");
-		ifmsh->nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
+		nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
 	} else {
 		mps_dbg(sdata, "setting non-peer PM to user value\n");
-		ifmsh->nonpeer_pm = ifmsh->mshcfg.power_mode;
+		nonpeer_pm = ifmsh->mshcfg.power_mode;
 	}
 
+	if (ifmsh->nonpeer_pm != nonpeer_pm ||
+	    ifmsh->ps_peers_light_sleep != light_sleep_cnt ||
+	    ifmsh->ps_peers_deep_sleep != deep_sleep_cnt)
+		changed = BSS_CHANGED_BEACON;
+
+	ifmsh->nonpeer_pm = nonpeer_pm;
 	ifmsh->ps_peers_light_sleep = light_sleep_cnt;
 	ifmsh->ps_peers_deep_sleep = deep_sleep_cnt;
+
+	return changed;
 }
 
 /**
@@ -133,8 +144,9 @@ void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
  *
  * @sta: mesh STA
  * @pm: the power mode to set
+ * Return BSS_CHANGED_BEACON if a beacon update is in order.
  */
-void ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
+u32 ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
 				    enum nl80211_mesh_power_mode pm)
 {
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
@@ -151,7 +163,7 @@ void ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
 	if (sta->plink_state == NL80211_PLINK_ESTAB)
 		mps_qos_null_tx(sta);
 
-	ieee80211_mps_local_status_update(sdata);
+	return ieee80211_mps_local_status_update(sdata);
 }
 
 /**
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] mac80211: cache mesh beacon
  2013-02-11 21:07 [PATCH 1/3] mac80211: consolidate MBSS change notification Thomas Pedersen
@ 2013-02-11 21:07 ` Thomas Pedersen
  2013-02-11 21:17   ` Johannes Berg
  2013-02-11 21:07 ` [PATCH 3/3] mac80211: generate mesh probe responses Thomas Pedersen
  2013-02-12 17:56 ` [PATCH 1/3] mac80211: consolidate MBSS change notification Marco Porsch
  2 siblings, 1 reply; 10+ messages in thread
From: Thomas Pedersen @ 2013-02-11 21:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, devel, Thomas Pedersen

Previously, the entire mesh beacon would be generated each
time the beacon timer fired. Instead generate a beacon
head and tail (so the TIM can easily be inserted when mesh
power save is on) when starting a mesh or the MBSS
parameters change.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/cfg.c         |    5 +-
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mesh.c        |  145 +++++++++++++++++++++++++++++++++++++++++++-
 net/mac80211/mesh.h        |    3 +
 net/mac80211/mesh_plink.c  |    4 +-
 net/mac80211/tx.c          |   61 +++----------------
 6 files changed, 159 insertions(+), 60 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index c5a6704..dd70568 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1802,11 +1802,10 @@ static int ieee80211_update_mesh_config(struct wiphy *wiphy,
 		conf->power_mode = nconf->power_mode;
 		ieee80211_mps_local_status_update(sdata);
 	}
-	if (_chg_mesh_attr(NL80211_MESHCONF_AWAKE_WINDOW, mask)) {
+	if (_chg_mesh_attr(NL80211_MESHCONF_AWAKE_WINDOW, mask))
 		conf->dot11MeshAwakeWindowDuration =
 			nconf->dot11MeshAwakeWindowDuration;
-		ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
-	}
+	ieee80211_mbss_info_change_notify(sdata, BSS_CHANGED_BEACON);
 	return 0;
 }
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 5635dfc..5f71ebe 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -578,6 +578,7 @@ struct ieee80211_if_mesh {
 	u32 mesh_seqnum;
 	bool accepting_plinks;
 	int num_gates;
+	struct beacon_data __rcu *beacon;
 	const u8 *ie;
 	u8 ie_len;
 	enum {
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 0adec3d..ffbe1d5 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -171,7 +171,7 @@ void mesh_sta_cleanup(struct sta_info *sta)
 	}
 
 	if (changed)
-		ieee80211_bss_info_change_notify(sdata, changed);
+		ieee80211_mbss_info_change_notify(sdata, changed);
 }
 
 int mesh_rmc_init(struct ieee80211_sub_if_data *sdata)
@@ -593,7 +593,7 @@ static void ieee80211_mesh_housekeeping(struct ieee80211_sub_if_data *sdata,
 	mesh_path_expire(sdata);
 
 	changed = mesh_accept_plinks_update(sdata);
-	ieee80211_bss_info_change_notify(sdata, changed);
+	ieee80211_mbss_info_change_notify(sdata, changed);
 
 	mod_timer(&ifmsh->housekeeping_timer,
 		  round_jiffies(jiffies + IEEE80211_MESH_HOUSEKEEPING_INTERVAL));
@@ -644,6 +644,140 @@ void ieee80211_mesh_restart(struct ieee80211_sub_if_data *sdata)
 }
 #endif
 
+static int
+ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
+{
+	struct beacon_data *bcn;
+	int head_len, tail_len;
+	struct sk_buff *skb;
+	struct ieee80211_mgmt *mgmt;
+	struct ieee80211_chanctx_conf *chanctx_conf;
+	enum ieee80211_band band;
+	u8 *pos;
+	struct ieee80211_sub_if_data *sdata;
+	int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
+		      sizeof(mgmt->u.beacon);
+
+	sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
+	rcu_read_lock();
+	chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+	band = chanctx_conf->def.chan->band;
+	rcu_read_unlock();
+
+	head_len = hdr_len +
+		   2 + /* NULL SSID */
+		   2 + 8 + /* supported rates */
+		   2 + 3; /* DS params */
+	tail_len = 2 + (IEEE80211_MAX_SUPP_RATES - 8) +
+		   2 + sizeof(struct ieee80211_ht_cap) +
+		   2 + sizeof(struct ieee80211_ht_operation) +
+		   2 + ifmsh->mesh_id_len +
+		   2 + sizeof(struct ieee80211_meshconf_ie) +
+		   2 + sizeof(__le16) + /* awake window */
+		   ifmsh->ie_len;
+
+	bcn = kzalloc(sizeof(*bcn) + head_len + tail_len, GFP_KERNEL);
+	/* need an skb for IE builders to operate on */
+	skb = dev_alloc_skb(max(head_len, tail_len));
+
+	if (!bcn || !skb)
+		goto out_free;
+
+	/*
+	 * pointers go into the block we allocated,
+	 * memory is | beacon_data | head | tail |
+	 */
+	bcn->head = ((u8 *) bcn) + sizeof(*bcn);
+
+	/* fill in the head */
+	mgmt = (struct ieee80211_mgmt *) skb_put(skb, hdr_len);
+	memset(mgmt, 0, hdr_len);
+	mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
+					  IEEE80211_STYPE_BEACON);
+	eth_broadcast_addr(mgmt->da);
+	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 |= cpu_to_le16(
+		sdata->u.mesh.security ? WLAN_CAPABILITY_PRIVACY : 0);
+
+	pos = skb_put(skb, 2);
+	*pos++ = WLAN_EID_SSID;
+	*pos++ = 0x0;
+
+	if (ieee80211_add_srates_ie(sdata, skb, true, band) ||
+	    mesh_add_ds_params_ie(skb, sdata)) {
+		mpl_dbg(sdata, "couldn't add beacon head IEs\n");
+		goto out_free;
+	}
+	bcn->head_len = skb->len;
+	memcpy(bcn->head, skb->data, bcn->head_len);
+
+	/* now the tail */
+	skb_trim(skb, 0);
+	bcn->tail = bcn->head + bcn->head_len;
+
+	if (ieee80211_add_ext_srates_ie(sdata, skb, true, band) ||
+	    mesh_add_rsn_ie(skb, sdata) ||
+	    mesh_add_ht_cap_ie(skb, sdata) ||
+	    mesh_add_ht_oper_ie(skb, sdata) ||
+	    mesh_add_meshid_ie(skb, sdata) ||
+	    mesh_add_meshconf_ie(skb, sdata) ||
+	    mesh_add_awake_window_ie(skb, sdata) ||
+	    mesh_add_vendor_ies(skb, sdata)) {
+		mpl_dbg(sdata, "couldn't add beacon tail IEs!\n");
+		goto out_free;
+	}
+	bcn->tail_len = skb->len;
+	memcpy(bcn->tail, skb->data, bcn->tail_len);
+
+	dev_kfree_skb(skb);
+	rcu_assign_pointer(ifmsh->beacon, bcn);
+	return 0;
+out_free:
+	kfree(bcn);
+	dev_kfree_skb(skb);
+	return -ENOMEM;
+}
+
+static int
+ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
+{
+	struct ieee80211_sub_if_data *sdata;
+	struct beacon_data *old_bcn;
+	int ret;
+	sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
+
+	rcu_read_lock();
+	old_bcn = rcu_dereference(ifmsh->beacon);
+	ret = ieee80211_mesh_build_beacon(ifmsh);
+	if (ret) {
+		mpl_dbg(sdata, "couldn't rebuild mesh beacon! %d\n", ret);
+		/* just reuse old beacon */
+		goto out;
+	}
+
+	kfree_rcu(old_bcn, rcu_head);
+out:
+	rcu_read_unlock();
+	return ret;
+}
+
+void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,
+				       u32 changed)
+{
+	if (sdata->vif.bss_conf.enable_beacon &&
+	    (changed & (BSS_CHANGED_BEACON |
+			BSS_CHANGED_HT |
+			BSS_CHANGED_BASIC_RATES |
+			BSS_CHANGED_BEACON_INT)))
+		if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
+			return;
+	ieee80211_bss_info_change_notify(sdata, changed);
+	return;
+}
+
 void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
@@ -677,6 +811,11 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 
 	changed |= ieee80211_mps_local_status_update(sdata);
 
+	if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {
+		ieee80211_stop_mesh(sdata);
+		return;
+	}
+
 	ieee80211_bss_info_change_notify(sdata, changed);
 
 	netif_carrier_on(sdata->dev);
@@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
 	sdata->vif.bss_conf.enable_beacon = false;
 	clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
 	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
+	kfree_rcu(ifmsh->beacon, rcu_head);
 
 	/* flush STAs and mpaths on this iface */
 	sta_info_flush(sdata);
@@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
 	skb_queue_head_init(&ifmsh->ps.bc_buf);
 	spin_lock_init(&ifmsh->mesh_preq_queue_lock);
 	spin_lock_init(&ifmsh->sync_offset_lock);
+	RCU_INIT_POINTER(ifmsh->beacon, NULL);
 
 	sdata->vif.bss_conf.bssid = zero_addr;
 }
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 7ad035f..3ce00ba 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -243,6 +243,9 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata);
 void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata);
 void ieee80211_mesh_root_setup(struct ieee80211_if_mesh *ifmsh);
 const struct ieee80211_mesh_sync_ops *ieee80211_mesh_sync_ops_get(u8 method);
+/* wrapper for ieee80211_bss_info_change_notify() */
+void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,
+				       u32 changed);
 
 /* mesh power save */
 u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata);
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index a4c7a7e..13983b6 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -510,7 +510,7 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
 	ieee80211_mps_frame_release(sta, elems);
 out:
 	rcu_read_unlock();
-	ieee80211_bss_info_change_notify(sdata, changed);
+	ieee80211_mbss_info_change_notify(sdata, changed);
 }
 
 static void mesh_plink_timer(unsigned long data)
@@ -1092,5 +1092,5 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
 	rcu_read_unlock();
 
 	if (changed)
-		ieee80211_bss_info_change_notify(sdata, changed);
+		ieee80211_mbss_info_change_notify(sdata, changed);
 }
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 2ef0e19..1da61fd 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2431,71 +2431,26 @@ 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;
 		struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
-		u8 *pos;
-		int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
-			      sizeof(mgmt->u.beacon);
+		struct beacon_data *bcn = rcu_dereference(ifmsh->beacon);
 
-#ifdef CONFIG_MAC80211_MESH
-		if (!sdata->u.mesh.mesh_id_len)
+		if (!bcn)
 			goto out;
-#endif
 
 		if (ifmsh->sync_ops)
 			ifmsh->sync_ops->adjust_tbtt(
 						sdata);
 
 		skb = dev_alloc_skb(local->tx_headroom +
-				    hdr_len +
-				    2 + /* NULL SSID */
-				    2 + 8 + /* supported rates */
-				    2 + 3 + /* DS params */
+				    bcn->head_len +
 				    256 + /* TIM IE */
-				    2 + (IEEE80211_MAX_SUPP_RATES - 8) +
-				    2 + sizeof(struct ieee80211_ht_cap) +
-				    2 + sizeof(struct ieee80211_ht_operation) +
-				    2 + sdata->u.mesh.mesh_id_len +
-				    2 + sizeof(struct ieee80211_meshconf_ie) +
-				    sdata->u.mesh.ie_len +
-				    2 + sizeof(__le16)); /* awake window */
+				    bcn->tail_len);
 		if (!skb)
 			goto out;
-
-		skb_reserve(skb, local->hw.extra_tx_headroom);
-		mgmt = (struct ieee80211_mgmt *) skb_put(skb, hdr_len);
-		memset(mgmt, 0, hdr_len);
-		mgmt->frame_control =
-		    cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_BEACON);
-		eth_broadcast_addr(mgmt->da);
-		memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
-		memcpy(mgmt->bssid, sdata->vif.addr, ETH_ALEN);
-		ieee80211_mps_set_frame_flags(sdata, NULL, (void *) mgmt);
-		mgmt->u.beacon.beacon_int =
-			cpu_to_le16(sdata->vif.bss_conf.beacon_int);
-		mgmt->u.beacon.capab_info |= cpu_to_le16(
-			sdata->u.mesh.security ? WLAN_CAPABILITY_PRIVACY : 0);
-
-		pos = skb_put(skb, 2);
-		*pos++ = WLAN_EID_SSID;
-		*pos++ = 0x0;
-
-		band = chanctx_conf->def.chan->band;
-
-		if (ieee80211_add_srates_ie(sdata, skb, true, band) ||
-		    mesh_add_ds_params_ie(skb, sdata) ||
-		    ieee80211_beacon_add_tim(sdata, &ifmsh->ps, skb) ||
-		    ieee80211_add_ext_srates_ie(sdata, skb, true, band) ||
-		    mesh_add_rsn_ie(skb, sdata) ||
-		    mesh_add_ht_cap_ie(skb, sdata) ||
-		    mesh_add_ht_oper_ie(skb, sdata) ||
-		    mesh_add_meshid_ie(skb, sdata) ||
-		    mesh_add_meshconf_ie(skb, sdata) ||
-		    mesh_add_awake_window_ie(skb, sdata) ||
-		    mesh_add_vendor_ies(skb, sdata)) {
-			pr_err("o11s: couldn't add ies!\n");
-			goto out;
-		}
+		skb_reserve(skb, local->tx_headroom);
+		memcpy(skb_put(skb, bcn->head_len), bcn->head, bcn->head_len);
+		ieee80211_beacon_add_tim(sdata, &ifmsh->ps, skb);
+		memcpy(skb_put(skb, bcn->tail_len), bcn->tail, bcn->tail_len);
 	} else {
 		WARN_ON(1);
 		goto out;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] mac80211: generate mesh probe responses
  2013-02-11 21:07 [PATCH 1/3] mac80211: consolidate MBSS change notification Thomas Pedersen
  2013-02-11 21:07 ` [PATCH 2/3] mac80211: cache mesh beacon Thomas Pedersen
@ 2013-02-11 21:07 ` Thomas Pedersen
  2013-02-11 21:20   ` Johannes Berg
  2013-02-12 17:56 ` [PATCH 1/3] mac80211: consolidate MBSS change notification Marco Porsch
  2 siblings, 1 reply; 10+ messages in thread
From: Thomas Pedersen @ 2013-02-11 21:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, devel, Thomas Pedersen

Mesh interfaces will now respond to any broadcast (or
matching directed mesh) probe requests with a probe
response.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/mesh.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/rx.c   |    5 ++--
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index ffbe1d5..0bb7754 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -862,6 +862,66 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
 	sdata->u.mesh.timers_running = 0;
 }
 
+static void
+ieee80211_mesh_rx_probe_req(struct ieee80211_sub_if_data *sdata,
+			    struct ieee80211_mgmt *mgmt, size_t len)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+	struct sk_buff *presp;
+	struct beacon_data *bcn;
+	struct ieee80211_mgmt *hdr;
+	struct ieee802_11_elems elems;
+	size_t baselen;
+	u8 *pos, *end;
+
+	end = ((u8 *) mgmt) + len;
+	pos = mgmt->u.probe_req.variable;
+	baselen = (u8 *) pos - (u8 *) mgmt;
+	if (baselen > len)
+		return;
+
+	ieee802_11_parse_elems(pos, len - baselen, &elems);
+
+	/* 802.11-2012 10.1.4.3.2 */
+	if ((!ether_addr_equal(mgmt->da, sdata->vif.addr) &&
+	     !is_broadcast_ether_addr(mgmt->da)) ||
+	    elems.ssid_len != 0)
+		return;
+
+	if (elems.mesh_id_len != 0 &&
+	    (elems.mesh_id_len != ifmsh->mesh_id_len ||
+	     memcmp(elems.mesh_id, ifmsh->mesh_id, ifmsh->mesh_id_len))) {
+		mpl_dbg(sdata, "ignoring probe request for different MBSS\n");
+		return;
+	}
+
+	rcu_read_lock();
+	bcn = rcu_dereference(ifmsh->beacon);
+
+	if (!bcn)
+		goto out;
+
+	presp = dev_alloc_skb(local->tx_headroom +
+			      bcn->head_len + bcn->tail_len);
+	if (WARN_ON(!presp))
+		goto out;
+
+	skb_reserve(presp, local->tx_headroom);
+	memcpy(skb_put(presp, bcn->head_len), bcn->head, bcn->head_len);
+	memcpy(skb_put(presp, bcn->tail_len), bcn->tail, bcn->tail_len);
+	hdr = (struct ieee80211_mgmt *) presp->data;
+	hdr->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
+					 IEEE80211_STYPE_PROBE_RESP);
+	memcpy(hdr->da, mgmt->sa, ETH_ALEN);
+	mpl_dbg(sdata, "sending probe resp. to %pM\n", hdr->da);
+	IEEE80211_SKB_CB(presp)->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
+	ieee80211_tx_skb(sdata, presp);
+out:
+	rcu_read_unlock();
+	return;
+}
+
 static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
 					u16 stype,
 					struct ieee80211_mgmt *mgmt,
@@ -951,6 +1011,9 @@ void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
 		ieee80211_mesh_rx_bcn_presp(sdata, stype, mgmt, skb->len,
 					    rx_status);
 		break;
+	case IEEE80211_STYPE_PROBE_REQ:
+		ieee80211_mesh_rx_probe_req(sdata, mgmt, skb->len);
+		break;
 	case IEEE80211_STYPE_ACTION:
 		ieee80211_mesh_rx_mgmt_action(sdata, mgmt, skb->len, rx_status);
 		break;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index b5f1bba..34aa975 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2692,8 +2692,9 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_data *rx)
 			return RX_DROP_MONITOR;
 		break;
 	case cpu_to_le16(IEEE80211_STYPE_PROBE_REQ):
-		/* process only for ibss */
-		if (sdata->vif.type != NL80211_IFTYPE_ADHOC)
+		/* process only for ibss and mesh */
+		if (sdata->vif.type != NL80211_IFTYPE_ADHOC &&
+		    sdata->vif.type != NL80211_IFTYPE_MESH_POINT)
 			return RX_DROP_MONITOR;
 		break;
 	default:
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] mac80211: cache mesh beacon
  2013-02-11 21:07 ` [PATCH 2/3] mac80211: cache mesh beacon Thomas Pedersen
@ 2013-02-11 21:17   ` Johannes Berg
  2013-02-11 21:24     ` Thomas Pedersen
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2013-02-11 21:17 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel

On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:

> +static int
> +ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
> +{
> +	struct beacon_data *bcn;
> +	int head_len, tail_len;
> +	struct sk_buff *skb;
> +	struct ieee80211_mgmt *mgmt;
> +	struct ieee80211_chanctx_conf *chanctx_conf;
> +	enum ieee80211_band band;
> +	u8 *pos;
> +	struct ieee80211_sub_if_data *sdata;
> +	int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
> +		      sizeof(mgmt->u.beacon);
> +
> +	sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> +	rcu_read_lock();

This is weird since you already did it outside the function?

> +	chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> +	band = chanctx_conf->def.chan->band;

could be NULL? I guess not at this point though.

> +static int
> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
> +{
> +	struct ieee80211_sub_if_data *sdata;
> +	struct beacon_data *old_bcn;
> +	int ret;
> +	sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> +
> +	rcu_read_lock();
> +	old_bcn = rcu_dereference(ifmsh->beacon);
> +	ret = ieee80211_mesh_build_beacon(ifmsh);

This looks totally wrong. You must protect the assignment to
ifmsg->beacon by some lock, so then you don't need the rcu_read_lock()
here since you're under that lock, so this should be
rcu_dereference_protected(..., lockdep_is_held(whatever_lock));

> +	if (sdata->vif.bss_conf.enable_beacon &&
> +	    (changed & (BSS_CHANGED_BEACON |
> +			BSS_CHANGED_HT |
> +			BSS_CHANGED_BASIC_RATES |
> +			BSS_CHANGED_BEACON_INT)))
> +		if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
> +			return;

Does that return make any sense?

>  	changed |= ieee80211_mps_local_status_update(sdata);
>  
> +	if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {

no warning for memory allocation failures please

> @@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
>  	sdata->vif.bss_conf.enable_beacon = false;
>  	clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
>  	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
> +	kfree_rcu(ifmsh->beacon, rcu_head);

I think you should set it to NULL first, just so it's clearer.
 
> @@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
>  	skb_queue_head_init(&ifmsh->ps.bc_buf);
>  	spin_lock_init(&ifmsh->mesh_preq_queue_lock);
>  	spin_lock_init(&ifmsh->sync_offset_lock);
> +	RCU_INIT_POINTER(ifmsh->beacon, NULL);

Isn't everything initialized to 0/null?

johannes


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] mac80211: generate mesh probe responses
  2013-02-11 21:07 ` [PATCH 3/3] mac80211: generate mesh probe responses Thomas Pedersen
@ 2013-02-11 21:20   ` Johannes Berg
  2013-02-11 22:11     ` Thomas Pedersen
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2013-02-11 21:20 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel

On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:

> +	if (elems.mesh_id_len != 0 &&
> +	    (elems.mesh_id_len != ifmsh->mesh_id_len ||
> +	     memcmp(elems.mesh_id, ifmsh->mesh_id, ifmsh->mesh_id_len))) {
> +		mpl_dbg(sdata, "ignoring probe request for different MBSS\n");

> +	presp = dev_alloc_skb(local->tx_headroom +
> +			      bcn->head_len + bcn->tail_len);
> +	if (WARN_ON(!presp))
> +		goto out;

same as before.


> +	mpl_dbg(sdata, "sending probe resp. to %pM\n", hdr->da);

I also think you should probably drop both mpl_dbg() statements as
they're likely to just be noise for any other debugging when you
actually use this?

johannes


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] mac80211: cache mesh beacon
  2013-02-11 21:17   ` Johannes Berg
@ 2013-02-11 21:24     ` Thomas Pedersen
  2013-02-11 21:34       ` Johannes Berg
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Pedersen @ 2013-02-11 21:24 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, devel

On Mon, Feb 11, 2013 at 1:17 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:
>
>> +static int
>> +ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
>> +{
>> +     struct beacon_data *bcn;
>> +     int head_len, tail_len;
>> +     struct sk_buff *skb;
>> +     struct ieee80211_mgmt *mgmt;
>> +     struct ieee80211_chanctx_conf *chanctx_conf;
>> +     enum ieee80211_band band;
>> +     u8 *pos;
>> +     struct ieee80211_sub_if_data *sdata;
>> +     int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
>> +                   sizeof(mgmt->u.beacon);
>> +
>> +     sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
>> +     rcu_read_lock();
>
> This is weird since you already did it outside the function?

Yes, but we shouldn't rely on the caller creating an RCU read section?

>> +     chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> +     band = chanctx_conf->def.chan->band;
>
> could be NULL? I guess not at this point though.

Yeah it will have been set.

>> +static int
>> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
>> +{
>> +     struct ieee80211_sub_if_data *sdata;
>> +     struct beacon_data *old_bcn;
>> +     int ret;
>> +     sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
>> +
>> +     rcu_read_lock();
>> +     old_bcn = rcu_dereference(ifmsh->beacon);
>> +     ret = ieee80211_mesh_build_beacon(ifmsh);
>
> This looks totally wrong. You must protect the assignment to
> ifmsg->beacon by some lock, so then you don't need the rcu_read_lock()
> here since you're under that lock, so this should be
> rcu_dereference_protected(..., lockdep_is_held(whatever_lock));

OK, I guess we better protect assignment by some lock then :)

>> +     if (sdata->vif.bss_conf.enable_beacon &&
>> +         (changed & (BSS_CHANGED_BEACON |
>> +                     BSS_CHANGED_HT |
>> +                     BSS_CHANGED_BASIC_RATES |
>> +                     BSS_CHANGED_BEACON_INT)))
>> +             if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
>> +                     return;
>
> Does that return make any sense?

The alternative is to keep notifying the driver. I just wanted to stop
everything since we're out of memory, but we can keep calling
bss_info_change_notify() is you think that makes more sense.

>>       changed |= ieee80211_mps_local_status_update(sdata);
>>
>> +     if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {
>
> no warning for memory allocation failures please

OK.

>> @@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
>>       sdata->vif.bss_conf.enable_beacon = false;
>>       clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
>>       ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
>> +     kfree_rcu(ifmsh->beacon, rcu_head);
>
> I think you should set it to NULL first, just so it's clearer.

For who? It seems there is no need in this path, but OK.

>> @@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
>>       skb_queue_head_init(&ifmsh->ps.bc_buf);
>>       spin_lock_init(&ifmsh->mesh_preq_queue_lock);
>>       spin_lock_init(&ifmsh->sync_offset_lock);
>> +     RCU_INIT_POINTER(ifmsh->beacon, NULL);
>
> Isn't everything initialized to 0/null?

Yep, I wanted any needed RCU magic to happen here though.

-- 
Thomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] mac80211: cache mesh beacon
  2013-02-11 21:24     ` Thomas Pedersen
@ 2013-02-11 21:34       ` Johannes Berg
  0 siblings, 0 replies; 10+ messages in thread
From: Johannes Berg @ 2013-02-11 21:34 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel

On Mon, 2013-02-11 at 13:24 -0800, Thomas Pedersen wrote:

> >> +     sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> >> +     rcu_read_lock();
> >
> > This is weird since you already did it outside the function?
> 
> Yes, but we shouldn't rely on the caller creating an RCU read section?

I don't really see a problem with that, but the other locking issue
means that you need this anyway.

> >> +static int
> >> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
> >> +{
> >> +     struct ieee80211_sub_if_data *sdata;
> >> +     struct beacon_data *old_bcn;
> >> +     int ret;
> >> +     sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> >> +
> >> +     rcu_read_lock();
> >> +     old_bcn = rcu_dereference(ifmsh->beacon);
> >> +     ret = ieee80211_mesh_build_beacon(ifmsh);
> >
> > This looks totally wrong. You must protect the assignment to
> > ifmsg->beacon by some lock, so then you don't need the rcu_read_lock()
> > here since you're under that lock, so this should be
> > rcu_dereference_protected(..., lockdep_is_held(whatever_lock));
> 
> OK, I guess we better protect assignment by some lock then :)

I'm sure there's some lock already? Otherwise doing mesh operations from
userspace and the workqueue would probably be quite racy?

> >> +     if (sdata->vif.bss_conf.enable_beacon &&
> >> +         (changed & (BSS_CHANGED_BEACON |
> >> +                     BSS_CHANGED_HT |
> >> +                     BSS_CHANGED_BASIC_RATES |
> >> +                     BSS_CHANGED_BEACON_INT)))
> >> +             if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
> >> +                     return;
> >
> > Does that return make any sense?
> 
> The alternative is to keep notifying the driver. I just wanted to stop
> everything since we're out of memory, but we can keep calling
> bss_info_change_notify() is you think that makes more sense.

Either way is fine to me.

> >> @@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
> >>       sdata->vif.bss_conf.enable_beacon = false;
> >>       clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
> >>       ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
> >> +     kfree_rcu(ifmsh->beacon, rcu_head);
> >
> > I think you should set it to NULL first, just so it's clearer.
> 
> For who? It seems there is no need in this path, but OK.

You don't have any synchronize_rcu() here so how can you be sure there's
not someone, say in a tasklet, using ifmsh->beacon at this point?

> >> @@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
> >>       skb_queue_head_init(&ifmsh->ps.bc_buf);
> >>       spin_lock_init(&ifmsh->mesh_preq_queue_lock);
> >>       spin_lock_init(&ifmsh->sync_offset_lock);
> >> +     RCU_INIT_POINTER(ifmsh->beacon, NULL);
> >
> > Isn't everything initialized to 0/null?
> 
> Yep, I wanted any needed RCU magic to happen here though.

I don't think there's any RCU magic, particularly not with
RCU_INIT_POINTER, but I don't mind the assignment much :)

johannes


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] mac80211: generate mesh probe responses
  2013-02-11 21:20   ` Johannes Berg
@ 2013-02-11 22:11     ` Thomas Pedersen
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Pedersen @ 2013-02-11 22:11 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, devel

On Mon, Feb 11, 2013 at 1:20 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:
>
>> +     if (elems.mesh_id_len != 0 &&
>> +         (elems.mesh_id_len != ifmsh->mesh_id_len ||
>> +          memcmp(elems.mesh_id, ifmsh->mesh_id, ifmsh->mesh_id_len))) {
>> +             mpl_dbg(sdata, "ignoring probe request for different MBSS\n");
>
>> +     presp = dev_alloc_skb(local->tx_headroom +
>> +                           bcn->head_len + bcn->tail_len);
>> +     if (WARN_ON(!presp))
>> +             goto out;
>
> same as before.
>
>
>> +     mpl_dbg(sdata, "sending probe resp. to %pM\n", hdr->da);
>
> I also think you should probably drop both mpl_dbg() statements as
> they're likely to just be noise for any other debugging when you
> actually use this?

OK. I'll replace them with sdata_info() then.

-- 
Thomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] mac80211: consolidate MBSS change notification
  2013-02-11 21:07 [PATCH 1/3] mac80211: consolidate MBSS change notification Thomas Pedersen
  2013-02-11 21:07 ` [PATCH 2/3] mac80211: cache mesh beacon Thomas Pedersen
  2013-02-11 21:07 ` [PATCH 3/3] mac80211: generate mesh probe responses Thomas Pedersen
@ 2013-02-12 17:56 ` Marco Porsch
  2013-02-12 18:23   ` Thomas Pedersen
  2 siblings, 1 reply; 10+ messages in thread
From: Marco Porsch @ 2013-02-12 17:56 UTC (permalink / raw)
  To: devel; +Cc: Thomas Pedersen, johannes, linux-wireless

On 02/11/2013 10:07 PM, Thomas Pedersen wrote:
> A few mesh utility functions will call
> ieee80211_bss_info_change_notify(), and then the caller
> might notify the driver of the same change again. Avoid
> this redundancy by propagating the BSS changes and
> generally calling bss_info_change_notify() once per
> change.
>
> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
> ---
>   net/mac80211/cfg.c        |   19 +++++++++++--------
>   net/mac80211/mesh.c       |    2 +-
>   net/mac80211/mesh.h       |   10 +++++-----
>   net/mac80211/mesh_plink.c |   33 ++++++++++++++++++---------------
>   net/mac80211/mesh_ps.c    |   24 ++++++++++++++++++------
>   5 files changed, 53 insertions(+), 35 deletions(-)

[...]

> diff --git a/net/mac80211/mesh_ps.c b/net/mac80211/mesh_ps.c
> index b677962..aa83eac 100644
> --- a/net/mac80211/mesh_ps.c
> +++ b/net/mac80211/mesh_ps.c
> @@ -74,14 +74,17 @@ static void mps_qos_null_tx(struct sta_info *sta)
>    * @sdata: local mesh subif
>    *
>    * sets the non-peer power mode and triggers the driver PS (re-)configuration
> + * Return BSS_CHANGED_BEACON if a beacon update is necessary.
>    */
> -void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
> +u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
>   {
>   	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>   	struct sta_info *sta;
>   	bool peering = false;
>   	int light_sleep_cnt = 0;
>   	int deep_sleep_cnt = 0;
> +	u32 changed = 0;
> +	enum nl80211_mesh_power_mode nonpeer_pm;
>
>   	rcu_read_lock();
>   	list_for_each_entry_rcu(sta, &sdata->local->sta_list, list) {
> @@ -115,17 +118,25 @@ void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
>   	 */
>   	if (peering) {
>   		mps_dbg(sdata, "setting non-peer PM to active for peering\n");
> -		ifmsh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
> +		nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
>   	} else if (light_sleep_cnt || deep_sleep_cnt) {
>   		mps_dbg(sdata, "setting non-peer PM to deep sleep\n");
> -		ifmsh->nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
> +		nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
>   	} else {
>   		mps_dbg(sdata, "setting non-peer PM to user value\n");
> -		ifmsh->nonpeer_pm = ifmsh->mshcfg.power_mode;
> +		nonpeer_pm = ifmsh->mshcfg.power_mode;
>   	}
>
> +	if (ifmsh->nonpeer_pm != nonpeer_pm ||
> +	    ifmsh->ps_peers_light_sleep != light_sleep_cnt ||
> +	    ifmsh->ps_peers_deep_sleep != deep_sleep_cnt)
> +		changed = BSS_CHANGED_BEACON;

Here it only affects the beacon if the number of light/deep sleep peers 
changed from/to zero or non-zero.
The following should avoid some unnecessary updates:

!ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt ||
!ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt)

> +
> +	ifmsh->nonpeer_pm = nonpeer_pm;
>   	ifmsh->ps_peers_light_sleep = light_sleep_cnt;
>   	ifmsh->ps_peers_deep_sleep = deep_sleep_cnt;
> +
> +	return changed;
>   }
>
>   /**


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] mac80211: consolidate MBSS change notification
  2013-02-12 17:56 ` [PATCH 1/3] mac80211: consolidate MBSS change notification Marco Porsch
@ 2013-02-12 18:23   ` Thomas Pedersen
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Pedersen @ 2013-02-12 18:23 UTC (permalink / raw)
  To: Marco Porsch; +Cc: devel, johannes, linux-wireless

On Tue, Feb 12, 2013 at 9:56 AM, Marco Porsch <marco@cozybit.com> wrote:
> On 02/11/2013 10:07 PM, Thomas Pedersen wrote:
>> +       if (ifmsh->nonpeer_pm != nonpeer_pm ||
>> +           ifmsh->ps_peers_light_sleep != light_sleep_cnt ||
>> +           ifmsh->ps_peers_deep_sleep != deep_sleep_cnt)
>> +               changed = BSS_CHANGED_BEACON;
>
>
> Here it only affects the beacon if the number of light/deep sleep peers
> changed from/to zero or non-zero.
> The following should avoid some unnecessary updates:
>
> !ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt ||
> !ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt)

Ah nice. I'll add this to the v2, not checking deep_sleep_cnt twice of course :)

--
Thomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-02-12 18:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-11 21:07 [PATCH 1/3] mac80211: consolidate MBSS change notification Thomas Pedersen
2013-02-11 21:07 ` [PATCH 2/3] mac80211: cache mesh beacon Thomas Pedersen
2013-02-11 21:17   ` Johannes Berg
2013-02-11 21:24     ` Thomas Pedersen
2013-02-11 21:34       ` Johannes Berg
2013-02-11 21:07 ` [PATCH 3/3] mac80211: generate mesh probe responses Thomas Pedersen
2013-02-11 21:20   ` Johannes Berg
2013-02-11 22:11     ` Thomas Pedersen
2013-02-12 17:56 ` [PATCH 1/3] mac80211: consolidate MBSS change notification Marco Porsch
2013-02-12 18:23   ` Thomas Pedersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.