[PATCH 0/4] mac80211: bss_info_changed cleanups

[PATCH 0/4] mac80211: bss_info_changed cleanups

[Johannes Berg]

The recalculations going on in the function are rather odd
and hard to follow, replace them with explicit interface
type dependent handling where needed at all, overall this
makes the code simpler and little bit smaller:

   text	   data	    bss	    dec	    hex	filename
 610582	  38811	    720	 650113	  9eb81	net/mac80211/mac80211.ko (before)
 610415	  38812	    720	 649947	  9eadb	net/mac80211/mac80211.ko (after)

johannes

[PATCH 1/4] mac80211: track enable_beacon explicitly

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Instead of calculating in ieee80211_bss_info_change_notify()
whether beaconing should be enabled or not, set it in the
correct places in the callers. This simplifies the logic in
this function at the expense of offchannel, but is also more
robust.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c         |  4 ++++
 net/mac80211/ibss.c        |  3 +++
 net/mac80211/ieee80211_i.h |  3 +++
 net/mac80211/main.c        | 32 --------------------------------
 net/mac80211/mesh.c        |  3 +++
 net/mac80211/offchannel.c  | 15 +++++++++------
 6 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index af2804c..8576723 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -933,6 +933,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 
 	sdata->vif.bss_conf.beacon_int = params->beacon_interval;
 	sdata->vif.bss_conf.dtim_period = params->dtim_period;
+	sdata->vif.bss_conf.enable_beacon = true;
 
 	sdata->vif.bss_conf.ssid_len = params->ssid_len;
 	if (params->ssid_len)
@@ -1015,6 +1016,9 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
 	list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list)
 		sta_info_flush(vlan);
 	sta_info_flush(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);
 
 	drv_stop_ap(sdata->local, sdata);
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 4521459..acdab28 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -191,6 +191,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 
 	rcu_assign_pointer(ifibss->presp, skb);
 
+	sdata->vif.bss_conf.enable_beacon = true;
 	sdata->vif.bss_conf.beacon_int = beacon_int;
 	sdata->vif.bss_conf.basic_rates = basic_rates;
 	bss_change = BSS_CHANGED_BEACON_INT;
@@ -1189,6 +1190,8 @@ int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata)
 	RCU_INIT_POINTER(sdata->u.ibss.presp, NULL);
 	sdata->vif.bss_conf.ibss_joined = false;
 	sdata->vif.bss_conf.ibss_creator = false;
+	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 |
 						BSS_CHANGED_IBSS);
 	synchronize_rcu();
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 2bb0c98..d4114e9 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -661,10 +661,13 @@ enum ieee80211_sub_if_data_flags {
  *	change handling while the interface is up
  * @SDATA_STATE_OFFCHANNEL: This interface is currently in offchannel
  *	mode, so queues are stopped
+ * @SDATA_STATE_OFFCHANNEL_BEACON_STOPPED: Beaconing was stopped due
+ *	to offchannel, reset when offchannel returns
  */
 enum ieee80211_sdata_state_bits {
 	SDATA_STATE_RUNNING,
 	SDATA_STATE_OFFCHANNEL,
+	SDATA_STATE_OFFCHANNEL_BEACON_STOPPED,
 };
 
 /**
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index e35d13e..e991d30 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -245,38 +245,6 @@ void ieee80211_bss_info_change_notify(struct ieee80211_sub_if_data *sdata,
 		break;
 	}
 
-	if (changed & BSS_CHANGED_BEACON_ENABLED) {
-		if (local->quiescing || !ieee80211_sdata_running(sdata) ||
-		    test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state)) {
-			sdata->vif.bss_conf.enable_beacon = false;
-		} else {
-			/*
-			 * Beacon should be enabled, but AP mode must
-			 * check whether there is a beacon configured.
-			 */
-			switch (sdata->vif.type) {
-			case NL80211_IFTYPE_AP:
-				sdata->vif.bss_conf.enable_beacon =
-					!!sdata->u.ap.beacon;
-				break;
-			case NL80211_IFTYPE_ADHOC:
-				sdata->vif.bss_conf.enable_beacon =
-					!!sdata->u.ibss.presp;
-				break;
-#ifdef CONFIG_MAC80211_MESH
-			case NL80211_IFTYPE_MESH_POINT:
-				sdata->vif.bss_conf.enable_beacon =
-					!!sdata->u.mesh.mesh_id_len;
-				break;
-#endif
-			default:
-				/* not reached */
-				WARN_ON(1);
-				break;
-			}
-		}
-	}
-
 	drv_bss_info_changed(local, sdata, &sdata->vif.bss_conf, changed);
 }
 
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index f5f01e0..df2e36b 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -631,6 +631,7 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 	sdata->vif.bss_conf.ht_operation_mode =
 				ifmsh->mshcfg.ht_opmode;
 	sdata->vif.bss_conf.beacon_int = MESH_DEFAULT_BEACON_INTERVAL;
