Discussion:
[RFC 2/9] mac80211: treat IBSS CSA finish failure seriously
(too old to reply)
Michal Kazior
2014-01-15 12:04:47 UTC
Permalink
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-++***@public.gmane.org>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 12:04:48 UTC
Permalink
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-++***@public.gmane.org>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-15 13:17:33 UTC
Permalink
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


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 12:04:50 UTC
Permalink
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-++***@public.gmane.org>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 12:04:52 UTC
Permalink
This reverts commit dda444d52496aa8ddc501561bca580f1374a96a9.

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

Signed-off-by: Michal Kazior <michal.kazior-++***@public.gmane.org>
---
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;
Michal Kazior
2014-01-15 12:04:53 UTC
Permalink
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-++***@public.gmane.org>
---
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;
Michal Kazior
2014-01-15 12:04:51 UTC
Permalink
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-++***@public.gmane.org>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-15 13:19:23 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 13:28:14 UTC
Permalink
On 15 January 2014 14:19, Johannes Berg <johannes-***@public.gmane.org> wro=
te:
> 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=C5=82
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-15 13:29:28 UTC
Permalink
On Wed, 2014-01-15 at 14:28 +0100, Michal Kazior wrote:
> On 15 January 2014 14:19, Johannes Berg <johannes-***@public.gmane.org> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 14:11:19 UTC
Permalink
On 15 January 2014 14:29, Johannes Berg <johannes-***@public.gmane.org> wro=
te:
> On Wed, 2014-01-15 at 14:28 +0100, Michal Kazior wrote:
>> On 15 January 2014 14:19, Johannes Berg <johannes-***@public.gmane.org> =
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 chann=
el
>> context switches. I wanted to avoid any channel context mangling whi=
le
>> 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=C5=82
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-15 14:21:08 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 12:04:49 UTC
Permalink
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-++***@public.gmane.org>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 12:04:54 UTC
Permalink
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-++***@public.gmane.org>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-15 13:22:19 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-16 09:28:04 UTC
Permalink
On 15 January 2014 14:22, Johannes Berg <johannes-***@public.gmane.org> wro=
te:
> 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 th=
e
> 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=C5=82
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-16 09:31:46 UTC
Permalink
On Thu, 2014-01-16 at 10:28 +0100, Michal Kazior wrote:
> On 15 January 2014 14:22, Johannes Berg <johannes-***@public.gmane.org> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-16 10:33:27 UTC
Permalink
On 16 January 2014 10:31, Johannes Berg <johannes-***@public.gmane.org> wro=
te:
> On Thu, 2014-01-16 at 10:28 +0100, Michal Kazior wrote:
>> On 15 January 2014 14:22, Johannes Berg <johannes-***@public.gmane.org> =
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 adverti=
se
>> 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 whet=
her
> it can expect multi-vif CSA to be available? Or would you just want t=
o
> try & fail? Wouldn't that potentially impact channel selection since
> you'd want to not pick radar channels for multi-vif? Not really sure =
=2E..
>
> Maybe the expectation is that internally there will be no drivers tha=
t
> 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=C5=82
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-20 11:06:45 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Eliad Peller
2014-01-16 11:04:03 UTC
Permalink
On Wed, Jan 15, 2014 at 2:04 PM, Michal Kazior <michal.kazior-++***@public.gmane.org> 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-++***@public.gmane.org>
> ---
[...]

>
> +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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-16 11:41:33 UTC
Permalink
On 16 January 2014 12:04, Eliad Peller <eliad-Ix1uc/W3ht7QT0dZR+***@public.gmane.org> wrote:
> On Wed, Jan 15, 2014 at 2:04 PM, Michal Kazior <***@tieto.c=
om> 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-++***@public.gmane.org>
>> ---
> [...]
>
>>
>> +static int ieee80211_ap_beacon_presp_backup(struct ieee80211_sub_if=
_data *sdata)
>> +{
>> + struct beacon_data *beacon;
>> + struct probe_resp *probe_resp;
>> +
>> + beacon =3D sdata_dereference(sdata->u.ap.beacon, sdata);
>> + if (beacon) {
>> + sdata->u.ap.prev_beacon =3D kmemdup(beacon, sizeof(b=
eacon) +
>> + 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=C5=82
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Michal Kazior
2014-01-15 12:04:46 UTC
Permalink
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-++***@public.gmane.org>
---
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;
Luca Coelho
2014-01-15 13:07:34 UTC
Permalink
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-++***@public.gmane.org>
> ---
> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-01-15 13:21:31 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...