[RFC 0/9] mac80211: multi-interface CSA

[RFC 0/9] mac80211: multi-interface CSA

[Michal Kazior]

Hi,

This patchet bases of my recently posted cfg80211
multi-interface CSA patchset.

Probably some patches in this patchset can be
cherry-picked regardless if the multi-interface
CSA itself is accepted or not.

I'm planning on posting properly split patchsets
after getting some feedback on the mac80211 part.

For anyone interested in getting the whole thing
easily (cfg80211 is not yet fixed there as per
Johaness' comments):

	https://github.com/kazikcz/linux/tree/multi-interface-csa-review-2014-01-15-1
	https://github.com/kazikcz/hostap/tree/multi-interface-csa-review-2014-01-15-1

I've tested it a bit and it seems to work with
ath9k and ath10k (with some local patches).


Michal Kazior (9):
  mac80211: fix possible memory leak on AP CSA failure
  mac80211: treat IBSS CSA finish failure seriously
  mac80211: move csa_active setting in STA CSA
  mac80211: fix sdata->radar_required locking
  mac80211: improve CSA locking
  mac80211: track CSA globally
  Revert "cfg80211: disable CSA for all drivers"
  ath9k: prepare for multi-interface CSA support
  mac80211: implement multi-interface CSA

 drivers/net/wireless/ath/ath9k/ath9k.h  |   4 +-
 drivers/net/wireless/ath/ath9k/beacon.c |  29 ++-
 drivers/net/wireless/ath/ath9k/main.c   |  12 +-
 drivers/net/wireless/ath/ath9k/xmit.c   |   2 +-
 net/mac80211/cfg.c                      | 419 ++++++++++++++++++++++++++------
 net/mac80211/chan.c                     | 117 ++++++---
 net/mac80211/ibss.c                     |  42 +++-
 net/mac80211/ieee80211_i.h              |  24 +-
 net/mac80211/iface.c                    |  39 ++-
 net/mac80211/mesh.c                     |  16 +-
 net/mac80211/mlme.c                     |  37 ++-
 net/mac80211/scan.c                     |   5 +
 net/mac80211/tx.c                       |  10 +-
 net/mac80211/util.c                     |  18 ++
 net/wireless/core.c                     |   6 -
 15 files changed, 606 insertions(+), 174 deletions(-)

-- 
1.8.4.rc3

[RFC 1/9] mac80211: fix possible memory leak on AP CSA failure

[Michal Kazior]

If CSA for AP interface failed and the interface
was not stopped afterwards another CSA request
would leak sdata->u.ap.next_beacon.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 543b18f..8c78572 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2987,6 +2987,21 @@ cfg80211_beacon_dup(struct cfg80211_beacon_data *beacon)
 	return new_beacon;
 }
 
+static int ieee80211_ap_finish_csa(struct ieee80211_sub_if_data *sdata)
+{
+	int err;
+
+	err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon);
+	kfree(sdata->u.ap.next_beacon);
+	sdata->u.ap.next_beacon = NULL;
+
+	if (err < 0)
+		return err;
+
+	ieee80211_bss_info_change_notify(sdata, err);
+	return 0;
+}
+
 void ieee80211_csa_finalize_work(struct work_struct *work)
 {
 	struct ieee80211_sub_if_data *sdata =
@@ -3020,15 +3035,9 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
 	sdata->vif.csa_active = false;
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_AP:
-		err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon);
+		err = ieee80211_ap_finish_csa(sdata);
 		if (err < 0)
 			goto unlock;
-
-		changed |= err;
-		kfree(sdata->u.ap.next_beacon);
-		sdata->u.ap.next_beacon = NULL;
-
-		ieee80211_bss_info_change_notify(sdata, err);
 		break;
 	case NL80211_IFTYPE_ADHOC:
 		ieee80211_ibss_finish_csa(sdata);
-- 
1.8.4.rc3

[RFC 2/9] mac80211: treat IBSS CSA finish failure seriously

[Michal Kazior]

Other interface modes are checked against failure.
This should avoid false-positive channel switch
events where IBSS CSA actually failed.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 8c78572..2ab5f49 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3040,7 +3040,9 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
 			goto unlock;
 		break;
 	case NL80211_IFTYPE_ADHOC:
-		ieee80211_ibss_finish_csa(sdata);
+		err = ieee80211_ibss_finish_csa(sdata);
+		if (err < 0)
+			goto unlock;
 		break;
 #ifdef CONFIG_MAC80211_MESH
 	case NL80211_IFTYPE_MESH_POINT:
-- 
1.8.4.rc3

Re: [RFC 2/9] mac80211: treat IBSS CSA finish failure seriously

[Luca Coelho]

Hi Michal,

On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
> Other interface modes are checked against failure.
> This should avoid false-positive channel switch
> events where IBSS CSA actually failed.
> 
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  net/mac80211/cfg.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 8c78572..2ab5f49 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -3040,7 +3040,9 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
>  			goto unlock;
>  		break;
>  	case NL80211_IFTYPE_ADHOC:
> -		ieee80211_ibss_finish_csa(sdata);
> +		err = ieee80211_ibss_finish_csa(sdata);
> +		if (err < 0)
> +			goto unlock;
>  		break;
>  #ifdef CONFIG_MAC80211_MESH
>  	case NL80211_IFTYPE_MESH_POINT:

This makes sense, but there have been lots of changes in this code that
Johannes just applied on mac80211-next.git.  Maybe it would be better to
rebase your patches on top of mac80211-next or wait until they get into
wireless-testing?

--
Luca.

[RFC 3/9] mac80211: move csa_active setting in STA CSA

[Michal Kazior]

The sdata->vif.csa_active could be left set after,
e.g. channel context contraints check fail in STA
mode leaving the interface in a strange state for
a brief period of time until it is disconnected.
This was harmless but ugly.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index fc1d824..bfb81cb 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1001,7 +1001,6 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	}
 
 	ifmgd->flags |= IEEE80211_STA_CSA_RECEIVED;
-	sdata->vif.csa_active = true;
 
 	mutex_lock(&local->chanctx_mtx);
 	if (local->use_chanctx) {
@@ -1039,6 +1038,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	mutex_unlock(&local->chanctx_mtx);
 
 	sdata->csa_chandef = csa_ie.chandef;
+	sdata->vif.csa_active = true;
 
 	if (csa_ie.mode)
 		ieee80211_stop_queues_by_reason(&local->hw,
-- 
1.8.4.rc3

Re: [RFC 3/9] mac80211: move csa_active setting in STA CSA

[Johannes Berg]

On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
> The sdata->vif.csa_active could be left set after,
> e.g. channel context contraints check fail in STA

typo: constraints

[RFC 4/9] mac80211: fix sdata->radar_required locking

[Michal Kazior]

radar_required setting wasn't protected by
local->mtx in some places. This should prevent
from scanning/radar detection/roc colliding.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c  | 4 ++--
 net/mac80211/ibss.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 2ab5f49..ef679de 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -970,9 +970,9 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	/* TODO: make hostapd tell us what it wants */
 	sdata->smps_mode = IEEE80211_SMPS_OFF;
 	sdata->needed_rx_chains = sdata->local->rx_chains;
-	sdata->radar_required = params->radar_required;
 
 	mutex_lock(&local->mtx);
+	sdata->radar_required = params->radar_required;
 	err = ieee80211_vif_use_channel(sdata, &params->chandef,
 					IEEE80211_CHANCTX_SHARED);
 	mutex_unlock(&local->mtx);
@@ -3018,8 +3018,8 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
 	if (!ieee80211_sdata_running(sdata))
 		goto unlock;
 
-	sdata->radar_required = sdata->csa_radar_required;
 	mutex_lock(&local->mtx);
+	sdata->radar_required = sdata->csa_radar_required;
 	err = ieee80211_vif_change_channel(sdata, &changed);
 	mutex_unlock(&local->mtx);
 	if (WARN_ON(err < 0))
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 5b9c95a..d1dc641 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -303,6 +303,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 		mutex_unlock(&local->mtx);
 		return;
 	}
+	sdata->radar_required = radar_required;
 	mutex_unlock(&local->mtx);
 
 	memcpy(ifibss->bssid, bssid, ETH_ALEN);
@@ -318,7 +319,6 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	rcu_assign_pointer(ifibss->presp, presp);
 	mgmt = (void *)presp->head;
 
-	sdata->radar_required = radar_required;
 	sdata->vif.bss_conf.enable_beacon = true;
 	sdata->vif.bss_conf.beacon_int = beacon_int;
 	sdata->vif.bss_conf.basic_rates = basic_rates;
-- 
1.8.4.rc3

[RFC 5/9] mac80211: improve CSA locking

[Michal Kazior]

The patch improves channel switch related locking
(STA, IBSS, AP, mesh).

Now read access to sdata->vif.csa_active is
protected by wdev.mtx and local->mtx so holding
either is enough for read access but both are
required for write access.

The only exception is ieee80211_beacon_get_tim()
but it's safe to leave it as is and it doesn't
influence mac80211 state in any way.

The patch adds a few lockdep assertions along for
easier code/locking maintenance.

This also prepares for multi-interface CSA.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c   | 18 +++++++++++++++---
 net/mac80211/ibss.c  | 18 ++++++++++++++----
 net/mac80211/iface.c | 27 +++++++++++++++++++++++++--
 net/mac80211/mesh.c  | 13 ++++++++++++-
 net/mac80211/mlme.c  | 20 ++++++++++++++------
 5 files changed, 80 insertions(+), 16 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ef679de..9a2421d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1052,6 +1052,7 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 	int err;
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	sdata_assert_lock(sdata);
 
 	/* don't allow changing the beacon while CSA is in place - offset
 	 * of channel switch counter may change
@@ -1079,15 +1080,19 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
 	struct probe_resp *old_probe_resp;
 	struct cfg80211_chan_def chandef;
 
+	sdata_assert_lock(sdata);
+
 	old_beacon = sdata_dereference(sdata->u.ap.beacon, sdata);
 	if (!old_beacon)
 		return -ENOENT;
 	old_probe_resp = sdata_dereference(sdata->u.ap.probe_resp, sdata);
 
 	/* abort any running channel switch */
+	mutex_lock(&local->mtx);
 	sdata->vif.csa_active = false;
 	kfree(sdata->u.ap.next_beacon);
 	sdata->u.ap.next_beacon = NULL;
+	mutex_unlock(&local->mtx);
 
 	cancel_work_sync(&sdata->u.ap.request_smps_work);
 
@@ -2991,6 +2996,8 @@ static int ieee80211_ap_finish_csa(struct ieee80211_sub_if_data *sdata)
 {
 	int err;
 
+	lockdep_assert_held(&sdata->local->mtx);
+
 	err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon);
 	kfree(sdata->u.ap.next_beacon);
 	sdata->u.ap.next_beacon = NULL;
@@ -3011,6 +3018,7 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
 	int err, changed = 0;
 
 	sdata_lock(sdata);
+	mutex_lock(&local->mtx);
 	/* AP might have been stopped while waiting for the lock. */
 	if (!sdata->vif.csa_active)
 		goto unlock;
@@ -3018,10 +3026,9 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
 	if (!ieee80211_sdata_running(sdata))
 		goto unlock;
 
-	mutex_lock(&local->mtx);
 	sdata->radar_required = sdata->csa_radar_required;
+
 	err = ieee80211_vif_change_channel(sdata, &changed);
-	mutex_unlock(&local->mtx);
 	if (WARN_ON(err < 0))
 		goto unlock;
 
@@ -3063,6 +3070,7 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
 	cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
 
 unlock:
+	mutex_unlock(&local->mtx);
 	sdata_unlock(sdata);
 }
 