+	sdata->vif.bss_conf.enable_beacon = true;
 	sdata->vif.bss_conf.basic_rates =
 		ieee80211_mandatory_rates(local, band);
 
@@ -653,6 +654,8 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
 
 	/* stop the beacon */
 	ifmsh->mesh_id_len = 0;
+	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);
 
 	/* flush STAs and mpaths on this iface */
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 5abddfe..3d18769 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -126,11 +126,13 @@ void ieee80211_offchannel_stop_vifs(struct ieee80211_local *local,
 			set_bit(SDATA_STATE_OFFCHANNEL, &sdata->state);
 
 		/* Check to see if we should disable beaconing. */
-		if (sdata->vif.type == NL80211_IFTYPE_AP ||
-		    sdata->vif.type == NL80211_IFTYPE_ADHOC ||
-		    sdata->vif.type == NL80211_IFTYPE_MESH_POINT)
+		if (sdata->vif.bss_conf.enable_beacon) {
+			set_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED,
+				&sdata->state);
+			sdata->vif.bss_conf.enable_beacon = false;
 			ieee80211_bss_info_change_notify(
 				sdata, BSS_CHANGED_BEACON_ENABLED);
+		}
 
 		if (sdata->vif.type != NL80211_IFTYPE_MONITOR) {
 			netif_tx_stop_all_queues(sdata->dev);
@@ -183,11 +185,12 @@ void ieee80211_offchannel_return(struct ieee80211_local *local,
 			netif_tx_wake_all_queues(sdata->dev);
 		}
 
-		if (sdata->vif.type == NL80211_IFTYPE_AP ||
-		    sdata->vif.type == NL80211_IFTYPE_ADHOC ||
-		    sdata->vif.type == NL80211_IFTYPE_MESH_POINT)
+		if (test_and_clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED,
+				       &sdata->state)) {
+			sdata->vif.bss_conf.enable_beacon = true;
 			ieee80211_bss_info_change_notify(
 				sdata, BSS_CHANGED_BEACON_ENABLED);
+		}
 	}
 	mutex_unlock(&local->iflist_mtx);
 }
-- 
1.8.0

[PATCH 2/4] mac80211: reconfig bss_info_changed only if beaconing

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

For AP/IBSS/mesh interfaces, call the driver to reconfigure
bss_info_changed only if the interface was beaconing before
suspend, otherwise we call the driver and it might interpret
the change as going from enabled to disabled.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/util.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index c170b9d..9954927 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1562,9 +1562,11 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 
 			/* fall through */
 		case NL80211_IFTYPE_MESH_POINT:
-			changed |= BSS_CHANGED_BEACON |
-				   BSS_CHANGED_BEACON_ENABLED;
-			ieee80211_bss_info_change_notify(sdata, changed);
+			if (sdata->vif.bss_conf.enable_beacon) {
+				changed |= BSS_CHANGED_BEACON |
+					   BSS_CHANGED_BEACON_ENABLED;
+				ieee80211_bss_info_change_notify(sdata, changed);
+			}
 			break;
 		case NL80211_IFTYPE_WDS:
 			break;
-- 
1.8.0

