Hi,
More changes, hopefully the last ones for this series. ;)
I have hacked wpa_supplicant to support CSA in IBSS, so I could test
it too. I created some ma80211_hwsim testcases on top of this. The
tests now pass, though I get some semi-random WARNs [1] when using 3
stations. It seems to be a race condition and it doesn't seem to be
related to this series, so I think we should address it separately
(Simon, hint! ;).
I have also created some basic BSS CSA test cases for mac80211_hwsim.
The tests pass, except when I try count < 2, which is expected, since
we did not implement the CSA action frame in BSS mode.
I'll upstream the BSS CSA test cases soon. The IBSS test cases still
need to be cleaned up (actually they're worthless without the IBSS CSA
support in wpa_supplicant, which needs to be cleaned up). If someone
is interested in this, let me know and I can send them
"without-warranty of any kind". ;)
Please review and test!
In v9:
* fix lock in ieee80211_ibss_finish_csa();
* remove double check for sdata_running() (thanks Simon);
* check cfg80211_chandef_identical before calling
ieee80211_channel_switch() to avoid disconnecting;
In v8:
* removed bogus sdata_unlock from ieee80211_csa_finalize();
In v7:
* (hopefully) a fix for the kfree_rcu() oops (thanks Chun-Yeow);
* removed stray extra space at the top of cfg.c (thanks Johannes);
* removed the TODO action frame comment for mesh and IBSS (thanks Simon);
--
Cheers,
Luca.
Luciano Coelho (5):
mac80211: refactor ieee80211_ibss_process_chanswitch()
mac80211: align ieee80211_ibss_csa_beacon() with
ieee80211_csa_beacon()
mac80211: refactor ieee80211_mesh_process_chanswitch()
mac80211: align ieee80211_mesh_csa_beacon() with
ieee80211_csa_beacon()
mac80211: only set CSA beacon when at least one beacon must be
transmitted
include/net/mac80211.h | 10 ++--
net/mac80211/cfg.c | 121 ++++++++++++++++++++++++++++++++++-----------
net/mac80211/ibss.c | 72 +++++++--------------------
net/mac80211/ieee80211_i.h | 11 +++--
net/mac80211/mesh.c | 86 +++++++++-----------------------
net/mac80211/tx.c | 19 +++----
net/mac80211/util.c | 1 -
7 files changed, 153 insertions(+), 167 deletions(-)
--
1.8.5.1
Refactor ieee80211_ibss_process_chanswitch() to use
ieee80211_channel_switch() and avoid code duplication.
Change-Id: I265a12c7f825dc20535bad1197a81437310d0086
Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/cfg.c | 4 +--
net/mac80211/ibss.c | 61 ++++++++++------------------------------------
net/mac80211/ieee80211_i.h | 2 ++
3 files changed, 17 insertions(+), 50 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a74d61d..96aebe5 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3048,8 +3048,8 @@ unlock:
sdata_unlock(sdata);
}
-static int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
- struct cfg80211_csa_settings *params)
+int ieee80211_channel_switch(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;
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index df21e4f..0c3a45c 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -784,18 +784,10 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
struct cfg80211_csa_settings params;
struct ieee80211_csa_ie csa_ie;
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
- struct ieee80211_chanctx_conf *chanctx_conf;
- struct ieee80211_chanctx *chanctx;
enum nl80211_channel_type ch_type;
- int err, num_chanctx;
+ int err;
u32 sta_flags;
- if (sdata->vif.csa_active)
- return true;
-
- if (!sdata->vif.bss_conf.ibss_joined)
- return false;
-
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (ifibss->chandef.width) {
case NL80211_CHAN_WIDTH_5:
@@ -826,9 +818,6 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.count = csa_ie.count;
params.chandef = csa_ie.chandef;
- if (ifibss->chandef.chan->band != params.chandef.chan->band)
- goto disconnect;
-
switch (ifibss->chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
case NL80211_CHAN_WIDTH_20:
@@ -884,28 +873,12 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.radar_required = true;
}
- rcu_read_lock();
- chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
- if (!chanctx_conf) {
- rcu_read_unlock();
- goto disconnect;
- }
-
- /* don't handle for multi-VIF cases */
- chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
- if (chanctx->refcount > 1) {
- rcu_read_unlock();
- goto disconnect;
- }
- num_chanctx = 0;
- list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
- num_chanctx++;
-
- if (num_chanctx > 1) {
- rcu_read_unlock();
- goto disconnect;
+ if (cfg80211_chandef_identical(¶ms.chandef,
+ &sdata->vif.bss_conf.chandef)) {
+ ibss_dbg(sdata,
+ "received csa with an identical chandef, ignoring\n");
+ return true;
}
- rcu_read_unlock();
/* all checks done, now perform the channel switch. */
ibss_dbg(sdata,
@@ -914,19 +887,9 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.block_tx = !!csa_ie.mode;
- ieee80211_ibss_csa_beacon(sdata, ¶ms);
- sdata->csa_radar_required = params.radar_required;
-
- if (params.block_tx)
- ieee80211_stop_queues_by_reason(&sdata->local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
-
- sdata->csa_chandef = params.chandef;
- sdata->vif.csa_active = true;
-
- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, ¶ms.chandef);
+ if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
+ ¶ms))
+ goto disconnect;
ieee80211_ibss_csa_mark_radar(sdata);
@@ -962,7 +925,8 @@ ieee80211_rx_mgmt_spectrum_mgmt(struct ieee80211_sub_if_data *sdata,
if (len < required_len)
return;
- ieee80211_ibss_process_chanswitch(sdata, elems, false);
+ if (!sdata->vif.csa_active)
+ ieee80211_ibss_process_chanswitch(sdata, elems, false);
}
static void ieee80211_rx_mgmt_deauth_ibss(struct ieee80211_sub_if_data *sdata,
@@ -1143,7 +1107,8 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
goto put_bss;
/* process channel switch */
- if (ieee80211_ibss_process_chanswitch(sdata, elems, true))
+ if (sdata->vif.csa_active ||
+ ieee80211_ibss_process_chanswitch(sdata, elems, true))
goto put_bss;
/* same BSSID */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 32bae21..a7b4d55 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1442,6 +1442,8 @@ void ieee80211_handle_roc_started(struct ieee80211_roc_work *roc);
/* channel switch handling */
void ieee80211_csa_finalize_work(struct work_struct *work);
+int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
+ struct cfg80211_csa_settings *params);
/* interface handling */
int ieee80211_iface_init(void);
--
1.8.5.1
T24gRnJpLCAyMDEzLTEyLTEzIGF0IDE4OjIzICswMTAwLCBTaW1vbiBXdW5kZXJsaWNoIHdyb3Rl
Og0KPiBIZXkgTHVjYSwNCj4gDQo+ID4gSGksDQo+ID4gDQo+ID4gTW9yZSBjaGFuZ2VzLCBob3Bl
ZnVsbHkgdGhlIGxhc3Qgb25lcyBmb3IgdGhpcyBzZXJpZXMuIDspDQo+ID4gDQo+ID4gSSBoYXZl
IGhhY2tlZCB3cGFfc3VwcGxpY2FudCB0byBzdXBwb3J0IENTQSBpbiBJQlNTLCBzbyBJIGNvdWxk
IHRlc3QNCj4gPiBpdCB0b28uICBJIGNyZWF0ZWQgc29tZSBtYTgwMjExX2h3c2ltIHRlc3RjYXNl
cyBvbiB0b3Agb2YgdGhpcy4gIFRoZQ0KPiA+IHRlc3RzIG5vdyBwYXNzLCB0aG91Z2ggSSBnZXQg
c29tZSBzZW1pLXJhbmRvbSBXQVJOcyBbMV0gd2hlbiB1c2luZyAzDQo+ID4gc3RhdGlvbnMuICBJ
dCBzZWVtcyB0byBiZSBhIHJhY2UgY29uZGl0aW9uIGFuZCBpdCBkb2Vzbid0IHNlZW0gdG8gYmUN
Cj4gPiByZWxhdGVkIHRvIHRoaXMgc2VyaWVzLCBzbyBJIHRoaW5rIHdlIHNob3VsZCBhZGRyZXNz
IGl0IHNlcGFyYXRlbHkNCj4gPiAoU2ltb24sIGhpbnQhIDspLg0KPiA+IA0KPiA+IEkgaGF2ZSBh
bHNvIGNyZWF0ZWQgc29tZSBiYXNpYyBCU1MgQ1NBIHRlc3QgY2FzZXMgZm9yIG1hYzgwMjExX2h3
c2ltLg0KPiA+IFRoZSB0ZXN0cyBwYXNzLCBleGNlcHQgd2hlbiBJIHRyeSBjb3VudCA8IDIsIHdo
aWNoIGlzIGV4cGVjdGVkLCBzaW5jZQ0KPiA+IHdlIGRpZCBub3QgaW1wbGVtZW50IHRoZSBDU0Eg
YWN0aW9uIGZyYW1lIGluIEJTUyBtb2RlLg0KPiA+IA0KPiA+IEknbGwgdXBzdHJlYW0gdGhlIEJT
UyBDU0EgdGVzdCBjYXNlcyBzb29uLiAgVGhlIElCU1MgdGVzdCBjYXNlcyBzdGlsbA0KPiA+IG5l
ZWQgdG8gYmUgY2xlYW5lZCB1cCAoYWN0dWFsbHkgdGhleSdyZSB3b3J0aGxlc3Mgd2l0aG91dCB0
aGUgSUJTUyBDU0ENCj4gPiBzdXBwb3J0IGluIHdwYV9zdXBwbGljYW50LCB3aGljaCBuZWVkcyB0
byBiZSBjbGVhbmVkIHVwKS4gIElmIHNvbWVvbmUNCj4gPiBpcyBpbnRlcmVzdGVkIGluIHRoaXMs
IGxldCBtZSBrbm93IGFuZCBJIGNhbiBzZW5kIHRoZW0NCj4gPiAid2l0aG91dC13YXJyYW50eSBv
ZiBhbnkga2luZCIuIDspDQo+ID4gDQo+ID4gUGxlYXNlIHJldmlldyBhbmQgdGVzdCENCj4gDQo+
IFBhdGNoZXMgbG9vayBnb29kIHRvIG1lIGFuZCB3b3JrZWQgZmluZSBpbiBteSB0ZXN0IGVudmly
b25tZW50LiBFdmVyeXRoaW5nIGFzIA0KPiBleHBlY3RlZCBmb3IgQ1NBIGNvdW50cyBvZiAwLCAx
LCAyIGFuZCAxMCwgbm8gd2FybmluZ3Mgb3Igd2VpcmQgYmVoYXZpb3VyLiANCj4gSSd2ZSB0ZXN0
ZWQgd2l0aCB0d28gc3RhdGlvbnMgaW4gSUJTUyBtb2RlIHVzaW5nIGF0aDlrLg0KPiANCj4gU28s
IGFwYXJ0IGZyb20gdGhhdCBmdW5jdGlvbiBuYW1lIGlzc3VlLCBmZWVsIGZyZWUgdG8gYWRkIGZv
ciBQYXRjaCAxLDIgYW5kIDU6DQo+IA0KPiBBY2tlZC1ieS9UZXN0ZWQtYnk6IFNpbW9uIFd1bmRl
cmxpY2ggPHN3QHNpbW9ud3VuZGVybGljaC5kZT4NCg0KVGhhbmsgeW91IHZlcnkgbXVjaCBmb3Ig
YWxsIHlvdXIgaGVscCwgU2ltb24hDQoNCg0KDQo+IEknbGwgdHJ5IHRvIHJlcHJvZHVjZSB0aGUg
aXNzdWUgeW91IHdlcmUgc2VlaW5nIHdoZW4gdGhlc2UgdGVzdGNhc2VzL3dwYV9zIA0KPiBpbXBs
ZW1lbnRhdGlvbiBhcmUgdXBzdHJlYW0gKG9yIHBhdGNoZXMgYXJlIGF2YWlsYWJsZSkuIEkgZGlk
bid0IHNlZSB0aGUgDQo+IHByb2JsZW0gb24gYSBmaXJzdCBnbGFuY2UsIGJ1dCB0aGUgd2Fybmlu
ZyBpcyBub3QgY3JpdGljYWwgKGJhc2ljYWxseSB0aGUgQ1NBIA0KPiB3aWxsIGZhaWwpLCBzbyBJ
IGd1ZXNzIGl0J3MgT0sgdG8gZml4IGl0IHNvb25bdG1dLg0KDQpZZWFoLCBsZXQncyBmaXggaXQg
c2VwYXJhdGVseS4gIFByb2JhYmx5IGJlc3QgdG8gd2FpdCB1bnRpbCBhZnRlciB0aGUNCmhvbGlk
YXlzLCBzaW5jZSBJJ20gaGF2aW5nIHZhY2F0aW9ucyBhbHJlYWR5IG5leHQgd2Vlay4NCg0KLS0N
CkNoZWVycywNCkx1Y2EuDQo=
The return value of ieee80211_mesh_csa_beacon is not aligned with the
return value of ieee80211_csa_beacon() and
ieee80211_ibss_csa_beacon(). For consistency and to be able to use
both functions with similar code, change ieee80211_mesh_csa_beacon()
not to send the bss changed notification itself, but return what has
changed so the caller can send the notification instead.
Change-Id: If4c111fd874a3e17a5df98d306f7f1c8fcaa6a1a
Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/mesh.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index d0c4131..f255bd9 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1096,12 +1096,10 @@ int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
return ret;
}
- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
-
if (csa_action)
ieee80211_send_action_csa(sdata, csa_settings);
- return 0;
+ return BSS_CHANGED_BEACON;
}
static int mesh_fwd_csa_frame(struct ieee80211_sub_if_data *sdata,
--
1.8.5.1
> The return value of ieee80211_ibss_csa_beacon is not aligned with the
> return value of ieee80211_csa_beacon(). For consistency and to be
You probably mean "ieee80211_assign_beacon()" here? There is no
ieee80211_csa_beacon(), but AP uses ieee80211_assign_beacon(). Same goes for
the commit message of patch 4. Content looks good.
Cheers,
Simon
Hey Luca,
> Hi,
>
> More changes, hopefully the last ones for this series. ;)
>
> I have hacked wpa_supplicant to support CSA in IBSS, so I could test
> it too. I created some ma80211_hwsim testcases on top of this. The
> tests now pass, though I get some semi-random WARNs [1] when using 3
> stations. It seems to be a race condition and it doesn't seem to be
> related to this series, so I think we should address it separately
> (Simon, hint! ;).
>
> I have also created some basic BSS CSA test cases for mac80211_hwsim.
> The tests pass, except when I try count < 2, which is expected, since
> we did not implement the CSA action frame in BSS mode.
>
> I'll upstream the BSS CSA test cases soon. The IBSS test cases still
> need to be cleaned up (actually they're worthless without the IBSS CSA
> support in wpa_supplicant, which needs to be cleaned up). If someone
> is interested in this, let me know and I can send them
> "without-warranty of any kind". ;)
>
> Please review and test!
Patches look good to me and worked fine in my test environment. Everything as
expected for CSA counts of 0, 1, 2 and 10, no warnings or weird behaviour.
I've tested with two stations in IBSS mode using ath9k.
So, apart from that function name issue, feel free to add for Patch 1,2 and 5:
Acked-by/Tested-by: Simon Wunderlich <[email protected]>
I'll try to reproduce the issue you were seeing when these testcases/wpa_s
implementation are upstream (or patches are available). I didn't see the
problem on a first glance, but the warning is not critical (basically the CSA
will fail), so I guess it's OK to fix it soon[tm].
Thanks for your work!
Simon
T24gRnJpLCAyMDEzLTEyLTEzIGF0IDE4OjEyICswMTAwLCBTaW1vbiBXdW5kZXJsaWNoIHdyb3Rl
Og0KPiA+IFRoZSByZXR1cm4gdmFsdWUgb2YgaWVlZTgwMjExX2lic3NfY3NhX2JlYWNvbiBpcyBu
b3QgYWxpZ25lZCB3aXRoIHRoZQ0KPiA+IHJldHVybiB2YWx1ZSBvZiBpZWVlODAyMTFfY3NhX2Jl
YWNvbigpLiAgRm9yIGNvbnNpc3RlbmN5IGFuZCB0byBiZQ0KPiANCj4gWW91IHByb2JhYmx5IG1l
YW4gImllZWU4MDIxMV9hc3NpZ25fYmVhY29uKCkiIGhlcmU/IFRoZXJlIGlzIG5vIA0KPiBpZWVl
ODAyMTFfY3NhX2JlYWNvbigpLCBidXQgQVAgdXNlcyBpZWVlODAyMTFfYXNzaWduX2JlYWNvbigp
LiBTYW1lIGdvZXMgZm9yIA0KPiB0aGUgY29tbWl0IG1lc3NhZ2Ugb2YgcGF0Y2ggNC4gQ29udGVu
dCBsb29rcyBnb29kLg0KDQpIZWghIEdvb2QgY2F0Y2guICBJJ2xsIGNoYW5nZSBpdC4NCg0KLS0N
Ckx1Y2EuDQo=
The return value of ieee80211_ibss_csa_beacon is not aligned with the
return value of ieee80211_csa_beacon(). For consistency and to be
able to use both functions with similar code, change
ieee80211_ibss_csa_beacon() not to send the bss changed notification
itself, but return what has changed so the caller can send the
notification instead.
Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/ibss.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 0c3a45c..262fd56 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -522,7 +522,7 @@ int ieee80211_ibss_csa_beacon(struct ieee80211_sub_if_data *sdata,
if (csa_settings)
ieee80211_send_action_csa(sdata, csa_settings);
- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
+ return BSS_CHANGED_BEACON;
out:
return ret;
}
@@ -563,6 +563,9 @@ int ieee80211_ibss_finish_csa(struct ieee80211_sub_if_data *sdata)
if (err < 0)
return err;
+ if (err)
+ ieee80211_bss_info_change_notify(sdata, err);
+
return 0;
}
--
1.8.5.1
A beacon should never have a Channel Switch Announcement information
element with a count of 0, because a count of 1 means switch just
before the next beacon. So, if a count of 0 was valid in a beacon, it
would have been transmitted in the next channel already, which is
useless. A CSA count equal to zero is only meaningful in action
frames or probe_responses.
Fix the ieee80211_csa_is_complete() and ieee80211_update_csa()
functions accordingly.
With a CSA count of 0, we won't transmit any CSA beacons, because the
switch will happen before the next TBTT. To avoid extra work and
potential confusion in the drivers, complete the CSA immediately,
instead of waiting for the driver to call ieee80211_csa_finish().
To keep things simpler, we also switch immediately when the CSA count
is 1, while in theory we should delay the switch until just before the
next TBTT.
Additionally, move the ieee80211_csa_finish() function to cfg.c,
where it makes more sense.
Signed-off-by: Luciano Coelho <[email protected]>
---
include/net/mac80211.h | 10 ++--
net/mac80211/cfg.c | 115 +++++++++++++++++++++++++++++++++------------
net/mac80211/ibss.c | 6 ---
net/mac80211/ieee80211_i.h | 3 +-
net/mac80211/mesh.c | 9 ++--
net/mac80211/tx.c | 19 +++-----
6 files changed, 102 insertions(+), 60 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index c014acc..5cc9a79 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2730,11 +2730,13 @@ enum ieee80211_roc_type {
* @channel_switch_beacon: Starts a channel switch to a new channel.
* Beacons are modified to include CSA or ECSA IEs before calling this
* function. The corresponding count fields in these IEs must be
- * decremented, and when they reach zero the driver must call
+ * decremented, and when they reach 1 the driver must call
* ieee80211_csa_finish(). Drivers which use ieee80211_beacon_get()
* get the csa counter decremented by mac80211, but must check if it is
- * zero using ieee80211_csa_is_complete() after the beacon has been
+ * 1 using ieee80211_csa_is_complete() after the beacon has been
* transmitted and then call ieee80211_csa_finish().
+ * If the CSA count starts as zero or 1, this function will not be called,
+ * since there won't be any time to beacon before the switch anyway.
*
* @join_ibss: Join an IBSS (on an IBSS interface); this is called after all
* information in bss_conf is set up and the beacon can be retrieved. A
@@ -3429,13 +3431,13 @@ static inline struct sk_buff *ieee80211_beacon_get(struct ieee80211_hw *hw,
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
*
* After a channel switch announcement was scheduled and the counter in this
- * announcement hit zero, this function must be called by the driver to
+ * announcement hits 1, this function must be called by the driver to
* notify mac80211 that the channel can be changed.
*/
void ieee80211_csa_finish(struct ieee80211_vif *vif);
/**
- * ieee80211_csa_is_complete - find out if counters reached zero
+ * ieee80211_csa_is_complete - find out if counters reached 1
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
*
* This function returns whether the channel switch counters reached zero.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 4e98c49..ab9587f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2982,26 +2982,24 @@ cfg80211_beacon_dup(struct cfg80211_beacon_data *beacon)
return new_beacon;
}
-void ieee80211_csa_finalize_work(struct work_struct *work)
+void ieee80211_csa_finish(struct ieee80211_vif *vif)
{
- 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;
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
- sdata_lock(sdata);
- /* AP might have been stopped while waiting for the lock. */
- if (!sdata->vif.csa_active)
- goto unlock;
+ ieee80211_queue_work(&sdata->local->hw,
+ &sdata->csa_finalize_work);
+}
+EXPORT_SYMBOL(ieee80211_csa_finish);
- if (!ieee80211_sdata_running(sdata))
- goto unlock;
+static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_local *local = sdata->local;
+ int err, changed = 0;
sdata->radar_required = sdata->csa_radar_required;
err = ieee80211_vif_change_channel(sdata, &changed);
if (WARN_ON(err < 0))
- goto unlock;
+ return;
if (!local->use_chanctx) {
local->_oper_chandef = sdata->csa_chandef;
@@ -3015,7 +3013,7 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
case NL80211_IFTYPE_AP:
err = ieee80211_assign_beacon(sdata, sdata->u.ap.next_beacon);
if (err < 0)
- goto unlock;
+ return;
changed |= err;
kfree(sdata->u.ap.next_beacon);
@@ -3030,12 +3028,12 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
case NL80211_IFTYPE_MESH_POINT:
err = ieee80211_mesh_finish_csa(sdata);
if (err < 0)
- goto unlock;
+ return;
break;
#endif
default:
WARN_ON(1);
- goto unlock;
+ return;
}
ieee80211_wake_queues_by_reason(&sdata->local->hw,
@@ -3043,6 +3041,23 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
IEEE80211_QUEUE_STOP_REASON_CSA);
cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
+}
+
+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);
+
+ sdata_lock(sdata);
+ /* AP might have been stopped while waiting for the lock. */
+ if (!sdata->vif.csa_active)
+ goto unlock;
+
+ if (!ieee80211_sdata_running(sdata))
+ goto unlock;
+
+ ieee80211_csa_finalize(sdata);
unlock:
sdata_unlock(sdata);
@@ -3056,7 +3071,7 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_chanctx_conf *chanctx_conf;
struct ieee80211_chanctx *chanctx;
struct ieee80211_if_mesh __maybe_unused *ifmsh;
- int err, num_chanctx;
+ int err, num_chanctx, changed = 0;
lockdep_assert_held(&sdata->wdev.mtx);
@@ -3097,19 +3112,40 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP:
- sdata->csa_counter_offset_beacon =
- params->counter_offset_beacon;
- sdata->csa_counter_offset_presp = params->counter_offset_presp;
sdata->u.ap.next_beacon =
cfg80211_beacon_dup(¶ms->beacon_after);
if (!sdata->u.ap.next_beacon)
return -ENOMEM;
+ /*
+ * With a count of 0, we don't have to wait for any
+ * TBTT before switching, so complete the CSA
+ * immediately. In theory, with a count == 1 we
+ * should delay the switch until just before the next
+ * TBTT, but that would complicate things so we switch
+ * immediately too. If we would delay the switch
+ * until the next TBTT, we would have to set the probe
+ * response here.
+ *
+ * TODO: A channel switch with count <= 1 without
+ * sending a CSA action frame is kind of useless,
+ * because the clients won't know we're changing
+ * channels. The action frame must be implemented
+ * either here or in the userspace.
+ */
+ if (params->count <= 1)
+ break;
+
+ sdata->csa_counter_offset_beacon =
+ params->counter_offset_beacon;
+ sdata->csa_counter_offset_presp = params->counter_offset_presp;
err = ieee80211_assign_beacon(sdata, ¶ms->beacon_csa);
if (err < 0) {
kfree(sdata->u.ap.next_beacon);
return err;
}
+ changed |= err;
+
break;
case NL80211_IFTYPE_ADHOC:
if (!sdata->vif.bss_conf.ibss_joined)
@@ -3137,9 +3173,16 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
params->chandef.chan->band)
return -EINVAL;
- err = ieee80211_ibss_csa_beacon(sdata, params);
- if (err < 0)
- return err;
+ /* see comments in the NL80211_IFTYPE_AP block */
+ if (params->count > 1) {
+ err = ieee80211_ibss_csa_beacon(sdata, params);
+ if (err < 0)
+ return err;
+ changed |= err;
+ }
+
+ ieee80211_send_action_csa(sdata, params);
+
break;
#ifdef CONFIG_MAC80211_MESH
case NL80211_IFTYPE_MESH_POINT:
@@ -3159,12 +3202,19 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
- err = ieee80211_mesh_csa_beacon(sdata, params,
- (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
- if (err < 0) {
- ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
- return err;
+ /* see comments in the NL80211_IFTYPE_AP block */
+ if (params->count > 1) {
+ err = ieee80211_mesh_csa_beacon(sdata, params);
+ if (err < 0) {
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
+ return err;
+ }
+ changed |= err;
}
+
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT)
+ ieee80211_send_action_csa(sdata, params);
+
break;
#endif
default:
@@ -3181,8 +3231,13 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
sdata->csa_chandef = params->chandef;
sdata->vif.csa_active = true;
- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, ¶ms->chandef);
+ if (changed) {
+ ieee80211_bss_info_change_notify(sdata, changed);
+ drv_channel_switch_beacon(sdata, ¶ms->chandef);
+ } else {
+ /* if the beacon didn't change, we can finalize immediately */
+ ieee80211_csa_finalize(sdata);
+ }
return 0;
}
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 262fd56..3f643ea 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -516,12 +516,6 @@ int ieee80211_ibss_csa_beacon(struct ieee80211_sub_if_data *sdata,
if (old_presp)
kfree_rcu(old_presp, rcu_head);
- /* it might not send the beacon for a while. send an action frame
- * immediately to announce the channel switch.
- */
- if (csa_settings)
- ieee80211_send_action_csa(sdata, csa_settings);
-
return BSS_CHANGED_BEACON;
out:
return ret;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index cf38a74..9b5f1bc 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1400,8 +1400,7 @@ void ieee80211_mesh_work(struct ieee80211_sub_if_data *sdata);
void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb);
int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
- struct cfg80211_csa_settings *csa_settings,
- bool csa_action);
+ struct cfg80211_csa_settings *csa_settings);
int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata);
/* scan/BSS handling */
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index f255bd9..dbf23eb 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1058,7 +1058,8 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
/* Remove the CSA and MCSP elements from the beacon */
tmp_csa_settings = rcu_dereference(ifmsh->csa);
rcu_assign_pointer(ifmsh->csa, NULL);
- kfree_rcu(tmp_csa_settings, rcu_head);
+ if (tmp_csa_settings)
+ kfree_rcu(tmp_csa_settings, rcu_head);
ret = ieee80211_mesh_rebuild_beacon(sdata);
if (ret)
return -EINVAL;
@@ -1071,8 +1072,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
}
int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
- struct cfg80211_csa_settings *csa_settings,
- bool csa_action)
+ struct cfg80211_csa_settings *csa_settings)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
struct mesh_csa_settings *tmp_csa_settings;
@@ -1096,9 +1096,6 @@ int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
return ret;
}
- if (csa_action)
- ieee80211_send_action_csa(sdata, csa_settings);
-
return BSS_CHANGED_BEACON;
}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6d59e21..b86a679 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2397,15 +2397,6 @@ static int ieee80211_beacon_add_tim(struct ieee80211_sub_if_data *sdata,
return 0;
}
-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);
-}
-EXPORT_SYMBOL(ieee80211_csa_finish);
-
static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata,
struct beacon_data *beacon)
{
@@ -2434,8 +2425,12 @@ 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))
+ /* Warn if the driver did not check for/react to csa
+ * completeness. A beacon with CSA counter set to 0 should
+ * never occur, because a counter of 1 means switch just
+ * before the next beacon.
+ */
+ if (WARN_ON(beacon_data[counter_offset_beacon] == 1))
return;
beacon_data[counter_offset_beacon]--;
@@ -2501,7 +2496,7 @@ bool ieee80211_csa_is_complete(struct ieee80211_vif *vif)
if (WARN_ON(counter_beacon > beacon_data_len))
goto out;
- if (beacon_data[counter_beacon] == 0)
+ if (beacon_data[counter_beacon] == 1)
ret = true;
out:
rcu_read_unlock();
--
1.8.5.1
On Fri, 2013-12-13 at 13:54 +0200, Luciano Coelho wrote:
> Hi,
>
> More changes, hopefully the last ones for this series. ;)
>
> I have hacked wpa_supplicant to support CSA in IBSS, so I could test
> it too. I created some ma80211_hwsim testcases on top of this. The
> tests now pass, though I get some semi-random WARNs [1] when using 3
> stations. It seems to be a race condition and it doesn't seem to be
> related to this series, so I think we should address it separately
> (Simon, hint! ;).
Forgot the reference. Here it is:
[1] Sporadic warning during IBSS CSA:
[ 65.601056] ------------[ cut here ]------------
[ 65.601057] WARNING: CPU: 0 PID: 6 at net/mac80211/ibss.c:491 ieee80211_ibss_csa_beacon+0x30f/0x320()
[ 65.601058] Modules linked in:
[ 65.601059] CPU: 0 PID: 6 Comm: kworker/u8:0 Tainted: G W 3.12.0-rc1+ #76
[ 65.601060] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 65.601061] Workqueue: phy2 ieee80211_iface_work
[ 65.601063] 0000000000000009 ffff88000696b950 ffffffff817a035c 0000000000000000
[ 65.601065] ffff88000696b988 ffffffff810649c8 ffff880000014b00 00000000ffffffea
[ 65.601066] 0000000000000000 ffff88000696ba68 ffff880005e8ca00 ffff88000696b998
[ 65.601067] Call Trace:
[ 65.601069] [<ffffffff817a035c>] dump_stack+0x54/0x74
[ 65.601070] [<ffffffff810649c8>] warn_slowpath_common+0x78/0xa0
[ 65.601072] [<ffffffff81064aa5>] warn_slowpath_null+0x15/0x20
[ 65.601073] [<ffffffff8171c0cf>] ieee80211_ibss_csa_beacon+0x30f/0x320
[ 65.601075] [<ffffffff81730680>] ieee80211_channel_switch+0x620/0x870
[ 65.601077] [<ffffffff81730149>] ? ieee80211_channel_switch+0xe9/0x870
[ 65.601079] [<ffffffff81718bec>] ieee80211_ibss_process_chanswitch+0x2ec/0x340
[ 65.601080] [<ffffffff8171c9e7>] ieee80211_ibss_rx_queued_mgmt+0x477/0x8d0
[ 65.601082] [<ffffffff810d6048>] ? __lock_acquire+0x408/0x1d50
[ 65.601084] [<ffffffff8171e434>] ieee80211_iface_work+0x274/0x360
[ 65.601085] [<ffffffff81086e61>] process_one_work+0x201/0x6b0
[ 65.601087] [<ffffffff81086df6>] ? process_one_work+0x196/0x6b0
[ 65.601088] [<ffffffff81087429>] worker_thread+0x119/0x3a0
[ 65.601090] [<ffffffff81087310>] ? process_one_work+0x6b0/0x6b0
[ 65.601091] [<ffffffff8108f358>] kthread+0xe8/0xf0
[ 65.601094] [<ffffffff8108f270>] ? insert_kthread_work+0x70/0x70
[ 65.601095] [<ffffffff817b29cc>] ret_from_fork+0x7c/0xb0
[ 65.601097] [<ffffffff8108f270>] ? insert_kthread_work+0x70/0x70
[ 65.601098] ---[ end trace 5ff67f51ef1f40bb ]---
Refactor ieee80211_mesh_process_chanswitch() to use
ieee80211_channel_switch() and avoid code duplication.
Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/cfg.c | 10 +++++--
net/mac80211/ieee80211_i.h | 6 +++-
net/mac80211/mesh.c | 73 +++++++++++++---------------------------------
net/mac80211/util.c | 1 -
4 files changed, 33 insertions(+), 57 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 96aebe5..4e98c49 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3156,9 +3156,15 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
params->chandef.chan->band)
return -EINVAL;
- err = ieee80211_mesh_csa_beacon(sdata, params, true);
- if (err < 0)
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
+
+ err = ieee80211_mesh_csa_beacon(sdata, params,
+ (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
+ if (err < 0) {
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
return err;
+ }
break;
#endif
default:
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a7b4d55..cf38a74 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -611,7 +611,11 @@ struct ieee80211_if_mesh {
struct ps_data ps;
/* Channel Switching Support */
struct mesh_csa_settings __rcu *csa;
- bool chsw_init;
+ enum {
+ IEEE80211_MESH_CSA_ROLE_NONE,
+ IEEE80211_MESH_CSA_ROLE_INIT,
+ IEEE80211_MESH_CSA_ROLE_REPEATER,
+ } csa_role;
u8 chsw_ttl;
u16 pre_value;
};
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 330d1f7..d0c4131 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -685,7 +685,7 @@ ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
*pos++ = csa->settings.count;
*pos++ = WLAN_EID_CHAN_SWITCH_PARAM;
*pos++ = 6;
- if (ifmsh->chsw_init) {
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT) {
*pos++ = ifmsh->mshcfg.dot11MeshTTL;
*pos |= WLAN_EID_CHAN_SWITCH_PARAM_INITIATOR;
} else {
@@ -853,19 +853,11 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
{
struct cfg80211_csa_settings params;
struct ieee80211_csa_ie csa_ie;
- struct ieee80211_chanctx_conf *chanctx_conf;
- struct ieee80211_chanctx *chanctx;
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
enum ieee80211_band band = ieee80211_get_sdata_band(sdata);
- int err, num_chanctx;
+ int err;
u32 sta_flags;
- if (sdata->vif.csa_active)
- return true;
-
- if (!ifmsh->mesh_id)
- return false;
-
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
@@ -890,10 +882,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
params.chandef = csa_ie.chandef;
params.count = csa_ie.count;
- if (sdata->vif.bss_conf.chandef.chan->band !=
- params.chandef.chan->band)
- return false;
-
if (!cfg80211_chandef_usable(sdata->local->hw.wiphy, ¶ms.chandef,
IEEE80211_CHAN_DISABLED)) {
sdata_info(sdata,
@@ -916,24 +904,12 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
return false;
}
- rcu_read_lock();
- chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
- if (!chanctx_conf)
- goto failed_chswitch;
-
- /* don't handle for multi-VIF cases */
- chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
- if (chanctx->refcount > 1)
- goto failed_chswitch;
-
- num_chanctx = 0;
- list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
- num_chanctx++;
-
- if (num_chanctx > 1)
- goto failed_chswitch;
-
- rcu_read_unlock();
+ if (cfg80211_chandef_identical(¶ms.chandef,
+ &sdata->vif.bss_conf.chandef)) {
+ ibss_dbg(sdata,
+ "received csa with an identical chandef, ignoring\n");
+ return true;
+ }
mcsa_dbg(sdata,
"received channel switch announcement to go to channel %d MHz\n",
@@ -945,27 +921,16 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
else
ifmsh->chsw_ttl = 0;
- if (ifmsh->chsw_ttl > 0)
- if (ieee80211_mesh_csa_beacon(sdata, ¶ms, false) < 0)
- return false;
-
- sdata->csa_radar_required = params.radar_required;
-
- if (params.block_tx)
- ieee80211_stop_queues_by_reason(&sdata->local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ if (ifmsh->chsw_ttl == 0)
+ return false;
- sdata->csa_chandef = params.chandef;
- sdata->vif.csa_active = true;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_REPEATER;
- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, ¶ms.chandef);
+ if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
+ ¶ms) < 0)
+ return false;
return true;
-failed_chswitch:
- rcu_read_unlock();
- return false;
}
static void
@@ -1075,7 +1040,8 @@ 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);
- if (!ifmsh->chsw_init)
+ if (ifmsh->csa_role != IEEE80211_MESH_CSA_ROLE_INIT &&
+ !sdata->vif.csa_active)
ieee80211_mesh_process_chnswitch(sdata, &elems, true);
}
@@ -1086,7 +1052,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
int ret = 0;
/* Reset the TTL value and Initiator flag */
- ifmsh->chsw_init = false;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
ifmsh->chsw_ttl = 0;
/* Remove the CSA and MCSP elements from the beacon */
@@ -1200,7 +1166,8 @@ 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)) {
+ if (!sdata->vif.csa_active &&
+ !ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
mcsa_dbg(sdata, "Failed to process CSA action frame");
return;
}
@@ -1355,7 +1322,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
mesh_rmc_init(sdata);
ifmsh->last_preq = jiffies;
ifmsh->next_perr = jiffies;
- ifmsh->chsw_init = false;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
/* Allocate all mesh structures when creating the first mesh interface. */
if (!mesh_allocated)
ieee80211s_init();
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 4a376a7..0dd7911 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2498,7 +2498,6 @@ int ieee80211_send_action_csa(struct ieee80211_sub_if_data *sdata,
ifmsh->pre_value++;
put_unaligned_le16(ifmsh->pre_value, pos);/* Precedence Value */
pos += 2;
- ifmsh->chsw_init = true;
}
ieee80211_tx_skb(sdata, skb);
--
1.8.5.1