@@ -3076,7 +3084,8 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	struct ieee80211_if_mesh __maybe_unused *ifmsh;
 	int err, num_chanctx;
 
-	lockdep_assert_held(&sdata->wdev.mtx);
+	sdata_assert_lock(sdata);
+	lockdep_assert_held(&local->mtx);
 
 	if (!list_empty(&local->roc_list) || local->scanning)
 		return -EBUSY;
@@ -3219,8 +3228,11 @@ int ieee80211_channel_switch(struct wiphy *wiphy,
 		return -EOPNOTSUPP;
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(params[0].dev);
+
 	sdata_lock(sdata);
+	mutex_lock(&sdata->local->mtx);
 	err = __ieee80211_channel_switch(wiphy, params[0].dev, &params[0]);
+	mutex_unlock(&sdata->local->mtx);
 	sdata_unlock(sdata);
 
 	return err;
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index d1dc641..706b666 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -802,6 +802,12 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	int err;
 	u32 sta_flags;
 
+	sdata_assert_lock(sdata);
+	lockdep_assert_held(&sdata->local->mtx);
+
+	if (sdata->vif.csa_active)
+		return true;
+
 	sta_flags = IEEE80211_STA_DISABLE_VHT;
 	switch (ifibss->chandef.width) {
 	case NL80211_CHAN_WIDTH_5:
@@ -943,8 +949,9 @@ ieee80211_rx_mgmt_spectrum_mgmt(struct ieee80211_sub_if_data *sdata,
 	if (len < required_len)
 		return;
 
-	if (!sdata->vif.csa_active)
-		ieee80211_ibss_process_chanswitch(sdata, elems, false);
+	mutex_lock(&sdata->local->mtx);
+	ieee80211_ibss_process_chanswitch(sdata, elems, false);
+	mutex_unlock(&sdata->local->mtx);
 }
 
 static void ieee80211_rx_mgmt_deauth_ibss(struct ieee80211_sub_if_data *sdata,
@@ -1125,9 +1132,12 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 		goto put_bss;
 
 	/* process channel switch */
-	if (sdata->vif.csa_active ||
-	    ieee80211_ibss_process_chanswitch(sdata, elems, true))
+	mutex_lock(&local->mtx);
+	if (ieee80211_ibss_process_chanswitch(sdata, elems, true)) {
+		mutex_unlock(&local->mtx);
 		goto put_bss;
+	}
+	mutex_unlock(&local->mtx);
 
 	/* same BSSID */
 	if (ether_addr_equal(cbss->bssid, sdata->u.ibss.bssid))
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 0aa9675..a339334 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -250,14 +250,18 @@ static inline int identical_mac_addr_allowed(int type1, int type2)
 			 type2 == NL80211_IFTYPE_AP_VLAN));
 }
 
-static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
-					    enum nl80211_iftype iftype)
+static int
+__ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
+				   enum nl80211_iftype iftype)
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_sub_if_data *nsdata;
 
 	ASSERT_RTNL();
 
+	/* access to vif.csa_active should be protected with `local->mtx` */
+	lockdep_assert_held(&local->mtx);
+
 	/* we hold the RTNL here so can safely walk the list */
 	list_for_each_entry(nsdata, &local->interfaces, list) {
 		if (nsdata != sdata && ieee80211_sdata_running(nsdata)) {
@@ -308,6 +312,21 @@ static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
+static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
+					    enum nl80211_iftype iftype)
+{
+	struct ieee80211_local *local = sdata->local;
+	int err;
+
+	ASSERT_RTNL();
+
+	mutex_lock(&local->mtx);
+	err = __ieee80211_check_concurrent_iface(sdata, iftype);
+	mutex_unlock(&local->mtx);
+
+	return err;
+}
+
 static int ieee80211_check_queues(struct ieee80211_sub_if_data *sdata,
 				  enum nl80211_iftype iftype)
 {
@@ -822,7 +841,11 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 	cancel_work_sync(&local->dynamic_ps_enable_work);
 
 	cancel_work_sync(&sdata->recalc_smps);
+	sdata_lock(sdata);
+	mutex_lock(&local->mtx);
 	sdata->vif.csa_active = false;
+	mutex_unlock(&local->mtx);
+	sdata_unlock(sdata);
 	cancel_work_sync(&sdata->csa_finalize_work);
 
 	cancel_delayed_work_sync(&sdata->dfs_cac_timer_work);
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 5a74b24..fa758dc 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -865,6 +865,9 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
 	int err, num_chanctx;
 	u32 sta_flags;
 
+	sdata_assert_lock(sdata);
+	lockdep_assert_held(&sdata->local->mtx);
+
 	if (sdata->vif.csa_active)
 		return true;
 
@@ -967,6 +970,7 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
 				IEEE80211_QUEUE_STOP_REASON_CSA);
 
 	sdata->csa_chandef = params.chandef;
+
 	sdata->vif.csa_active = true;
 
 	ieee80211_bss_info_change_notify(sdata, err);
@@ -1085,8 +1089,10 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
 		ifmsh->sync_ops->rx_bcn_presp(sdata,
 			stype, mgmt, &elems, rx_status);
 
+	mutex_lock(&sdata->local->mtx);
 	if (!ifmsh->chsw_init)
 		ieee80211_mesh_process_chnswitch(sdata, &elems, true);
+	mutex_unlock(&sdata->local->mtx);
 }
 
 int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
@@ -1189,6 +1195,7 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata,
 	bool fwd_csa = true;
 	size_t baselen;
 	u8 *pos;
+	int err;
 
 	if (mgmt->u.action.u.measurement.action_code !=
 	    WLAN_ACTION_SPCT_CHL_SWITCH)
@@ -1209,7 +1216,11 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata,
 
 	ifmsh->pre_value = pre_value;
 
-	if (!ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
+	mutex_lock(&sdata->local->mtx);
+	err = ieee80211_mesh_process_chnswitch(sdata, &elems, false);
+	mutex_unlock(&sdata->local->mtx);
+
+	if (!err) {
 		mcsa_dbg(sdata, "Failed to process CSA action frame");
 		return;
 	}
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index bfb81cb..d898dc9 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -885,12 +885,11 @@ static void ieee80211_chswitch_work(struct work_struct *work)
 		return;
 
 	sdata_lock(sdata);
+	mutex_lock(&local->mtx);
 	if (!ifmgd->associated)
 		goto out;
 
-	mutex_lock(&local->mtx);
 	ret = ieee80211_vif_change_channel(sdata, &changed);
-	mutex_unlock(&local->mtx);
 	if (ret) {
 		sdata_info(sdata,
 			   "vif channel switch failed, disconnecting\n");
@@ -923,6 +922,7 @@ static void ieee80211_chswitch_work(struct work_struct *work)
  out:
 	sdata->vif.csa_active = false;
 	ifmgd->flags &= ~IEEE80211_STA_CSA_RECEIVED;
+	mutex_unlock(&local->mtx);
 	sdata_unlock(sdata);
 }
 
@@ -965,6 +965,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	int res;
 
 	sdata_assert_lock(sdata);
+	lockdep_assert_held(&sdata->local->mtx);
 
 	if (!cbss)
 		return;
@@ -1987,10 +1988,9 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
 	u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
 
 	sdata_lock(sdata);
-	if (!ifmgd->associated) {
-		sdata_unlock(sdata);
-		return;
-	}
+	mutex_lock(&sdata->local->mtx);
+	if (!ifmgd->associated)
+		goto out;
 
 	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH,
 			       WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY,
@@ -2003,6 +2003,8 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
 
 	cfg80211_tx_mlme_mgmt(sdata->dev, frame_buf,
 			      IEEE80211_DEAUTH_FRAME_LEN);
+out:
+	mutex_unlock(&sdata->local->mtx);
 	sdata_unlock(sdata);
 }
 