[PATCH 3/4] mac80211: further simplify ieee80211_bss_info_change_notify

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The special case in the function isn't really needed,
instead make the suspend code a bit better and also
easier to understand and move the warning into the
driver op wrapper inline.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/driver-ops.h |  6 ++++++
 net/mac80211/main.c       | 14 --------------
 net/mac80211/pm.c         | 13 ++++++++++---
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 70dcc42..7be48a9 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -207,6 +207,12 @@ static inline void drv_bss_info_changed(struct ieee80211_local *local,
 {
 	might_sleep();
 
+	WARN_ON_ONCE(changed & (BSS_CHANGED_BEACON |
+				BSS_CHANGED_BEACON_ENABLED) &&
+		     sdata->vif.type != NL80211_IFTYPE_AP &&
+		     sdata->vif.type != NL80211_IFTYPE_ADHOC &&
+		     sdata->vif.type != NL80211_IFTYPE_MESH_POINT);
+
 	check_sdata_in_driver(sdata);
 
 	trace_drv_bss_info_changed(local, sdata, info, changed);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index e991d30..282f583 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -231,20 +231,6 @@ void ieee80211_bss_info_change_notify(struct ieee80211_sub_if_data *sdata,
 		return;
 	}
 
-	switch (sdata->vif.type) {
-	case NL80211_IFTYPE_AP:
-	case NL80211_IFTYPE_ADHOC:
-	case NL80211_IFTYPE_WDS:
-	case NL80211_IFTYPE_MESH_POINT:
-		break;
-	default:
-		/* do not warn to simplify caller in scan.c */
-		changed &= ~BSS_CHANGED_BEACON_ENABLED;
-		if (WARN_ON(changed & BSS_CHANGED_BEACON))
-			return;
-		break;
-	}
-
 	drv_bss_info_changed(local, sdata, &sdata->vif.bss_conf, changed);
 }
 
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index f3a9e5f..3d93cd3 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -121,7 +121,7 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 
 	/* remove all interfaces */
 	list_for_each_entry(sdata, &local->interfaces, list) {
-		u32 changed = BSS_CHANGED_BEACON_ENABLED;
+		u32 changed = 0;
 
 		if (!ieee80211_sdata_running(sdata))
 			continue;
@@ -133,12 +133,19 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 			continue;
 		case NL80211_IFTYPE_STATION:
 			changed = BSS_CHANGED_ASSOC;
-			/* fall through */
+			break;
+		case NL80211_IFTYPE_AP:
+		case NL80211_IFTYPE_ADHOC:
+		case NL80211_IFTYPE_MESH_POINT:
+			if (sdata->vif.bss_conf.enable_beacon)
+				changed = BSS_CHANGED_BEACON_ENABLED;
+			break;
 		default:
-			ieee80211_quiesce(sdata);
 			break;
 		}
 
+		ieee80211_quiesce(sdata);
+
 		sdata->suspend_bss_conf = sdata->vif.bss_conf;
 		memset(&sdata->vif.bss_conf, 0, sizeof(sdata->vif.bss_conf));
 
-- 
1.8.0

[PATCH 4/4] mac80211: assign bss_conf.bssid only once

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Instead of checking every time bss_info_changed is called,
assign the pointer once depending on the interface type
and then leave it untouched until the interface type is
changed. This makes the ieee80211_bss_info_change_notify()
now a simple wrapper to call the driver only.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/driver-ops.h |  2 ++
 net/mac80211/iface.c      |  7 +++++++
 net/mac80211/main.c       | 20 --------------------
 net/mac80211/mesh.c       |  3 +++
 net/mac80211/pm.c         |  3 +++
 5 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 7be48a9..a93eba0 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -212,6 +212,8 @@ static inline void drv_bss_info_changed(struct ieee80211_local *local,
 		     sdata->vif.type != NL80211_IFTYPE_AP &&
 		     sdata->vif.type != NL80211_IFTYPE_ADHOC &&
 		     sdata->vif.type != NL80211_IFTYPE_MESH_POINT);
+	WARN_ON_ONCE(sdata->vif.type == NL80211_IFTYPE_P2P_DEVICE &&
+		     changed & ~BSS_CHANGED_IDLE);
 
 	check_sdata_in_driver(sdata);
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index ab47df0..dcf943e 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1233,6 +1233,7 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
 	case NL80211_IFTYPE_AP:
 		skb_queue_head_init(&sdata->u.ap.ps.bc_buf);
 		INIT_LIST_HEAD(&sdata->u.ap.vlans);
+		sdata->vif.bss_conf.bssid = sdata->vif.addr;
 		break;
 	case NL80211_IFTYPE_P2P_CLIENT:
 		type = NL80211_IFTYPE_STATION;
@@ -1240,9 +1241,11 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
 		sdata->vif.p2p = true;
 		/* fall through */
 	case NL80211_IFTYPE_STATION:
+		sdata->vif.bss_conf.bssid = sdata->u.mgd.bssid;
 		ieee80211_sta_setup_sdata(sdata);
 		break;
 	case NL80211_IFTYPE_ADHOC:
+		sdata->vif.bss_conf.bssid = sdata->u.ibss.bssid;
 		ieee80211_ibss_setup_sdata(sdata);
 		break;
 	case NL80211_IFTYPE_MESH_POINT:
@@ -1256,8 +1259,12 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
 				      MONITOR_FLAG_OTHER_BSS;
 		break;
 	case NL80211_IFTYPE_WDS:
+		sdata->vif.bss_conf.bssid = NULL;
+		break;
 	case NL80211_IFTYPE_AP_VLAN:
+		break;
 	case NL80211_IFTYPE_P2P_DEVICE:
+		sdata->vif.bss_conf.bssid = sdata->vif.addr;
 		break;
 	case NL80211_IFTYPE_UNSPECIFIED:
 	case NUM_NL80211_IFTYPES:
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 282f583..475e85b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -207,30 +207,10 @@ void ieee80211_bss_info_change_notify(struct ieee80211_sub_if_data *sdata,
 				      u32 changed)
 {
 	struct ieee80211_local *local = sdata->local;
-	static const u8 zero[ETH_ALEN] = { 0 };
 
 	if (!changed)
 		return;
 
-	if (sdata->vif.type == NL80211_IFTYPE_STATION) {
-		sdata->vif.bss_conf.bssid = sdata->u.mgd.bssid;
-	} else if (sdata->vif.type == NL80211_IFTYPE_ADHOC)
-		sdata->vif.bss_conf.bssid = sdata->u.ibss.bssid;
-	else if (sdata->vif.type == NL80211_IFTYPE_AP)
-		sdata->vif.bss_conf.bssid = sdata->vif.addr;
-	else if (sdata->vif.type == NL80211_IFTYPE_WDS)
-		sdata->vif.bss_conf.bssid = NULL;
-	else if (ieee80211_vif_is_mesh(&sdata->vif)) {
-		sdata->vif.bss_conf.bssid = zero;
-	} else if (sdata->vif.type == NL80211_IFTYPE_P2P_DEVICE) {
-		sdata->vif.bss_conf.bssid = sdata->vif.addr;
-		WARN_ONCE(changed & ~(BSS_CHANGED_IDLE),
-			  "P2P Device BSS changed %#x", changed);
-	} else {
-		WARN_ON(1);
-		return;
-	}
-
 	drv_bss_info_changed(local, sdata, &sdata->vif.bss_conf, changed);
 }
 
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index df2e36b..776536a 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -815,6 +815,7 @@ void ieee80211_mesh_notify_scan_completed(struct ieee80211_local *local)
 void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+	static u8 zero_addr[ETH_ALEN] = {};
 
 	setup_timer(&ifmsh->housekeeping_timer,
 		    ieee80211_mesh_housekeeping_timer,
@@ -840,4 +841,6 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
 	INIT_LIST_HEAD(&ifmsh->preq_queue.list);
 	spin_lock_init(&ifmsh->mesh_preq_queue_lock);
 	spin_lock_init(&ifmsh->sync_offset_lock);
+
+	sdata->vif.bss_conf.bssid = zero_addr;
 }
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index 3d93cd3..f725622 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -121,6 +121,7 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 
 	/* remove all interfaces */
 	list_for_each_entry(sdata, &local->interfaces, list) {
+		static u8 zero_addr[ETH_ALEN] = {};
 		u32 changed = 0;
 
 		if (!ieee80211_sdata_running(sdata))
@@ -148,6 +149,8 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 
 		sdata->suspend_bss_conf = sdata->vif.bss_conf;
 		memset(&sdata->vif.bss_conf, 0, sizeof(sdata->vif.bss_conf));
+		if (sdata->suspend_bss_conf.bssid)
+			sdata->vif.bss_conf.bssid = zero_addr;
 
 		/* disable beaconing or remove association */
 		ieee80211_bss_info_change_notify(sdata, changed);
-- 
1.8.0

Re: [PATCH 0/4] mac80211: bss_info_changed cleanups

[Johannes Berg]

On Fri, 2012-12-14 at 14:46 +0100, Johannes Berg wrote:
> The recalculations going on in the function are rather odd
> and hard to follow, replace them with explicit interface
> type dependent handling where needed at all

applied all

johannes