@@ -2969,8 +2971,10 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 
 	ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, &elems);
 
+	mutex_lock(&local->mtx);
 	ieee80211_sta_process_chanswitch(sdata, rx_status->mactime,
 					 &elems, true);
+	mutex_unlock(&local->mtx);
 
 	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_WMM) &&
 	    ieee80211_sta_wmm_params(local, sdata, elems.wmm_param,
@@ -3101,9 +3105,11 @@ void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
 			if (elems.parse_error)
 				break;
 
+			mutex_lock(&sdata->local->mtx);
 			ieee80211_sta_process_chanswitch(sdata,
 							 rx_status->mactime,
 							 &elems, false);
+			mutex_unlock(&sdata->local->mtx);
 		} else if (mgmt->u.action.category == WLAN_CATEGORY_PUBLIC) {
 			ies_len = skb->len -
 				  offsetof(struct ieee80211_mgmt,
@@ -3123,9 +3129,11 @@ void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
 			elems.ext_chansw_ie =
 				&mgmt->u.action.u.ext_chan_switch.data;
 
+			mutex_lock(&sdata->local->mtx);
 			ieee80211_sta_process_chanswitch(sdata,
 							 rx_status->mactime,
 							 &elems, false);
+			mutex_unlock(&sdata->local->mtx);
 		}
 		break;
 	}
-- 
1.8.4.rc3

[RFC 6/9] mac80211: track CSA globally

[Michal Kazior]

For CSA to be safe it needs to be treated the same
way as radar detection, scanning and remain on
channel - all of those (including CSA) must be
mutually exclusive.

This prepares for multi-interface CSA.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c         | 34 ++++++++++++++++++++++------------
 net/mac80211/ibss.c        | 22 +++++++++++++++-------
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/iface.c       |  7 +++++--
 net/mac80211/mesh.c        |  3 +++
 net/mac80211/mlme.c        |  6 ++++++
 net/mac80211/scan.c        |  5 +++++
 net/mac80211/util.c        | 18 ++++++++++++++++++
 8 files changed, 75 insertions(+), 21 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9a2421d..13d1624 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -973,8 +973,12 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 
 	mutex_lock(&local->mtx);
 	sdata->radar_required = params->radar_required;
-	err = ieee80211_vif_use_channel(sdata, &params->chandef,
-					IEEE80211_CHANCTX_SHARED);
+
+	if (ieee80211_is_csa_active(local))
+		err = -EBUSY;
+	else
+		err = ieee80211_vif_use_channel(sdata, &params->chandef,
+						IEEE80211_CHANCTX_SHARED);
 	mutex_unlock(&local->mtx);
 	if (err)
 		return err;
@@ -1957,8 +1961,11 @@ static int ieee80211_join_mesh(struct wiphy *wiphy, struct net_device *dev,
 	sdata->needed_rx_chains = sdata->local->rx_chains;
 
 	mutex_lock(&sdata->local->mtx);
-	err = ieee80211_vif_use_channel(sdata, &setup->chandef,
-					IEEE80211_CHANCTX_SHARED);
+	if (ieee80211_is_csa_active(sdata->local))
+		err = -EBUSY;
+	else
+		err = ieee80211_vif_use_channel(sdata, &setup->chandef,
+						IEEE80211_CHANCTX_SHARED);
 	mutex_unlock(&sdata->local->mtx);
 	if (err)
 		return err;
@@ -2644,7 +2651,8 @@ static int ieee80211_start_roc_work(struct ieee80211_local *local,
 
 	/* if there's one pending or we're scanning, queue this one */
 	if (!list_empty(&local->roc_list) ||
-	    local->scanning || local->radar_detect_enabled)
+	    local->scanning || local->radar_detect_enabled ||
+	    ieee80211_is_csa_active(local))
 		goto out_check_combine;
 
 	/* if not HW assist, just queue & schedule work */
@@ -2912,7 +2920,8 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy,
 	int err;
 
 	mutex_lock(&local->mtx);
-	if (!list_empty(&local->roc_list) || local->scanning) {
+	if (!list_empty(&local->roc_list) || local->scanning ||
+	    ieee80211_is_csa_active(local)) {
 		err = -EBUSY;
 		goto out_unlock;
 	}
@@ -3087,7 +3096,8 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	sdata_assert_lock(sdata);
 	lockdep_assert_held(&local->mtx);
 
-	if (!list_empty(&local->roc_list) || local->scanning)
+	if (!list_empty(&local->roc_list) || local->scanning ||
+	    ieee80211_is_csa_active(local))
 		return -EBUSY;
 
 	if (sdata->wdev.cac_started)
@@ -3097,23 +3107,23 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 				       &sdata->vif.bss_conf.chandef))
 		return -EINVAL;
 
-	rcu_read_lock();
+	mutex_lock(&local->chanctx_mtx);
 	chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 	if (!chanctx_conf) {
-		rcu_read_unlock();
+		mutex_unlock(&local->chanctx_mtx);
 		return -EBUSY;
 	}
 
 	/* don't handle for multi-VIF cases */
 	chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
 	if (chanctx->refcount > 1) {
-		rcu_read_unlock();
+		mutex_unlock(&local->chanctx_mtx);
 		return -EBUSY;
 	}
 	num_chanctx = 0;
-	list_for_each_entry_rcu(chanctx, &local->chanctx_list, list)
+	list_for_each_entry(chanctx, &local->chanctx_list, list)
 		num_chanctx++;
-	rcu_read_unlock();
+	mutex_unlock(&local->chanctx_mtx);
 
 	if (num_chanctx > 1)
 		return -EBUSY;
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 706b666..12c6019 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -295,16 +295,21 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 
 	mutex_lock(&local->mtx);
 	ieee80211_vif_release_channel(sdata);
-	if (ieee80211_vif_use_channel(sdata, &chandef,
-				      ifibss->fixed_channel ?
-					IEEE80211_CHANCTX_SHARED :
-					IEEE80211_CHANCTX_EXCLUSIVE)) {
+	if (ieee80211_is_csa_active(local))
+		err = -EBUSY;
+	else
+		err = ieee80211_vif_use_channel(sdata, &chandef,
+						ifibss->fixed_channel ?
+						IEEE80211_CHANCTX_SHARED :
+						IEEE80211_CHANCTX_EXCLUSIVE);
+
+	sdata->radar_required = radar_required;
+	mutex_unlock(&local->mtx);
+
+	if (err) {
 		sdata_info(sdata, "Failed to join IBSS, no channel context\n");
-		mutex_unlock(&local->mtx);
 		return;
 	}
-	sdata->radar_required = radar_required;
-	mutex_unlock(&local->mtx);
 
 	memcpy(ifibss->bssid, bssid, ETH_ALEN);
 
@@ -808,6 +813,9 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	if (sdata->vif.csa_active)
 		return true;
 
+	if (ieee80211_is_csa_active(sdata->local))
+		return false;
+
 	sta_flags = IEEE80211_STA_DISABLE_VHT;
 	switch (ifibss->chandef.width) {
 	case NL80211_CHAN_WIDTH_5:
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e3bab2f..e523429 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1462,6 +1462,7 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 int ieee80211_channel_switch(struct wiphy *wiphy,
 			     struct cfg80211_csa_settings *params,
 			     int num_params);
+bool ieee80211_is_csa_active(struct ieee80211_local *local);
 
 /* interface handling */
 int ieee80211_iface_init(void);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index a339334..58cc061 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -438,8 +438,11 @@ int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
 	}
 
 	mutex_lock(&local->mtx);
-	ret = ieee80211_vif_use_channel(sdata, &local->monitor_chandef,
-					IEEE80211_CHANCTX_EXCLUSIVE);
+	if (ieee80211_is_csa_active(local))
+		ret = -EBUSY;
+	else
+		ret = ieee80211_vif_use_channel(sdata, &local->monitor_chandef,
+						IEEE80211_CHANCTX_EXCLUSIVE);
 	mutex_unlock(&local->mtx);
 	if (ret) {
 		drv_remove_interface(local, sdata);
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index fa758dc..8a48e0e 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -871,6 +871,9 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
 	if (sdata->vif.csa_active)
 		return true;
 
+	if (ieee80211_is_csa_active(sdata->local))
+		return false;
+
 	if (!ifmsh->mesh_id)
 		return false;
 
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index d898dc9..b3fa66a 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3691,6 +3691,12 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 	sdata->smps_mode = IEEE80211_SMPS_OFF;
 
 	mutex_lock(&local->mtx);
+
+	if (ieee80211_is_csa_active(local)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
 	/*
 	 * If this fails (possibly due to channel context sharing
 	 * on incompatible channels, e.g. 80+80 and 160 sharing the
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 88c8161..6c0a765 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -389,9 +389,14 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
 static bool ieee80211_can_scan(struct ieee80211_local *local,
 			       struct ieee80211_sub_if_data *sdata)
 {
+	lockdep_assert_held(&local->mtx);
+
 	if (local->radar_detect_enabled)
 		return false;
 
+	if (ieee80211_is_csa_active(local))
+		return false;
+
 	if (!list_empty(&local->roc_list))
 		return false;
 
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index df00f19..c8b3f15 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2734,3 +2734,21 @@ int ieee80211_parse_p2p_noa(const struct ieee80211_p2p_noa_attr *attr,
 	return ret;
 }
 EXPORT_SYMBOL(ieee80211_parse_p2p_noa);
+
+bool ieee80211_is_csa_active(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	lockdep_assert_held(&local->mtx);
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		if (sdata->vif.csa_active) {
+			rcu_read_unlock();
+			return true;
+		}
+	}
+	rcu_read_unlock();
+
+	return false;
+}
-- 
1.8.4.rc3

Re: [RFC 6/9] mac80211: track CSA globally

[Johannes Berg]

On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
> For CSA to be safe it needs to be treated the same
> way as radar detection, scanning and remain on
> channel - all of those (including CSA) must be
> mutually exclusive.

This I don't understand. Why couldn't you do a remain-on(some
other)-channel or scan while counting down the beacons?

johannes

Re: [RFC 6/9] mac80211: track CSA globally

[Michal Kazior]

On 15 January 2014 14:19, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
>> For CSA to be safe it needs to be treated the same
>> way as radar detection, scanning and remain on
>> channel - all of those (including CSA) must be
>> mutually exclusive.
>
> This I don't understand. Why couldn't you do a remain-on(some
> other)-channel or scan while counting down the beacons?

My concern is software offchannel (be it scan or roc) involves channel
context switches. I wanted to avoid any channel context mangling while
CSA is in progress. Does that make sense to you?


Michał

Re: [RFC 6/9] mac80211: track CSA globally

[Johannes Berg]

On Wed, 2014-01-15 at 14:28 +0100, Michal Kazior wrote:
> On 15 January 2014 14:19, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
> >> For CSA to be safe it needs to be treated the same
> >> way as radar detection, scanning and remain on
> >> channel - all of those (including CSA) must be
> >> mutually exclusive.
> >
> > This I don't understand. Why couldn't you do a remain-on(some
> > other)-channel or scan while counting down the beacons?
> 
> My concern is software offchannel (be it scan or roc) involves channel
> context switches. I wanted to avoid any channel context mangling while
> CSA is in progress. Does that make sense to you?

No, I don't really get it. Why?

johannes

Re: [RFC 6/9] mac80211: track CSA globally

[Michal Kazior]

On 15 January 2014 14:29, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2014-01-15 at 14:28 +0100, Michal Kazior wrote:
>> On 15 January 2014 14:19, Johannes Berg <johannes@sipsolutions.net> wrote:
>> > On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
>> >> For CSA to be safe it needs to be treated the same
>> >> way as radar detection, scanning and remain on
>> >> channel - all of those (including CSA) must be
>> >> mutually exclusive.
>> >
>> > This I don't understand. Why couldn't you do a remain-on(some
>> > other)-channel or scan while counting down the beacons?
>>
>> My concern is software offchannel (be it scan or roc) involves channel
>> context switches. I wanted to avoid any channel context mangling while
>> CSA is in progress. Does that make sense to you?
>
> No, I don't really get it. Why?

Hmm. Perhaps I was a little overcautious. Offchannel stuff doesn't use
channel contexts at all, right? It recalculates channel in hw_config()
and doesn't touch channel contexts so it should be safe.

Also, I just noticed the patch contains some code that doesn't belong
here (vif_use_channel is protected with the csa_is_active() -- this
should be in the final commit). And this is probably the only part
that is necessary - to prevent anything new from binding to a channel
context that is being used for CSA.


Michał

Re: [RFC 6/9] mac80211: track CSA globally

[Johannes Berg]

On Wed, 2014-01-15 at 15:11 +0100, Michal Kazior wrote:

> >> >> For CSA to be safe it needs to be treated the same
> >> >> way as radar detection, scanning and remain on
> >> >> channel - all of those (including CSA) must be
> >> >> mutually exclusive.
> >> >
> >> > This I don't understand. Why couldn't you do a remain-on(some
> >> > other)-channel or scan while counting down the beacons?
> >>
> >> My concern is software offchannel (be it scan or roc) involves channel
> >> context switches. I wanted to avoid any channel context mangling while
> >> CSA is in progress. Does that make sense to you?
> >
> > No, I don't really get it. Why?
> 
> Hmm. Perhaps I was a little overcautious. Offchannel stuff doesn't use
> channel contexts at all, right? It recalculates channel in hw_config()
> and doesn't touch channel contexts so it should be safe.

Software offchannel and scan are incompatible with channel contexts,
yes.

johannes

[RFC 7/9] Revert "cfg80211: disable CSA for all drivers"

[Michal Kazior]

This reverts commit dda444d52496aa8ddc501561bca580f1374a96a9.

Re-enable CSA as the locking issues have been solved.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/core.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index d89dee2..aa3def7 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -443,12 +443,6 @@ int wiphy_register(struct wiphy *wiphy)
 	/* support for 5/10 MHz is broken due to nl80211 API mess - disable */
 	wiphy->flags &= ~WIPHY_FLAG_SUPPORTS_5_10_MHZ;
 
-	/*
-	 * There are major locking problems in nl80211/mac80211 for CSA,
-	 * disable for all drivers until this has been reworked.
-	 */
-	wiphy->flags &= ~WIPHY_FLAG_HAS_CHANNEL_SWITCH;
-
 #ifdef CONFIG_PM
 	if (WARN_ON(wiphy->wowlan &&
 		    (wiphy->wowlan->flags & WIPHY_WOWLAN_GTK_REKEY_FAILURE) &&
-- 
1.8.4.rc3

[RFC 8/9] ath9k: prepare for multi-interface CSA support

[Michal Kazior]

Soon mac80211 will support multi-interface CSA so
using sc->csa_vif is not an option.

Instead just depend on vif->csa_active. Calling
ieee80211_csa_finish() multiple number of times
should not be an issue.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h  |  4 ++--
 drivers/net/wireless/ath/ath9k/beacon.c | 29 +++++++++++++++++++----------
 drivers/net/wireless/ath/ath9k/main.c   | 12 ++----------
 drivers/net/wireless/ath/ath9k/xmit.c   |  2 +-
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index f622a98..e15b67a 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -436,7 +436,8 @@ void ath9k_beacon_config(struct ath_softc *sc, struct ieee80211_vif *vif,
 void ath9k_beacon_assign_slot(struct ath_softc *sc, struct ieee80211_vif *vif);
 void ath9k_beacon_remove_slot(struct ath_softc *sc, struct ieee80211_vif *vif);
 void ath9k_set_beacon(struct ath_softc *sc);
-bool ath9k_csa_is_finished(struct ath_softc *sc);
+bool ath9k_csa_is_finished(struct ath_softc *sc, struct ieee80211_vif *vif);
+void ath9k_csa_update(struct ath_softc *sc);
 
 /*******************/
 /* Link Monitoring */
@@ -766,7 +767,6 @@ struct ath_softc {
 #endif
 
 	struct ath_descdma txsdma;
-	struct ieee80211_vif *csa_vif;
 
 	struct ath_ant_comb ant_comb;
 	u8 ant_tx, ant_rx;
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index 2e8bba0..32d00e8 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -292,11 +292,8 @@ static void ath9k_set_tsfadjust(struct ath_softc *sc, struct ieee80211_vif *vif)
 		(unsigned long long)tsfadjust, avp->av_bslot);
 }
 
-bool ath9k_csa_is_finished(struct ath_softc *sc)
+bool ath9k_csa_is_finished(struct ath_softc *sc, struct ieee80211_vif *vif)
 {
-	struct ieee80211_vif *vif;
-
-	vif = sc->csa_vif;
 	if (!vif || !vif->csa_active)
 		return false;
 
@@ -304,11 +301,23 @@ bool ath9k_csa_is_finished(struct ath_softc *sc)
 		return false;
 
 	ieee80211_csa_finish(vif);
-
-	sc->csa_vif = NULL;
 	return true;
 }
 
+static void ath9k_csa_update_vif(void *data, u8 *mac, struct ieee80211_vif *vif)
+{
+	struct ath_softc *sc = data;
+	ath9k_csa_is_finished(sc, vif);
+}
+
+void ath9k_csa_update(struct ath_softc *sc)
+{
+	ieee80211_iterate_active_interfaces(sc->hw,
+					    IEEE80211_IFACE_ITER_NORMAL,
+					    ath9k_csa_update_vif,
+					    sc);
+}
+
 void ath9k_beacon_tasklet(unsigned long data)
 {
 	struct ath_softc *sc = (struct ath_softc *)data;
@@ -362,13 +371,13 @@ void ath9k_beacon_tasklet(unsigned long data)
 		return;
 	}
 
-	/* EDMA devices check that in the tx completion function. */
-	if (!edma && ath9k_csa_is_finished(sc))
-		return;
-
 	slot = ath9k_beacon_choose_slot(sc);
 	vif = sc->beacon.bslot[slot];
 
+	/* EDMA devices check that in the tx completion function. */
+	if (!edma && ath9k_csa_is_finished(sc, vif))
+		return;
+
 	if (!vif || !vif->bss_conf.enable_beacon)
 		return;
 
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index d0c3aec..6c8ba50 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1152,9 +1152,6 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
 	if (ath9k_uses_beacons(vif->type))
 		ath9k_beacon_remove_slot(sc, vif);
 
-	if (sc->csa_vif == vif)
-		sc->csa_vif = NULL;
-
 	ath9k_ps_wakeup(sc);
 	ath9k_calculate_summary_state(hw, NULL);
 	ath9k_ps_restore(sc);
@@ -2060,13 +2057,8 @@ static void ath9k_channel_switch_beacon(struct ieee80211_hw *hw,
 					struct ieee80211_vif *vif,
 					struct cfg80211_chan_def *chandef)
 {
-	struct ath_softc *sc = hw->priv;
-
-	/* mac80211 does not support CSA in multi-if cases (yet) */
-	if (WARN_ON(sc->csa_vif))
-		return;
-
-	sc->csa_vif = vif;
+	/* depend on vif->csa_active only */
+	return;
 }
 
 struct ieee80211_ops ath9k_ops = {
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index e8d0e7f..2bd35eb 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2568,7 +2568,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
 			sc->beacon.tx_processed = true;
 			sc->beacon.tx_last = !(ts.ts_status & ATH9K_TXERR_MASK);
 
-			ath9k_csa_is_finished(sc);
+			ath9k_csa_update(sc);
 			continue;
 		}
 
-- 
1.8.4.rc3

[RFC 9/9] mac80211: implement multi-interface CSA

[Michal Kazior]

This implements a fairly simple multi-interface
CSA. It doesn't support multiple channel
contexts so it doesn't support multi-channel.

Once a CSA is started other CSA requests are
denied until the first one is completed. A single
CSA may affect multiple interfaces. CSA can happen
only if it all target CSA chandefs are compatible
and it affects all interfaces are sharing a single
channel context exclusively.

A new worker is introduced: csa_complete_work
which is used to account per-interface countdowns
and issue the actual channel switch after last
interface completes its CSA.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c         | 364 ++++++++++++++++++++++++++++++++++++---------
 net/mac80211/chan.c        | 117 +++++++++++----
 net/mac80211/ibss.c        |   2 +-
 net/mac80211/ieee80211_i.h |  23 ++-
 net/mac80211/iface.c       |   5 +-
 net/mac80211/mlme.c        |   9 +-
 net/mac80211/tx.c          |  10 +-
 7 files changed, 419 insertions(+), 111 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 13d1624..873c57d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1093,9 +1093,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
 
 	/* abort any running channel switch */
 	mutex_lock(&local->mtx);
-	sdata->vif.csa_active = false;
-	kfree(sdata->u.ap.next_beacon);
-	sdata->u.ap.next_beacon = NULL;
+	ieee80211_csa_clear(sdata);
+	ieee80211_csa_free(sdata);
 	mutex_unlock(&local->mtx);
 
 	cancel_work_sync(&sdata->u.ap.request_smps_work);
@@ -3001,15 +3000,79 @@ cfg80211_beacon_dup(struct cfg80211_beacon_data *beacon)
 	return new_beacon;
 }
 
+static int ieee80211_ap_beacon_presp_backup(struct ieee80211_sub_if_data *sdata)
+{
+	struct beacon_data *beacon;
+	struct probe_resp *probe_resp;
+
+	beacon = sdata_dereference(sdata->u.ap.beacon, sdata);
+	if (beacon) {
+		sdata->u.ap.prev_beacon = kmemdup(beacon, sizeof(beacon) +
+						  beacon->head_len +
+						  beacon->tail_len, GFP_KERNEL);
+		if (!sdata->u.ap.prev_beacon)
+			return -ENOMEM;
+	}
+
+	probe_resp = sdata_dereference(sdata->u.ap.probe_resp, sdata);
+	if (probe_resp) {
+		sdata->u.ap.prev_presp = kmemdup(probe_resp,
+						 sizeof(probe_resp) +
+						 probe_resp->len, GFP_KERNEL);
+		if (!sdata->u.ap.prev_presp) {
+			kfree(sdata->u.ap.prev_beacon);
+			sdata->u.ap.prev_beacon = NULL;
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
+static int ieee80211_ap_beacon_presp_restore(struct ieee80211_sub_if_data *sdata)
+{
+	struct beacon_data *beacon;
+	struct probe_resp *probe_resp;
+	int changed = 0;
+
+	if (sdata->u.ap.prev_beacon) {
+		beacon = sdata_dereference(sdata->u.ap.beacon, sdata);
+		rcu_assign_pointer(sdata->u.ap.beacon, sdata->u.ap.prev_beacon);
+		if (beacon)
+			kfree_rcu(beacon, rcu_head);
+		sdata->u.ap.prev_beacon = NULL;
+		changed |= BSS_CHANGED_BEACON;
+	}
+
+	if (sdata->u.ap.prev_presp) {
+		probe_resp = sdata_dereference(sdata->u.ap.probe_resp, sdata);
+		rcu_assign_pointer(sdata->u.ap.probe_resp, sdata->u.ap.prev_presp);
+		if (probe_resp)
+			kfree_rcu(probe_resp, rcu_head);
+		sdata->u.ap.prev_presp = NULL;
+		changed |= BSS_CHANGED_AP_PROBE_RESP;
+	}
+
+	return changed;
+}
+
 static int ieee80211_ap_finish_csa(struct ieee80211_sub_if_data *sdata)
 {
-	int err;
+	int err = 0;
 
 	lockdep_assert_held(&sdata->local->mtx);
 
-	err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon);
+	if (sdata->u.ap.next_beacon)
+		err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon);
+	else
+		err = ieee80211_ap_beacon_presp_restore(sdata);
+
 	kfree(sdata->u.ap.next_beacon);
+	kfree(sdata->u.ap.prev_beacon);
+	kfree(sdata->u.ap.prev_presp);
 	sdata->u.ap.next_beacon = NULL;
+	sdata->u.ap.prev_beacon = NULL;
+	sdata->u.ap.prev_presp = NULL;
 
 	if (err < 0)
 		return err;
@@ -3018,88 +3081,227 @@ static int ieee80211_ap_finish_csa(struct ieee80211_sub_if_data *sdata)
 	return 0;
 }
 
-void ieee80211_csa_finalize_work(struct work_struct *work)
+void ieee80211_csa_clear(struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_sub_if_data *sdata =
-		container_of(work, struct ieee80211_sub_if_data,
-			     csa_finalize_work);
 	struct ieee80211_local *local = sdata->local;
-	int err, changed = 0;
 
-	sdata_lock(sdata);
-	mutex_lock(&local->mtx);
-	/* AP might have been stopped while waiting for the lock. */
-	if (!sdata->vif.csa_active)
-		goto unlock;
+	sdata_assert_lock(sdata);
+	lockdep_assert_held(&local->mtx);
 
-	if (!ieee80211_sdata_running(sdata))
-		goto unlock;
+	sdata->vif.csa_active = false;
+	sdata->csa_complete = false;
+
+	/* unblock queues when last CSA interface is cleared (either finalizes
+	 * or is cancelled) */
+	if (!ieee80211_is_csa_active(local))
+		ieee80211_wake_queues_by_reason(&local->hw,
+						IEEE80211_MAX_QUEUE_MAP,
+						IEEE80211_QUEUE_STOP_REASON_CSA);
+}
 
-	sdata->radar_required = sdata->csa_radar_required;
+void ieee80211_csa_free(struct ieee80211_sub_if_data *sdata)
+{
+	sdata_assert_lock(sdata);
+	lockdep_assert_held(&sdata->local->mtx);
 
-	err = ieee80211_vif_change_channel(sdata, &changed);
-	if (WARN_ON(err < 0))
-		goto unlock;
+	if (sdata->vif.type != NL80211_IFTYPE_AP)
+		return;
 
-	if (!local->use_chanctx) {
-		local->_oper_chandef = sdata->csa_chandef;
-		ieee80211_hw_config(local, 0);
-	}
+	kfree(sdata->u.ap.next_beacon);
+	kfree(sdata->u.ap.prev_beacon);
+	kfree(sdata->u.ap.prev_presp);
+	sdata->u.ap.next_beacon = NULL;
+	sdata->u.ap.prev_beacon = NULL;
+	sdata->u.ap.prev_presp = NULL;
+}
 
-	ieee80211_bss_info_change_notify(sdata, changed);
+static int ieee80211_csa_finish_beacon(struct ieee80211_sub_if_data *sdata)
+{
+	int err;
+
+	sdata_assert_lock(sdata);
+	lockdep_assert_held(&sdata->local->mtx);
 
-	sdata->vif.csa_active = false;
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_AP:
 		err = ieee80211_ap_finish_csa(sdata);
 		if (err < 0)
-			goto unlock;
+			return err;
 		break;
 	case NL80211_IFTYPE_ADHOC:
 		err = ieee80211_ibss_finish_csa(sdata);
 		if (err < 0)
-			goto unlock;
+			return err;
 		break;
 #ifdef CONFIG_MAC80211_MESH
 	case NL80211_IFTYPE_MESH_POINT:
 		err = ieee80211_mesh_finish_csa(sdata);
 		if (err < 0)
-			goto unlock;
+			return err;
 		break;
 #endif
 	default:
 		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+void ieee80211_csa_finalize_work(struct work_struct *work)
+{
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     csa_finalize_work);
+	struct ieee80211_local *local = sdata->local;
+	int err, changed = 0;
+
+	sdata_lock(sdata);
+	mutex_lock(&local->mtx);
+
+	if (!ieee80211_sdata_running(sdata))
+		goto unlock;
+
+	if (!sdata->vif.csa_active)
+		goto unlock;
+
+	if (sdata->vif.bss_conf.chandef.width != sdata->csa_chandef.width)
+		changed |= BSS_CHANGED_BANDWIDTH;
+
+	/* channel switch is called for each sdata csa is being performed, but
+	 * this shouldn't be a problem */
+	mutex_lock(&local->chanctx_mtx);
+	err = ieee80211_chanctx_chswitch(local);
+	mutex_unlock(&local->chanctx_mtx);
+
+	if (WARN_ON(err < 0))
 		goto unlock;
+
+	if (!local->use_chanctx) {
+		local->_oper_chandef = sdata->csa_chandef;
+		ieee80211_hw_config(local, 0);
 	}
 
-	ieee80211_wake_queues_by_reason(&sdata->local->hw,
-					IEEE80211_MAX_QUEUE_MAP,
-					IEEE80211_QUEUE_STOP_REASON_CSA);
+	ieee80211_bss_info_change_notify(sdata, changed);
+
+	err = ieee80211_csa_finish_beacon(sdata);
+	if (err)
+		goto unlock;
 
 	cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
 
 unlock:
+	ieee80211_csa_clear(sdata);
+
 	mutex_unlock(&local->mtx);
 	sdata_unlock(sdata);
 }
 
+static bool ieee80211_is_csa_complete(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata;
+	int num_active = 0;
+	int num_complete = 0;
+
+	lockdep_assert_held(&local->mtx);
+	lockdep_assert_held(&local->iflist_mtx);
+
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		if (sdata->vif.csa_active)
+			num_active++;
+		if (sdata->csa_complete)
+			num_complete++;
+	}
+
+	if (num_active == 0)
+		return false;
+	if (num_active != num_complete)
+		return false;
+
+	return true;
+}
+
+static void ieee80211_queue_csa_finalize(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	lockdep_assert_held(&local->mtx);
+	lockdep_assert_held(&local->iflist_mtx);
+
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		if (!ieee80211_sdata_running(sdata))
+			continue;
+
+		if (!sdata->vif.csa_active)
+			continue;
+
+		ieee80211_queue_work(&local->hw, &sdata->csa_finalize_work);
+	}
+}
+
+void ieee80211_csa_complete_work(struct work_struct *work)
+{
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     csa_complete_work);
+	struct ieee80211_local *local = sdata->local;
+
+	mutex_lock(&local->mtx);
+	mutex_lock(&local->iflist_mtx);
+
+	if (sdata->vif.csa_active)
+		sdata->csa_complete = true;
+
+	if (ieee80211_is_csa_complete(sdata->local))
+		ieee80211_queue_csa_finalize(sdata->local);
+
+	mutex_unlock(&local->iflist_mtx);
+	mutex_unlock(&local->mtx);
+}
+
+static void ieee80211_channel_switch_abort(struct wiphy *wiphy,
+					   struct net_device *dev,
+					   struct cfg80211_csa_settings *params)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_if_mesh __maybe_unused *ifmsh;
+
+	sdata_assert_lock(sdata);
+	lockdep_assert_held(&local->mtx);
+
+	ieee80211_csa_clear(sdata);
+
+	/* force to switch to previous AP beacon */
+	kfree(sdata->u.ap.next_beacon);
+	sdata->u.ap.next_beacon = NULL;
+
+	ieee80211_csa_finish_beacon(sdata);
+}
+
 int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
-			       struct cfg80211_csa_settings *params)
+			       struct cfg80211_csa_settings *params,
+			       bool first)
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_chanctx_conf *chanctx_conf;
 	struct ieee80211_chanctx *chanctx;
 	struct ieee80211_if_mesh __maybe_unused *ifmsh;
-	int err, num_chanctx;
+	int err;
 
 	sdata_assert_lock(sdata);
 	lockdep_assert_held(&local->mtx);
 
-	if (!list_empty(&local->roc_list) || local->scanning ||
-	    ieee80211_is_csa_active(local))
+	/* only first csa call-in should check this, otherwise second csa for a
+	 * multi-interface csa would always fail */
+	if (first && (!list_empty(&local->roc_list) ||
+		      local->scanning ||
+		      ieee80211_is_csa_active(local)))
 		return -EBUSY;
 
+	if (!ieee80211_sdata_running(sdata))
+		return -ENETDOWN;
+
 	if (sdata->wdev.cac_started)
 		return -EBUSY;
 
@@ -3108,24 +3310,10 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 		return -EINVAL;
 
 	mutex_lock(&local->chanctx_mtx);
-	chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
-	if (!chanctx_conf) {
-		mutex_unlock(&local->chanctx_mtx);
-		return -EBUSY;
-	}
-
-	/* don't handle for multi-VIF cases */
-	chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
-	if (chanctx->refcount > 1) {
-		mutex_unlock(&local->chanctx_mtx);
-		return -EBUSY;
-	}
-	num_chanctx = 0;
-	list_for_each_entry(chanctx, &local->chanctx_list, list)
-		num_chanctx++;
+	chanctx = ieee80211_get_csa_chanctx(local);
 	mutex_unlock(&local->chanctx_mtx);
 
-	if (num_chanctx > 1)
+	if (!chanctx)
 		return -EBUSY;
 
 	/* don't allow another channel switch if one is already active. */
@@ -3142,9 +3330,15 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 		if (!sdata->u.ap.next_beacon)
 			return -ENOMEM;
 
+		err = ieee80211_ap_beacon_presp_backup(sdata);
+		if (err) {
+			ieee80211_csa_free(sdata);
+			return -ENOMEM;
+		}
+
 		err = ieee80211_assign_beacon(sdata, &params->beacon_csa);
 		if (err < 0) {
-			kfree(sdata->u.ap.next_beacon);
+			ieee80211_csa_free(sdata);
 			return err;
 		}
 		break;
@@ -3226,26 +3420,62 @@ int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static int ieee80211_csa_allowed_settings(struct cfg80211_csa_settings *params,
+					  int num_params)
+{
+	const struct cfg80211_chan_def *chandef;
+	int i;
+
+	if (num_params == 0)
+		return -EINVAL;
+
+	chandef = &params[0].chandef;
+	for (i = 1; i < num_params; i++) {
+		chandef = cfg80211_chandef_compatible(chandef,
+						      &params[i].chandef);
+		if (!chandef)
+			return -EBUSY;
+	}
+
+	return 0;
+}
+
 int ieee80211_channel_switch(struct wiphy *wiphy,
 			     struct cfg80211_csa_settings *params,
 			     int num_params)
 {
 	struct ieee80211_sub_if_data *sdata;
-	int err;
-
-	/* multi-vif CSA is not implemented */
-	if (num_params > 1)
-		return -EOPNOTSUPP;
+	int err, i;
 
-	sdata = IEEE80211_DEV_TO_SUB_IF(params[0].dev);
+	err = ieee80211_csa_allowed_settings(params, num_params);
+	if (err)
+		return err;
 
-	sdata_lock(sdata);
-	mutex_lock(&sdata->local->mtx);
-	err = __ieee80211_channel_switch(wiphy, params[0].dev, &params[0]);
-	mutex_unlock(&sdata->local->mtx);
-	sdata_unlock(sdata);
+	for (i = 0; i < num_params; i++) {
+		sdata = IEEE80211_DEV_TO_SUB_IF(params[i].dev);
+
+		sdata_lock(sdata);
+		mutex_lock(&sdata->local->mtx);
+		err = __ieee80211_channel_switch(wiphy, params[i].dev,
+						 &params[i], i == 0);
+		mutex_unlock(&sdata->local->mtx);
+		sdata_unlock(sdata);
+
+		if (err) {
+			for (i--; i >= 0; i--) {
+				sdata_lock(sdata);
+				mutex_lock(&sdata->local->mtx);
+				ieee80211_channel_switch_abort(wiphy,
+							       params[i].dev,
+							       &params[i]);
+				mutex_unlock(&sdata->local->mtx);
+				sdata_unlock(sdata);
+			}
+			return err;
+		}
+	}
 
-	return err;
+	return 0;
 }
 
 static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index f43613a..71e3e18 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -545,49 +545,103 @@ int ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
 	return ret;
 }
 
-int ieee80211_vif_change_channel(struct ieee80211_sub_if_data *sdata,
-				 u32 *changed)
+const struct cfg80211_chan_def *
+ieee80211_get_csa_chandef(struct ieee80211_local *local)
 {
-	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_chanctx_conf *conf;
-	struct ieee80211_chanctx *ctx;
-	const struct cfg80211_chan_def *chandef = &sdata->csa_chandef;
-	int ret;
-	u32 chanctx_changed = 0;
+	struct ieee80211_sub_if_data *sdata;
+	const struct cfg80211_chan_def *chandef = NULL;
 
 	lockdep_assert_held(&local->mtx);
+	lockdep_assert_held(&local->chanctx_mtx);
 
-	/* should never be called if not performing a channel switch. */
-	if (WARN_ON(!sdata->vif.csa_active))
-		return -EINVAL;
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		if (!sdata->vif.csa_active)
+			continue;
 
-	if (!cfg80211_chandef_usable(sdata->local->hw.wiphy, chandef,
-				     IEEE80211_CHAN_DISABLED))
-		return -EINVAL;
+		if (!sdata->csa_complete)
+			return NULL;
 
-	mutex_lock(&local->chanctx_mtx);
-	conf = rcu_dereference_protected(sdata->vif.chanctx_conf,
-					 lockdep_is_held(&local->chanctx_mtx));
-	if (!conf) {
-		ret = -EINVAL;
-		goto out;
+		if (chandef == NULL)
+			chandef = &sdata->csa_chandef;
+		else
+			chandef = cfg80211_chandef_compatible(
+					chandef, &sdata->csa_chandef);
+
+		if (!chandef)
+			return NULL;
 	}
 
-	ctx = container_of(conf, struct ieee80211_chanctx, conf);
-	if (ctx->refcount != 1) {
-		ret = -EINVAL;
-		goto out;
+	return chandef;
+}
+
+static void ieee80211_use_csa_chandef(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		if (!sdata->vif.csa_active)
+			continue;
+
+		sdata->radar_required = sdata->csa_radar_required;
+		sdata->vif.bss_conf.chandef = sdata->csa_chandef;
 	}
+}
 
-	if (sdata->vif.bss_conf.chandef.width != chandef->width) {
-		chanctx_changed = IEEE80211_CHANCTX_CHANGE_WIDTH;
-		*changed |= BSS_CHANGED_BANDWIDTH;
+struct ieee80211_chanctx *
+ieee80211_get_csa_chanctx(struct ieee80211_local *local)
+{
+	struct ieee80211_chanctx *chanctx = NULL, *ctx;
+	int num_chanctx = 0;
+
+	lockdep_assert_held(&local->chanctx_mtx);
+
+	list_for_each_entry(ctx, &local->chanctx_list, list) {
+		chanctx = ctx;
+		num_chanctx++;
 	}
 
-	sdata->vif.bss_conf.chandef = *chandef;
-	ctx->conf.def = *chandef;
+	/* multi-channel is not supported, multi-vif is */
+	if (num_chanctx > 1)
+		return NULL;
+
+	return chanctx;
+}
+
+int ieee80211_chanctx_chswitch(struct ieee80211_local *local)
+{
+	u32 chanctx_changed = 0;
+	struct ieee80211_chanctx *ctx;
+	const struct cfg80211_chan_def *chandef;
+
+	lockdep_assert_held(&local->mtx);
+	lockdep_assert_held(&local->chanctx_mtx);
+
+	ctx = ieee80211_get_csa_chanctx(local);
+	if (!ctx)
+		return -EBUSY;
+
+	rcu_read_lock();
+	chandef = ieee80211_get_csa_chandef(local);
+	if (!chandef) {
+		rcu_read_unlock();
+		return -EINVAL;
+	}
+
+	if (!cfg80211_chandef_usable(local->hw.wiphy, chandef,
+				     IEEE80211_CHAN_DISABLED)) {
+		rcu_read_unlock();
+		return -EINVAL;
+	}
+
+	ieee80211_use_csa_chandef(local);
+	rcu_read_unlock();
 
 	chanctx_changed |= IEEE80211_CHANCTX_CHANGE_CHANNEL;
+
+	if (ctx->conf.def.width != chandef->width)
+		chanctx_changed = IEEE80211_CHANCTX_CHANGE_WIDTH;
+
+	ctx->conf.def = *chandef;
 	drv_change_chanctx(local, ctx, chanctx_changed);
 
 	ieee80211_recalc_chanctx_chantype(local, ctx);
@@ -595,10 +649,7 @@ int ieee80211_vif_change_channel(struct ieee80211_sub_if_data *sdata,
 	ieee80211_recalc_radar_chanctx(local, ctx);
 	ieee80211_recalc_chanctx_min_def(local, ctx);
 
-	ret = 0;
- out:
-	mutex_unlock(&local->chanctx_mtx);
-	return ret;
+	return 0;
 }
 
 int ieee80211_vif_change_bandwidth(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 12c6019..081beba 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -920,7 +920,7 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	params.block_tx = !!csa_ie.mode;
 
 	if (__ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
-				       &params))
+				       &params, true))
 		goto disconnect;
 
 	ieee80211_ibss_csa_mark_radar(sdata);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e523429..3adc5ea 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -262,6 +262,10 @@ struct ieee80211_if_ap {
 	struct cfg80211_beacon_data *next_beacon;
 	struct list_head vlans;
 
+	/* to be used if channel switch fails. */
+	struct beacon_data *prev_beacon;
+	struct probe_resp *prev_presp;
+
 	struct ps_data ps;
 	atomic_t num_mcast_sta; /* number of stations receiving multicast */
 	enum ieee80211_smps_mode req_smps, /* requested smps mode */
@@ -701,6 +705,7 @@ struct mac80211_qos_map {
 
 struct ieee80211_sub_if_data {
 	struct list_head list;
+	struct list_head csa_list;
 
 	struct wireless_dev wdev;
 
@@ -747,9 +752,11 @@ struct ieee80211_sub_if_data {
 	struct mac80211_qos_map __rcu *qos_map;
 
 	struct work_struct csa_finalize_work;
+	struct work_struct csa_complete_work;
 	int csa_counter_offset_beacon;
 	int csa_counter_offset_presp;
 	bool csa_radar_required;
+	bool csa_complete;
 	struct cfg80211_chan_def csa_chandef;
 
 	/* used to reconfigure hardware SM PS */
@@ -1457,12 +1464,22 @@ void ieee80211_handle_roc_started(struct ieee80211_roc_work *roc);
 
 /* channel switch handling */
 void ieee80211_csa_finalize_work(struct work_struct *work);
+void ieee80211_csa_complete_work(struct work_struct *work);
 int __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
-			       struct cfg80211_csa_settings *params);
+			       struct cfg80211_csa_settings *params,
+			       bool first);
 int ieee80211_channel_switch(struct wiphy *wiphy,
 			     struct cfg80211_csa_settings *params,
 			     int num_params);
 bool ieee80211_is_csa_active(struct ieee80211_local *local);
+void ieee80211_csa_clear(struct ieee80211_sub_if_data *sdata);
+void ieee80211_csa_free(struct ieee80211_sub_if_data *sdata);
+const struct cfg80211_chan_def *
+ieee80211_get_csa_chandef(struct ieee80211_local *local);
+struct ieee80211_chanctx *
+ieee80211_get_csa_chanctx(struct ieee80211_local *local);
+int ieee80211_chanctx_csa(struct ieee80211_local *local);
+int ieee80211_chanctx_chswitch(struct ieee80211_local *local);
 
 /* interface handling */
 int ieee80211_iface_init(void);
@@ -1775,10 +1792,6 @@ int __must_check
 ieee80211_vif_change_bandwidth(struct ieee80211_sub_if_data *sdata,
 			       const struct cfg80211_chan_def *chandef,
 			       u32 *changed);
-/* NOTE: only use ieee80211_vif_change_channel() for channel switch */
-int __must_check
-ieee80211_vif_change_channel(struct ieee80211_sub_if_data *sdata,
-			     u32 *changed);
 void ieee80211_vif_release_channel(struct ieee80211_sub_if_data *sdata);
 void ieee80211_vif_vlan_copy_chanctx(struct ieee80211_sub_if_data *sdata);
 void ieee80211_vif_copy_chanctx_to_vlans(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 58cc061..0286a73 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -846,9 +846,11 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 	cancel_work_sync(&sdata->recalc_smps);
 	sdata_lock(sdata);
 	mutex_lock(&local->mtx);
-	sdata->vif.csa_active = false;
+	ieee80211_csa_clear(sdata);
+	ieee80211_csa_free(sdata);
 	mutex_unlock(&local->mtx);
 	sdata_unlock(sdata);
+	cancel_work_sync(&sdata->csa_complete_work);
 	cancel_work_sync(&sdata->csa_finalize_work);
 
 	cancel_delayed_work_sync(&sdata->dfs_cac_timer_work);
@@ -1294,6 +1296,7 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
 	INIT_WORK(&sdata->work, ieee80211_iface_work);
 	INIT_WORK(&sdata->recalc_smps, ieee80211_recalc_smps_work);
 	INIT_WORK(&sdata->csa_finalize_work, ieee80211_csa_finalize_work);
+	INIT_WORK(&sdata->csa_complete_work, ieee80211_csa_complete_work);
 
 	switch (type) {
 	case NL80211_IFTYPE_P2P_GO:
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b3fa66a..a898036 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -889,7 +889,14 @@ static void ieee80211_chswitch_work(struct work_struct *work)
 	if (!ifmgd->associated)
 		goto out;
 
-	ret = ieee80211_vif_change_channel(sdata, &changed);
+	if (sdata->vif.bss_conf.chandef.width !=
+	    sdata->csa_chandef.width)
+		changed |= BSS_CHANGED_BANDWIDTH;
+
+	mutex_lock(&local->chanctx_mtx);
+	ret = ieee80211_chanctx_chswitch(local);
+	mutex_unlock(&local->chanctx_mtx);
+
 	if (ret) {
 		sdata_info(sdata,
 			   "vif channel switch failed, disconnecting\n");
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index ef3555e..9d4567c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2405,7 +2405,7 @@ void ieee80211_csa_finish(struct ieee80211_vif *vif)
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
 
 	ieee80211_queue_work(&sdata->local->hw,
-			     &sdata->csa_finalize_work);
+			     &sdata->csa_complete_work);
 }
 EXPORT_SYMBOL(ieee80211_csa_finish);
 
@@ -2437,9 +2437,13 @@ static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata,
 	if (WARN_ON(counter_offset_beacon >= beacon_data_len))
 		return;
 
-	/* warn if the driver did not check for/react to csa completeness */
-	if (WARN_ON(beacon_data[counter_offset_beacon] == 0))
+	if (beacon_data[counter_offset_beacon] == 0) {
+		/* warn if the driver did not check for/react to csa
+		 * completeness. keep in mind that for multi-interface csa some
+		 * BSSes may need to wait for others to complete */
+		WARN_ON(!sdata->csa_complete);
 		return;
+	}
 
 	beacon_data[counter_offset_beacon]--;
 
-- 
1.8.4.rc3

Re: [RFC 9/9] mac80211: implement multi-interface CSA

[Johannes Berg]

On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
> This implements a fairly simple multi-interface
> CSA. It doesn't support multiple channel
> contexts so it doesn't support multi-channel.

It seems that this also means that somewhere you need to advertise the
multi-switch feature. I guess you need that anyway, but this makes it
obvious.

johannes

Re: [RFC 9/9] mac80211: implement multi-interface CSA

[Michal Kazior]

On 15 January 2014 14:22, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
>> This implements a fairly simple multi-interface
>> CSA. It doesn't support multiple channel
>> contexts so it doesn't support multi-channel.
>
> It seems that this also means that somewhere you need to advertise the
> multi-switch feature. I guess you need that anyway, but this makes it
> obvious.

I don't really understand. Do we really need it? You want to advertise
it just to cfg80211 or all the way up to userspace?


Michał

Re: [RFC 9/9] mac80211: implement multi-interface CSA

[Johannes Berg]

On Thu, 2014-01-16 at 10:28 +0100, Michal Kazior wrote:
> On 15 January 2014 14:22, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
> >> This implements a fairly simple multi-interface
> >> CSA. It doesn't support multiple channel
> >> contexts so it doesn't support multi-channel.
> >
> > It seems that this also means that somewhere you need to advertise the
> > multi-switch feature. I guess you need that anyway, but this makes it
> > obvious.
> 
> I don't really understand. Do we really need it? You want to advertise
> it just to cfg80211 or all the way up to userspace?

It seems it would be needed all the way to userspace so it knows whether
it can expect multi-vif CSA to be available? Or would you just want to
try & fail? Wouldn't that potentially impact channel selection since
you'd want to not pick radar channels for multi-vif? Not really sure ...

Maybe the expectation is that internally there will be no drivers that
don't support it, i.e. if a driver implements the cfg80211 API at all
then it must support multi-vif CSA?

johannes

Re: [RFC 9/9] mac80211: implement multi-interface CSA

[Michal Kazior]

On 16 January 2014 10:31, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2014-01-16 at 10:28 +0100, Michal Kazior wrote:
>> On 15 January 2014 14:22, Johannes Berg <johannes@sipsolutions.net> wrote:
>> > On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
>> >> This implements a fairly simple multi-interface
>> >> CSA. It doesn't support multiple channel
>> >> contexts so it doesn't support multi-channel.
>> >
>> > It seems that this also means that somewhere you need to advertise the
>> > multi-switch feature. I guess you need that anyway, but this makes it
>> > obvious.
>>
>> I don't really understand. Do we really need it? You want to advertise
>> it just to cfg80211 or all the way up to userspace?
>
> It seems it would be needed all the way to userspace so it knows whether
> it can expect multi-vif CSA to be available? Or would you just want to
> try & fail? Wouldn't that potentially impact channel selection since
> you'd want to not pick radar channels for multi-vif? Not really sure ...
>
> Maybe the expectation is that internally there will be no drivers that
> don't support it, i.e. if a driver implements the cfg80211 API at all
> then it must support multi-vif CSA?

Typically if you run more than one AP on a DFS channel and you detect
a radar you want to move all APs - so you either succeed with
multi-vif CSA or you fallback and re-start them.

If you assume a driver supports multi-channel but isn't capable of
multi-interface CSA you'll be running into timing issues. After you
detect radar you have a limited time to quiesce. You might end up
exceeding that limit if you move APs one-by-one.

With my patches as they are new hostap will try sending new channel
switch command but since it doesn't contain ifindex old kernel will
respond with a -EINVAL and userspace has no way of knowing if the
command was actually malformed or isn't supported. hostap could still,
however, work in a best effort manner and fallback to the old command
variant.


Michał

Re: [RFC 9/9] mac80211: implement multi-interface CSA

[Johannes Berg]

On Thu, 2014-01-16 at 11:33 +0100, Michal Kazior wrote:

> If you assume a driver supports multi-channel but isn't capable of
> multi-interface CSA you'll be running into timing issues. After you
> detect radar you have a limited time to quiesce. You might end up
> exceeding that limit if you move APs one-by-one.

That probably won't work anyway since you could probably not move them
one by one anyway due to channel context limitations.

> With my patches as they are new hostap will try sending new channel
> switch command but since it doesn't contain ifindex old kernel will
> respond with a -EINVAL and userspace has no way of knowing if the
> command was actually malformed or isn't supported. hostap could still,
> however, work in a best effort manner and fallback to the old command
> variant.

Well, you're looking at it reactively. I was more thinking hostapd might
want to look at it proactively and, for example, refuse setting up
multiple BSSes on a radar channel if multi-switch isn't supported. But
for that, it has to know upfront rather than realizing it reactively
when trying to switch.

johannes

Re: [RFC 9/9] mac80211: implement multi-interface CSA

[Eliad Peller]

On Wed, Jan 15, 2014 at 2:04 PM, Michal Kazior <michal.kazior@tieto.com> wrote:
> This implements a fairly simple multi-interface
> CSA. It doesn't support multiple channel
> contexts so it doesn't support multi-channel.
>
> Once a CSA is started other CSA requests are
> denied until the first one is completed. A single
> CSA may affect multiple interfaces. CSA can happen
> only if it all target CSA chandefs are compatible
> and it affects all interfaces are sharing a single
> channel context exclusively.
>
> A new worker is introduced: csa_complete_work
> which is used to account per-interface countdowns
> and issue the actual channel switch after last
> interface completes its CSA.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
[...]

>
> +static int ieee80211_ap_beacon_presp_backup(struct ieee80211_sub_if_data *sdata)
> +{
> +       struct beacon_data *beacon;
> +       struct probe_resp *probe_resp;
> +
> +       beacon = sdata_dereference(sdata->u.ap.beacon, sdata);
> +       if (beacon) {
> +               sdata->u.ap.prev_beacon = kmemdup(beacon, sizeof(beacon) +
> +                                                 beacon->head_len +
> +                                                 beacon->tail_len, GFP_KERNEL);
> +               if (!sdata->u.ap.prev_beacon)
> +                       return -ENOMEM;
> +       }
> +
you must also update the pointers here...

Eliad.

Re: [RFC 9/9] mac80211: implement multi-interface CSA

[Michal Kazior]

On 16 January 2014 12:04, Eliad Peller <eliad@wizery.com> wrote:
> On Wed, Jan 15, 2014 at 2:04 PM, Michal Kazior <michal.kazior@tieto.com> wrote:
>> This implements a fairly simple multi-interface
>> CSA. It doesn't support multiple channel
>> contexts so it doesn't support multi-channel.
>>
>> Once a CSA is started other CSA requests are
>> denied until the first one is completed. A single
>> CSA may affect multiple interfaces. CSA can happen
>> only if it all target CSA chandefs are compatible
>> and it affects all interfaces are sharing a single
>> channel context exclusively.
>>
>> A new worker is introduced: csa_complete_work
>> which is used to account per-interface countdowns
>> and issue the actual channel switch after last
>> interface completes its CSA.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
> [...]
>
>>
>> +static int ieee80211_ap_beacon_presp_backup(struct ieee80211_sub_if_data *sdata)
>> +{
>> +       struct beacon_data *beacon;
>> +       struct probe_resp *probe_resp;
>> +
>> +       beacon = sdata_dereference(sdata->u.ap.beacon, sdata);
>> +       if (beacon) {
>> +               sdata->u.ap.prev_beacon = kmemdup(beacon, sizeof(beacon) +
>> +                                                 beacon->head_len +
>> +                                                 beacon->tail_len, GFP_KERNEL);
>> +               if (!sdata->u.ap.prev_beacon)
>> +                       return -ENOMEM;
>> +       }
>> +
> you must also update the pointers here...

You're right, thanks!


Michał

Re: [RFC 0/9] mac80211: multi-interface CSA

[Johannes Berg]

On Wed, 2014-01-15 at 13:04 +0100, Michal Kazior wrote:
> Hi,
> 
> This patchet bases of my recently posted cfg80211
> multi-interface CSA patchset.
> 
> Probably some patches in this patchset can be
> cherry-picked regardless if the multi-interface
> CSA itself is accepted or not.
> 
> I'm planning on posting properly split patchsets
> after getting some feedback on the mac80211 part.

I guess overall this looks fine, as Luca said it needs to be rebased.

It'd be great to have some tests in the hostapd test suite (once support
is there) so I can run them as well over hwsim.

johannes