2014-05-02 13:41:14

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

From: Luciano Coelho <[email protected]>

Add a new NL80211_CH_SWITCH_STARTED_NOTIFY message that can be sent to
the userspace when a channel switch process has started. This allows
userspace to take action, for instance, by requesting other interfaces
to switch channel as necessary.

This patch introduces a function that allows the drivers to send this
notification. It should be used when the driver starts processing a
channel switch initiated by a remote device (eg. when a STA receives a
CSA from the AP) and when it successfully starts a userspace-triggered
channel switch (eg. when hostapd triggers a channel swith in the AP).

Additionally, the corresponding notification and function for failure
cases were added as well.

Signed-off-by: Luciano Coelho <[email protected]>
---
include/net/cfg80211.h | 23 +++++++++++++++++++++++
include/uapi/linux/nl80211.h | 17 +++++++++++++++++
net/wireless/nl80211.c | 36 +++++++++++++++++++++++++++++++++---
net/wireless/trace.h | 32 ++++++++++++++++++++++++++++++++
4 files changed, 105 insertions(+), 3 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7eae46c..43c674e 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4574,6 +4574,29 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
void cfg80211_ch_switch_notify(struct net_device *dev,
struct cfg80211_chan_def *chandef);

+/*
+ * cfg80211_ch_switch_started_notify - notify channel switch start
+ * @dev: the device on which the channel switch started
+ * @chandef: the future channel definition
+ *
+ * Inform the userspace about the channel switch that has just
+ * started, so that it can take appropriate actions (eg. starting
+ * channel switch on other vifs), if necessary.
+ */
+void cfg80211_ch_switch_started_notify(struct net_device *dev,
+ struct cfg80211_chan_def *chandef);
+
+/*
+ * cfg80211_ch_switch_failed_notify - notify channel switch failure
+ * @dev: the device on which the channel switch has failed
+ * @chandef: the channel definition to which we failed to switch
+ *
+ * Inform the userspace that a channel switch has failed after the
+ * channel switch started notification has been sent.
+ */
+void cfg80211_ch_switch_failed_notify(struct net_device *dev,
+ struct cfg80211_chan_def *chandef);
+
/**
* ieee80211_operating_class_to_band - convert operating class to band
*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 406010d..d7c5b61 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -638,6 +638,20 @@
* %NL80211_ATTR_IFINDEX is now on %NL80211_ATTR_WIPHY_FREQ and the
* attributes determining channel width.
*
+ * @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY: Notify that a channel switch
+ * has been started on an interface, regardless of the initiator
+ * (ie. whether it was requested from a remote device or
+ * initiated on our own). It indicates that
+ * %NL80211_ATTR_IFINDEX will be on %NL80211_ATTR_WIPHY_FREQ
+ * after %NL80211_ATTR_CH_SWITCH_COUNT TBTT's. The userspace may
+ * decide to react to this indication by requesting other
+ * interfaces to change channel as well.
+ *
+ * @NL80211_CMD_CH_SWITCH_FAILED_NOTIFY: Notify that a channel switch
+ * has failed on %NL80211_ATTR_IFINDEX. This notification can
+ * only be sent after a @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY has
+ * been issued.
+ *
* @NL80211_CMD_START_P2P_DEVICE: Start the given P2P Device, identified by
* its %NL80211_ATTR_WDEV identifier. It must have been created with
* %NL80211_CMD_NEW_INTERFACE previously. After it has been started, the
@@ -890,6 +904,9 @@ enum nl80211_commands {

NL80211_CMD_SET_QOS_MAP,

+ NL80211_CMD_CH_SWITCH_STARTED_NOTIFY,
+ NL80211_CMD_CH_SWITCH_FAILED_NOTIFY,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0f1b18f2..804c3c9 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -11208,7 +11208,8 @@ EXPORT_SYMBOL(cfg80211_pmksa_candidate_notify);
static void nl80211_ch_switch_notify(struct cfg80211_registered_device *rdev,
struct net_device *netdev,
struct cfg80211_chan_def *chandef,
- gfp_t gfp)
+ gfp_t gfp,
+ enum nl80211_commands notif)
{
struct sk_buff *msg;
void *hdr;
@@ -11217,7 +11218,7 @@ static void nl80211_ch_switch_notify(struct cfg80211_registered_device *rdev,
if (!msg)
return;

- hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_CH_SWITCH_NOTIFY);
+ hdr = nl80211hdr_put(msg, 0, 0, 0, notif);
if (!hdr) {
nlmsg_free(msg);
return;
@@ -11259,10 +11260,39 @@ void cfg80211_ch_switch_notify(struct net_device *dev,

wdev->chandef = *chandef;
wdev->preset_chandef = *chandef;
- nl80211_ch_switch_notify(rdev, dev, chandef, GFP_KERNEL);
+ nl80211_ch_switch_notify(rdev, dev, chandef, GFP_KERNEL,
+ NL80211_CMD_CH_SWITCH_NOTIFY);
}
EXPORT_SYMBOL(cfg80211_ch_switch_notify);

+void cfg80211_ch_switch_started_notify(struct net_device *dev,
+ struct cfg80211_chan_def *chandef)
+{
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ struct wiphy *wiphy = wdev->wiphy;
+ struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+
+ trace_cfg80211_ch_switch_started_notify(dev, chandef);
+
+ nl80211_ch_switch_notify(rdev, dev, chandef, GFP_KERNEL,
+ NL80211_CMD_CH_SWITCH_STARTED_NOTIFY);
+}
+EXPORT_SYMBOL(cfg80211_ch_switch_started_notify);
+
+void cfg80211_ch_switch_failed_notify(struct net_device *dev,
+ struct cfg80211_chan_def *chandef)
+{
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ struct wiphy *wiphy = wdev->wiphy;
+ struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+
+ trace_cfg80211_ch_switch_failed_notify(dev, chandef);
+
+ nl80211_ch_switch_notify(rdev, dev, chandef, GFP_KERNEL,
+ NL80211_CMD_CH_SWITCH_FAILED_NOTIFY);
+}
+EXPORT_SYMBOL(cfg80211_ch_switch_failed_notify);
+
void cfg80211_cqm_txe_notify(struct net_device *dev,
const u8 *peer, u32 num_packets,
u32 rate, u32 intvl, gfp_t gfp)
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index f3c13ff..1f6443b 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2259,6 +2259,38 @@ TRACE_EVENT(cfg80211_ch_switch_notify,
NETDEV_PR_ARG, CHAN_DEF_PR_ARG)
);

+TRACE_EVENT(cfg80211_ch_switch_started_notify,
+ TP_PROTO(struct net_device *netdev,
+ struct cfg80211_chan_def *chandef),
+ TP_ARGS(netdev, chandef),
+ TP_STRUCT__entry(
+ NETDEV_ENTRY
+ CHAN_DEF_ENTRY
+ ),
+ TP_fast_assign(
+ NETDEV_ASSIGN;
+ CHAN_DEF_ASSIGN(chandef);
+ ),
+ TP_printk(NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT,
+ NETDEV_PR_ARG, CHAN_DEF_PR_ARG)
+);
+
+TRACE_EVENT(cfg80211_ch_switch_failed_notify,
+ TP_PROTO(struct net_device *netdev,
+ struct cfg80211_chan_def *chandef),
+ TP_ARGS(netdev, chandef),
+ TP_STRUCT__entry(
+ NETDEV_ENTRY
+ CHAN_DEF_ENTRY
+ ),
+ TP_fast_assign(
+ NETDEV_ASSIGN;
+ CHAN_DEF_ASSIGN(chandef);
+ ),
+ TP_printk(NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT,
+ NETDEV_PR_ARG, CHAN_DEF_PR_ARG)
+);
+
TRACE_EVENT(cfg80211_radar_event,
TP_PROTO(struct wiphy *wiphy, struct cfg80211_chan_def *chandef),
TP_ARGS(wiphy, chandef),
--
1.9.2



2014-05-07 06:39:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

On Wed, 2014-05-07 at 06:08 +0000, Peer, Ilan wrote:

> > I'm not really sure why you have this - it seems that in this case the interface
> > will be stopped so you'll get a STOP_AP or DISCONNECT or whatever other
> > notification. This may not actually be completely true right now (I seem to
> > remember a fix in this area but can't seem to find it in my tree!!) but we'll
> > have to fix that part anyway. Not really sure why then that couldn't be used
> > instead of this notification? The userspace code is going to have to worry
> > about that anyway, I'd think.
> >
>
> At least for AP/GO, there are cases that a failed channel switch
> should not necessarily trigger a STOP_AP flow. For example, in case
> that multi-channel is supported, wpa_supplicant might request a
> channel switch to try to switch an AP/GO interface to a channel used
> by a station interface (to avoid multi channel operation), but in case
> that such a transition fails, it is still a valid to continue to use
> current channel and do not stop the GO. In such a case, it would be
> useful to get a failure notification. Generally, I think that since
> hostapd/wpa_supplicant have started the channel switch, it might be
> better to have them handle a notification and take the appropriate
> action (if needed stop the AP/GO ...).

I don't understand this - there are two ways the channel switch can
fail:

* the request fails
this just returns an error code to userspace during the request,
there's no
notification even with these patches
* the request succeeds, but real switch fails
we beacon with the CSA, counting it down etc., but when it comes to
actually
switching, the switch fails. This should hopefully be rather
unlikely, but if
it really happens the only thing we can do to recover (and actually
do now
after Michal's patches) is to STOP_AP

In the first case, there's no notification, nor any need for it. In the
second case, the scenario you suggest doesn't apply, and STOP_AP has to
happen anyway because the state is completely messed up, clients will be
in the process of switching etc.

> Other than that, a STOP_AP might introduce some races, as
> wpa_supplicant/hostap will not know if the stop_ap was due to the
> failed CS or due to some other reason.

I don't see why that would matter - even if the STOP_AP *was* for some
other reason, but happened in the middle of the CS flow, the reaction
would presumably be the same?

johannes


2014-05-07 10:28:35

by Ilan Peer

[permalink] [raw]
Subject: RE: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

PiANCj4gPiA+IEknbSBub3QgcmVhbGx5IHN1cmUgd2h5IHlvdSBoYXZlIHRoaXMgLSBpdCBzZWVt
cyB0aGF0IGluIHRoaXMgY2FzZQ0KPiA+ID4gdGhlIGludGVyZmFjZSB3aWxsIGJlIHN0b3BwZWQg
c28geW91J2xsIGdldCBhIFNUT1BfQVAgb3IgRElTQ09OTkVDVA0KPiA+ID4gb3Igd2hhdGV2ZXIg
b3RoZXIgbm90aWZpY2F0aW9uLiBUaGlzIG1heSBub3QgYWN0dWFsbHkgYmUgY29tcGxldGVseQ0K
PiA+ID4gdHJ1ZSByaWdodCBub3cgKEkgc2VlbSB0byByZW1lbWJlciBhIGZpeCBpbiB0aGlzIGFy
ZWEgYnV0IGNhbid0IHNlZW0NCj4gPiA+IHRvIGZpbmQgaXQgaW4gbXkgdHJlZSEhKSBidXQgd2Un
bGwgaGF2ZSB0byBmaXggdGhhdCBwYXJ0IGFueXdheS4gTm90DQo+ID4gPiByZWFsbHkgc3VyZSB3
aHkgdGhlbiB0aGF0IGNvdWxkbid0IGJlIHVzZWQgaW5zdGVhZCBvZiB0aGlzDQo+ID4gPiBub3Rp
ZmljYXRpb24/IFRoZSB1c2Vyc3BhY2UgY29kZSBpcyBnb2luZyB0byBoYXZlIHRvIHdvcnJ5IGFi
b3V0IHRoYXQNCj4gYW55d2F5LCBJJ2QgdGhpbmsuDQo+ID4gPg0KPiA+DQo+ID4gQXQgbGVhc3Qg
Zm9yIEFQL0dPLCB0aGVyZSBhcmUgY2FzZXMgdGhhdCBhIGZhaWxlZCBjaGFubmVsIHN3aXRjaA0K
PiA+IHNob3VsZCBub3QgbmVjZXNzYXJpbHkgdHJpZ2dlciBhIFNUT1BfQVAgZmxvdy4gRm9yIGV4
YW1wbGUsIGluIGNhc2UNCj4gPiB0aGF0IG11bHRpLWNoYW5uZWwgaXMgc3VwcG9ydGVkLCB3cGFf
c3VwcGxpY2FudCBtaWdodCByZXF1ZXN0IGENCj4gPiBjaGFubmVsIHN3aXRjaCB0byB0cnkgdG8g
c3dpdGNoIGFuIEFQL0dPIGludGVyZmFjZSB0byBhIGNoYW5uZWwgdXNlZA0KPiA+IGJ5IGEgc3Rh
dGlvbiBpbnRlcmZhY2UgKHRvIGF2b2lkIG11bHRpIGNoYW5uZWwgb3BlcmF0aW9uKSwgYnV0IGlu
IGNhc2UNCj4gPiB0aGF0IHN1Y2ggYSB0cmFuc2l0aW9uIGZhaWxzLCBpdCBpcyBzdGlsbCBhIHZh
bGlkIHRvIGNvbnRpbnVlIHRvIHVzZQ0KPiA+IGN1cnJlbnQgY2hhbm5lbCBhbmQgZG8gbm90IHN0
b3AgdGhlIEdPLiBJbiBzdWNoIGEgY2FzZSwgaXQgd291bGQgYmUNCj4gPiB1c2VmdWwgdG8gZ2V0
IGEgZmFpbHVyZSBub3RpZmljYXRpb24uIEdlbmVyYWxseSwgSSB0aGluayB0aGF0IHNpbmNlDQo+
ID4gaG9zdGFwZC93cGFfc3VwcGxpY2FudCBoYXZlIHN0YXJ0ZWQgdGhlIGNoYW5uZWwgc3dpdGNo
LCBpdCBtaWdodCBiZQ0KPiA+IGJldHRlciB0byBoYXZlIHRoZW0gaGFuZGxlIGEgbm90aWZpY2F0
aW9uIGFuZCB0YWtlIHRoZSBhcHByb3ByaWF0ZQ0KPiA+IGFjdGlvbiAoaWYgbmVlZGVkIHN0b3Ag
dGhlIEFQL0dPIC4uLikuDQo+IA0KPiBJIGRvbid0IHVuZGVyc3RhbmQgdGhpcyAtIHRoZXJlIGFy
ZSB0d28gd2F5cyB0aGUgY2hhbm5lbCBzd2l0Y2ggY2FuDQo+IGZhaWw6DQo+IA0KPiAgKiB0aGUg
cmVxdWVzdCBmYWlscw0KPiAgICB0aGlzIGp1c3QgcmV0dXJucyBhbiBlcnJvciBjb2RlIHRvIHVz
ZXJzcGFjZSBkdXJpbmcgdGhlIHJlcXVlc3QsIHRoZXJlJ3Mgbm8NCj4gICAgbm90aWZpY2F0aW9u
IGV2ZW4gd2l0aCB0aGVzZSBwYXRjaGVzDQo+ICAqIHRoZSByZXF1ZXN0IHN1Y2NlZWRzLCBidXQg
cmVhbCBzd2l0Y2ggZmFpbHMNCj4gICAgd2UgYmVhY29uIHdpdGggdGhlIENTQSwgY291bnRpbmcg
aXQgZG93biBldGMuLCBidXQgd2hlbiBpdCBjb21lcyB0bw0KPiBhY3R1YWxseQ0KPiAgICBzd2l0
Y2hpbmcsIHRoZSBzd2l0Y2ggZmFpbHMuIFRoaXMgc2hvdWxkIGhvcGVmdWxseSBiZSByYXRoZXIg
dW5saWtlbHksIGJ1dCBpZg0KPiAgICBpdCByZWFsbHkgaGFwcGVucyB0aGUgb25seSB0aGluZyB3
ZSBjYW4gZG8gdG8gcmVjb3ZlciAoYW5kIGFjdHVhbGx5IGRvIG5vdw0KPiAgICBhZnRlciBNaWNo
YWwncyBwYXRjaGVzKSBpcyB0byBTVE9QX0FQDQo+IA0KDQpZZXAsIGFncmVlIHRoYXQgdGhlIG9u
bHkgdmFsaWQgb3B0aW9uIGhlcmUgaXMgdG8gc3RvcCB0aGUgQVAuDQoNCj4gSW4gdGhlIGZpcnN0
IGNhc2UsIHRoZXJlJ3Mgbm8gbm90aWZpY2F0aW9uLCBub3IgYW55IG5lZWQgZm9yIGl0LiBJbiB0
aGUgc2Vjb25kDQo+IGNhc2UsIHRoZSBzY2VuYXJpbyB5b3Ugc3VnZ2VzdCBkb2Vzbid0IGFwcGx5
LCBhbmQgU1RPUF9BUCBoYXMgdG8gaGFwcGVuDQo+IGFueXdheSBiZWNhdXNlIHRoZSBzdGF0ZSBp
cyBjb21wbGV0ZWx5IG1lc3NlZCB1cCwgY2xpZW50cyB3aWxsIGJlIGluIHRoZQ0KPiBwcm9jZXNz
IG9mIHN3aXRjaGluZyBldGMuDQo+IA0KPiA+IE90aGVyIHRoYW4gdGhhdCwgYSBTVE9QX0FQIG1p
Z2h0IGludHJvZHVjZSBzb21lIHJhY2VzLCBhcw0KPiA+IHdwYV9zdXBwbGljYW50L2hvc3RhcCB3
aWxsIG5vdCBrbm93IGlmIHRoZSBzdG9wX2FwIHdhcyBkdWUgdG8gdGhlDQo+ID4gZmFpbGVkIENT
IG9yIGR1ZSB0byBzb21lIG90aGVyIHJlYXNvbi4NCj4gDQo+IEkgZG9uJ3Qgc2VlIHdoeSB0aGF0
IHdvdWxkIG1hdHRlciAtIGV2ZW4gaWYgdGhlIFNUT1BfQVAgKndhcyogZm9yIHNvbWUNCj4gb3Ro
ZXIgcmVhc29uLCBidXQgaGFwcGVuZWQgaW4gdGhlIG1pZGRsZSBvZiB0aGUgQ1MgZmxvdywgdGhl
IHJlYWN0aW9uIHdvdWxkDQo+IHByZXN1bWFibHkgYmUgdGhlIHNhbWU/DQo+IA0KDQpJdCBjYW4g
YmUgYmVuZWZpY2lhbCB0byBrbm93IHRoYXQgdGhlIFNUT1BfQVAgd2FzIGNhbGxlZCBkdWUgdG8g
ZmFpbHVyZSBvZiBDUywgYXMgd3BhX3N1cHBsaWNhbnQvaG9zdGFwIGNhbiB0ZWFyIGRvd24gdGhl
IEFQIGFuZCB0aGVuIChpZiBwb3NzaWJsZSkgc2V0IGl0IHVwIGFnYWluIG9uIHRoZSBuZXcgY2hh
bm5lbCBvciBhbm90aGVyIGNoYW5uZWwuDQoNClJlZ2FyZHMsDQoNCklsYW4uDQo=

2014-05-06 10:49:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

On Fri, 2014-05-02 at 16:40 +0300, Luca Coelho wrote:

> + * @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY: Notify that a channel switch
> + * has been started on an interface, regardless of the initiator
> + * (ie. whether it was requested from a remote device or
> + * initiated on our own). It indicates that
> + * %NL80211_ATTR_IFINDEX will be on %NL80211_ATTR_WIPHY_FREQ
> + * after %NL80211_ATTR_CH_SWITCH_COUNT TBTT's.

You don't seem to have that now, but it may not always be needed, and we
could add it later (unless userspace needs to rely on having the
attribute - otherwise it has to check for it)

Actually though, maybe you do need it, so that you can determine whether
it makes sense to try anything smart - if the count is 1 or you received
an action frame then you probably don't have time at all?

> + * @NL80211_CMD_CH_SWITCH_FAILED_NOTIFY: Notify that a channel switch
> + * has failed on %NL80211_ATTR_IFINDEX. This notification can
> + * only be sent after a @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY has
> + * been issued.

(+Ilan, Andrei)

I'm not really sure why you have this - it seems that in this case the
interface will be stopped so you'll get a STOP_AP or DISCONNECT or
whatever other notification. This may not actually be completely true
right now (I seem to remember a fix in this area but can't seem to find
it in my tree!!) but we'll have to fix that part anyway. Not really sure
why then that couldn't be used instead of this notification? The
userspace code is going to have to worry about that anyway, I'd think.

johannes


2014-05-06 12:49:32

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

On 6 May 2014 12:47, Johannes Berg <[email protected]> wrote:
> On Fri, 2014-05-02 at 16:40 +0300, Luca Coelho wrote:
>
>> + * @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY: Notify that a channel switch
>> + * has been started on an interface, regardless of the initiator
>> + * (ie. whether it was requested from a remote device or
>> + * initiated on our own). It indicates that
>> + * %NL80211_ATTR_IFINDEX will be on %NL80211_ATTR_WIPHY_FREQ
>> + * after %NL80211_ATTR_CH_SWITCH_COUNT TBTT's.
>
> You don't seem to have that now, but it may not always be needed, and we
> could add it later (unless userspace needs to rely on having the
> attribute - otherwise it has to check for it)
>
> Actually though, maybe you do need it, so that you can determine whether
> it makes sense to try anything smart - if the count is 1 or you received
> an action frame then you probably don't have time at all?
>
>> + * @NL80211_CMD_CH_SWITCH_FAILED_NOTIFY: Notify that a channel switch
>> + * has failed on %NL80211_ATTR_IFINDEX. This notification can
>> + * only be sent after a @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY has
>> + * been issued.
>
> (+Ilan, Andrei)
>
> I'm not really sure why you have this - it seems that in this case the
> interface will be stopped so you'll get a STOP_AP or DISCONNECT or
> whatever other notification. This may not actually be completely true
> right now (I seem to remember a fix in this area but can't seem to find
> it in my tree!!) but we'll have to fix that part anyway. Not really sure
> why then that couldn't be used instead of this notification? The
> userspace code is going to have to worry about that anyway, I'd think.

Just a reminder - there's my pending patchset (mac80211/cfg80211: CSA
fixes/cleanups) for CSA that disconnects interfaces that fail to
switch channel. Perhaps I should re-post?


MichaƂ

2014-05-02 13:41:14

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 2/4] mac80211: send channel switch started notifications

From: Luciano Coelho <[email protected]>

Send a channel switch notification to userspace when a channel switch
is requested or when we react to a remote CSA.

Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/cfg.c | 2 ++
net/mac80211/mlme.c | 2 ++
2 files changed, 4 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 7b8d3cf..53a2cfe 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3281,6 +3281,8 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
sdata->csa_chandef = params->chandef;
sdata->vif.csa_active = true;

+ cfg80211_ch_switch_started_notify(sdata->dev, &sdata->csa_chandef);
+
if (changed) {
ieee80211_bss_info_change_notify(sdata, changed);
drv_channel_switch_beacon(sdata, &params->chandef);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 488826f..139005d 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1107,6 +1107,8 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
IEEE80211_MAX_QUEUE_MAP,
IEEE80211_QUEUE_STOP_REASON_CSA);

+ cfg80211_ch_switch_started_notify(sdata->dev, &csa_ie.chandef);
+
if (local->ops->channel_switch) {
/* use driver's channel switch callback */
struct ieee80211_channel_switch ch_switch = {
--
1.9.2


2014-05-07 11:45:30

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

On 7 May 2014 13:19, Johannes Berg <[email protected]> wrote:
> On Wed, 2014-05-07 at 10:27 +0000, Peer, Ilan wrote:
>
>> > In the first case, there's no notification, nor any need for it. In the second
>> > case, the scenario you suggest doesn't apply, and STOP_AP has to happen
>> > anyway because the state is completely messed up, clients will be in the
>> > process of switching etc.
>> >
>> > > Other than that, a STOP_AP might introduce some races, as
>> > > wpa_supplicant/hostap will not know if the stop_ap was due to the
>> > > failed CS or due to some other reason.
>> >
>> > I don't see why that would matter - even if the STOP_AP *was* for some
>> > other reason, but happened in the middle of the CS flow, the reaction would
>> > presumably be the same?
>> >
>>
>> It can be beneficial to know that the STOP_AP was called due to
>> failure of CS, as wpa_supplicant/hostap can tear down the AP and then
>> (if possible) set it up again on the new channel or another channel.
>
> Maybe that's more of an argument for adding a sort of "reason" or
> "cause" to the STOP_AP? (Btw, no tear down needed in this case - that's
> already done)
>
> OTOH, what else are you thinking of doing? Why would you ever, under any
> circumstances, not restart the AP if it was stopped by the device?

I can smell a recursive start-stop ping-pong if you restart unconditionally.


Michal

2014-05-06 13:08:40

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

On Tue, 2014-05-06 at 14:49 +0200, Michal Kazior wrote:
> > (I seem to remember a fix in this area but can't seem to find
> > it in my tree!!)
>
> Just a reminder - there's my pending patchset (mac80211/cfg80211: CSA
> fixes/cleanups) for CSA that disconnects interfaces that fail to
> switch channel. Perhaps I should re-post?

D'oh. No, I have it, I'll pick it up. I just forgot - there was some
minor thing with it and I thought Luca might pick it up, so I dropped
it. I'll go look at it now.

johannes


2014-05-07 11:56:08

by Ilan Peer

[permalink] [raw]
Subject: RE: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogSm9oYW5uZXMgQmVyZyBb
bWFpbHRvOmpvaGFubmVzQHNpcHNvbHV0aW9ucy5uZXRdDQo+IFNlbnQ6IFdlZG5lc2RheSwgTWF5
IDA3LCAyMDE0IDE0OjIwDQo+IFRvOiBQZWVyLCBJbGFuDQo+IENjOiBMdWNhIENvZWxobzsgbGlu
dXgtd2lyZWxlc3NAdmdlci5rZXJuZWwub3JnOyBtaWNoYWwua2F6aW9yQHRpZXRvLmNvbTsNCj4g
T3RjaGVyZXRpYW5za2ksIEFuZHJlaQ0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDEvNF0gY2ZnODAy
MTEvbmw4MDIxMTogYWRkIGNoYW5uZWwgc3dpdGNoIHN0YXJ0ZWQgYW5kDQo+IGZhaWxlZCBub3Rp
ZmljYXRpb25zDQo+IA0KPiBPbiBXZWQsIDIwMTQtMDUtMDcgYXQgMTA6MjcgKzAwMDAsIFBlZXIs
IElsYW4gd3JvdGU6DQo+IA0KPiA+ID4gSW4gdGhlIGZpcnN0IGNhc2UsIHRoZXJlJ3Mgbm8gbm90
aWZpY2F0aW9uLCBub3IgYW55IG5lZWQgZm9yIGl0LiBJbg0KPiA+ID4gdGhlIHNlY29uZCBjYXNl
LCB0aGUgc2NlbmFyaW8geW91IHN1Z2dlc3QgZG9lc24ndCBhcHBseSwgYW5kIFNUT1BfQVANCj4g
PiA+IGhhcyB0byBoYXBwZW4gYW55d2F5IGJlY2F1c2UgdGhlIHN0YXRlIGlzIGNvbXBsZXRlbHkg
bWVzc2VkIHVwLA0KPiA+ID4gY2xpZW50cyB3aWxsIGJlIGluIHRoZSBwcm9jZXNzIG9mIHN3aXRj
aGluZyBldGMuDQo+ID4gPg0KPiA+ID4gPiBPdGhlciB0aGFuIHRoYXQsIGEgU1RPUF9BUCBtaWdo
dCBpbnRyb2R1Y2Ugc29tZSByYWNlcywgYXMNCj4gPiA+ID4gd3BhX3N1cHBsaWNhbnQvaG9zdGFw
IHdpbGwgbm90IGtub3cgaWYgdGhlIHN0b3BfYXAgd2FzIGR1ZSB0byB0aGUNCj4gPiA+ID4gZmFp
bGVkIENTIG9yIGR1ZSB0byBzb21lIG90aGVyIHJlYXNvbi4NCj4gPiA+DQo+ID4gPiBJIGRvbid0
IHNlZSB3aHkgdGhhdCB3b3VsZCBtYXR0ZXIgLSBldmVuIGlmIHRoZSBTVE9QX0FQICp3YXMqIGZv
cg0KPiA+ID4gc29tZSBvdGhlciByZWFzb24sIGJ1dCBoYXBwZW5lZCBpbiB0aGUgbWlkZGxlIG9m
IHRoZSBDUyBmbG93LCB0aGUNCj4gPiA+IHJlYWN0aW9uIHdvdWxkIHByZXN1bWFibHkgYmUgdGhl
IHNhbWU/DQo+ID4gPg0KPiA+DQo+ID4gSXQgY2FuIGJlIGJlbmVmaWNpYWwgdG8ga25vdyB0aGF0
IHRoZSBTVE9QX0FQIHdhcyBjYWxsZWQgZHVlIHRvDQo+ID4gZmFpbHVyZSBvZiBDUywgYXMgd3Bh
X3N1cHBsaWNhbnQvaG9zdGFwIGNhbiB0ZWFyIGRvd24gdGhlIEFQIGFuZCB0aGVuDQo+ID4gKGlm
IHBvc3NpYmxlKSBzZXQgaXQgdXAgYWdhaW4gb24gdGhlIG5ldyBjaGFubmVsIG9yIGFub3RoZXIg
Y2hhbm5lbC4NCj4gDQo+IE1heWJlIHRoYXQncyBtb3JlIG9mIGFuIGFyZ3VtZW50IGZvciBhZGRp
bmcgYSBzb3J0IG9mICJyZWFzb24iIG9yICJjYXVzZSIgdG8NCj4gdGhlIFNUT1BfQVA/IChCdHcs
IG5vIHRlYXIgZG93biBuZWVkZWQgaW4gdGhpcyBjYXNlIC0gdGhhdCdzIGFscmVhZHkgZG9uZSkN
Cj4gDQoNClllcyA6KQ0KDQo+IE9UT0gsIHdoYXQgZWxzZSBhcmUgeW91IHRoaW5raW5nIG9mIGRv
aW5nPyBXaHkgd291bGQgeW91IGV2ZXIsIHVuZGVyIGFueQ0KPiBjaXJjdW1zdGFuY2VzLCBub3Qg
cmVzdGFydCB0aGUgQVAgaWYgaXQgd2FzIHN0b3BwZWQgYnkgdGhlIGRldmljZT8NCg0KQWN0dWFs
bHkgdGhpcyBpcyB0aGUgZGVmYXVsdCB0b2RheTogb24gU1RPUF9BUCBub3RpZmljYXRpb24gYSBH
TyBpbnRlcmZhY2UgaXMgc2ltcGx5IGRlbGV0ZWQsIGFzIHRoZSBhc3N1bXB0aW9uIGlzIHRoYXQg
c29tZXRoaW5nIHVucmVjb3ZlcmFibGUgaGFwcGVuZWQuIEluIGNhc2Ugb2YgQ1MgZmFpbHVyZSwg
aXQgd291bGQgYmUgKm5pY2UqIHRvIGtub3cgaWYgdGhlIGZhaWx1cmUgaXMgcmVjb3ZlcmFibGUg
b3Igbm90Lg0KDQpSZWdhcmRzLA0KDQpJbGFuLg0KDQoNCiANCg0K

2014-05-07 11:19:56

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

On Wed, 2014-05-07 at 10:27 +0000, Peer, Ilan wrote:

> > In the first case, there's no notification, nor any need for it. In the second
> > case, the scenario you suggest doesn't apply, and STOP_AP has to happen
> > anyway because the state is completely messed up, clients will be in the
> > process of switching etc.
> >
> > > Other than that, a STOP_AP might introduce some races, as
> > > wpa_supplicant/hostap will not know if the stop_ap was due to the
> > > failed CS or due to some other reason.
> >
> > I don't see why that would matter - even if the STOP_AP *was* for some
> > other reason, but happened in the middle of the CS flow, the reaction would
> > presumably be the same?
> >
>
> It can be beneficial to know that the STOP_AP was called due to
> failure of CS, as wpa_supplicant/hostap can tear down the AP and then
> (if possible) set it up again on the new channel or another channel.

Maybe that's more of an argument for adding a sort of "reason" or
"cause" to the STOP_AP? (Btw, no tear down needed in this case - that's
already done)

OTOH, what else are you thinking of doing? Why would you ever, under any
circumstances, not restart the AP if it was stopped by the device?

johannes


2014-05-07 06:08:23

by Ilan Peer

[permalink] [raw]
Subject: RE: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogbGludXgtd2lyZWxlc3Mt
b3duZXJAdmdlci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtd2lyZWxlc3MtDQo+IG93bmVyQHZn
ZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIEpvaGFubmVzIEJlcmcNCj4gU2VudDogVHVlc2Rh
eSwgTWF5IDA2LCAyMDE0IDEzOjQ3DQo+IFRvOiBMdWNhIENvZWxobw0KPiBDYzogbGludXgtd2ly
ZWxlc3NAdmdlci5rZXJuZWwub3JnOyBtaWNoYWwua2F6aW9yQHRpZXRvLmNvbTsgUGVlciwgSWxh
bjsNCj4gT3RjaGVyZXRpYW5za2ksIEFuZHJlaQ0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDEvNF0g
Y2ZnODAyMTEvbmw4MDIxMTogYWRkIGNoYW5uZWwgc3dpdGNoIHN0YXJ0ZWQgYW5kDQo+IGZhaWxl
ZCBub3RpZmljYXRpb25zDQo+IA0KPiBPbiBGcmksIDIwMTQtMDUtMDIgYXQgMTY6NDAgKzAzMDAs
IEx1Y2EgQ29lbGhvIHdyb3RlOg0KPiANCj4gPiArICogQE5MODAyMTFfQ01EX0NIX1NXSVRDSF9T
VEFSVEVEX05PVElGWTogTm90aWZ5IHRoYXQgYSBjaGFubmVsDQo+IHN3aXRjaA0KPiA+ICsgKglo
YXMgYmVlbiBzdGFydGVkIG9uIGFuIGludGVyZmFjZSwgcmVnYXJkbGVzcyBvZiB0aGUgaW5pdGlh
dG9yDQo+ID4gKyAqCShpZS4gd2hldGhlciBpdCB3YXMgcmVxdWVzdGVkIGZyb20gYSByZW1vdGUg
ZGV2aWNlIG9yDQo+ID4gKyAqCWluaXRpYXRlZCBvbiBvdXIgb3duKS4gIEl0IGluZGljYXRlcyB0
aGF0DQo+ID4gKyAqCSVOTDgwMjExX0FUVFJfSUZJTkRFWCB3aWxsIGJlIG9uICVOTDgwMjExX0FU
VFJfV0lQSFlfRlJFUQ0KPiA+ICsgKglhZnRlciAlTkw4MDIxMV9BVFRSX0NIX1NXSVRDSF9DT1VO
VCBUQlRUJ3MuDQo+IA0KPiBZb3UgZG9uJ3Qgc2VlbSB0byBoYXZlIHRoYXQgbm93LCBidXQgaXQg
bWF5IG5vdCBhbHdheXMgYmUgbmVlZGVkLCBhbmQgd2UNCj4gY291bGQgYWRkIGl0IGxhdGVyICh1
bmxlc3MgdXNlcnNwYWNlIG5lZWRzIHRvIHJlbHkgb24gaGF2aW5nIHRoZSBhdHRyaWJ1dGUgLQ0K
PiBvdGhlcndpc2UgaXQgaGFzIHRvIGNoZWNrIGZvciBpdCkNCj4gDQo+IEFjdHVhbGx5IHRob3Vn
aCwgbWF5YmUgeW91IGRvIG5lZWQgaXQsIHNvIHRoYXQgeW91IGNhbiBkZXRlcm1pbmUgd2hldGhl
ciBpdA0KPiBtYWtlcyBzZW5zZSB0byB0cnkgYW55dGhpbmcgc21hcnQgLSBpZiB0aGUgY291bnQg
aXMgMSBvciB5b3UgcmVjZWl2ZWQgYW4gYWN0aW9uDQo+IGZyYW1lIHRoZW4geW91IHByb2JhYmx5
IGRvbid0IGhhdmUgdGltZSBhdCBhbGw/DQo+IA0KPiA+ICsgKiBATkw4MDIxMV9DTURfQ0hfU1dJ
VENIX0ZBSUxFRF9OT1RJRlk6IE5vdGlmeSB0aGF0IGEgY2hhbm5lbA0KPiBzd2l0Y2gNCj4gPiAr
ICoJaGFzIGZhaWxlZCBvbiAlTkw4MDIxMV9BVFRSX0lGSU5ERVguICBUaGlzIG5vdGlmaWNhdGlv
biBjYW4NCj4gPiArICoJb25seSBiZSBzZW50IGFmdGVyIGEgQE5MODAyMTFfQ01EX0NIX1NXSVRD
SF9TVEFSVEVEX05PVElGWQ0KPiBoYXMNCj4gPiArICoJYmVlbiBpc3N1ZWQuDQo+IA0KPiAoK0ls
YW4sIEFuZHJlaSkNCj4gDQo+IEknbSBub3QgcmVhbGx5IHN1cmUgd2h5IHlvdSBoYXZlIHRoaXMg
LSBpdCBzZWVtcyB0aGF0IGluIHRoaXMgY2FzZSB0aGUgaW50ZXJmYWNlDQo+IHdpbGwgYmUgc3Rv
cHBlZCBzbyB5b3UnbGwgZ2V0IGEgU1RPUF9BUCBvciBESVNDT05ORUNUIG9yIHdoYXRldmVyIG90
aGVyDQo+IG5vdGlmaWNhdGlvbi4gVGhpcyBtYXkgbm90IGFjdHVhbGx5IGJlIGNvbXBsZXRlbHkg
dHJ1ZSByaWdodCBub3cgKEkgc2VlbSB0bw0KPiByZW1lbWJlciBhIGZpeCBpbiB0aGlzIGFyZWEg
YnV0IGNhbid0IHNlZW0gdG8gZmluZCBpdCBpbiBteSB0cmVlISEpIGJ1dCB3ZSdsbA0KPiBoYXZl
IHRvIGZpeCB0aGF0IHBhcnQgYW55d2F5LiBOb3QgcmVhbGx5IHN1cmUgd2h5IHRoZW4gdGhhdCBj
b3VsZG4ndCBiZSB1c2VkDQo+IGluc3RlYWQgb2YgdGhpcyBub3RpZmljYXRpb24/IFRoZSB1c2Vy
c3BhY2UgY29kZSBpcyBnb2luZyB0byBoYXZlIHRvIHdvcnJ5DQo+IGFib3V0IHRoYXQgYW55d2F5
LCBJJ2QgdGhpbmsuDQo+IA0KDQpBdCBsZWFzdCBmb3IgQVAvR08sIHRoZXJlIGFyZSBjYXNlcyB0
aGF0IGEgZmFpbGVkIGNoYW5uZWwgc3dpdGNoIHNob3VsZCBub3QgbmVjZXNzYXJpbHkgdHJpZ2dl
ciBhIFNUT1BfQVAgZmxvdy4gRm9yIGV4YW1wbGUsIGluIGNhc2UgdGhhdCBtdWx0aS1jaGFubmVs
IGlzIHN1cHBvcnRlZCwgd3BhX3N1cHBsaWNhbnQgbWlnaHQgcmVxdWVzdCBhIGNoYW5uZWwgc3dp
dGNoIHRvIHRyeSB0byBzd2l0Y2ggYW4gQVAvR08gaW50ZXJmYWNlIHRvIGEgY2hhbm5lbCB1c2Vk
IGJ5IGEgc3RhdGlvbiBpbnRlcmZhY2UgKHRvIGF2b2lkIG11bHRpIGNoYW5uZWwgb3BlcmF0aW9u
KSwgYnV0IGluIGNhc2UgdGhhdCBzdWNoIGEgdHJhbnNpdGlvbiBmYWlscywgaXQgaXMgc3RpbGwg
YSB2YWxpZCB0byBjb250aW51ZSB0byB1c2UgY3VycmVudCBjaGFubmVsIGFuZCBkbyBub3Qgc3Rv
cCB0aGUgR08uIEluIHN1Y2ggYSBjYXNlLCBpdCB3b3VsZCBiZSB1c2VmdWwgdG8gZ2V0IGEgZmFp
bHVyZSBub3RpZmljYXRpb24uIEdlbmVyYWxseSwgSSB0aGluayB0aGF0IHNpbmNlIGhvc3RhcGQv
d3BhX3N1cHBsaWNhbnQgaGF2ZSBzdGFydGVkIHRoZSBjaGFubmVsIHN3aXRjaCwgaXQgbWlnaHQg
YmUgYmV0dGVyIHRvIGhhdmUgdGhlbSBoYW5kbGUgYSBub3RpZmljYXRpb24gYW5kIHRha2UgdGhl
IGFwcHJvcHJpYXRlIGFjdGlvbiAoaWYgbmVlZGVkIHN0b3AgdGhlIEFQL0dPIC4uLikuIA0KDQpP
dGhlciB0aGFuIHRoYXQsIGEgU1RPUF9BUCBtaWdodCBpbnRyb2R1Y2Ugc29tZSByYWNlcywgYXMg
d3BhX3N1cHBsaWNhbnQvaG9zdGFwIHdpbGwgbm90IGtub3cgaWYgdGhlIHN0b3BfYXAgd2FzIGR1
ZSB0byB0aGUgZmFpbGVkIENTIG9yIGR1ZSB0byBzb21lIG90aGVyIHJlYXNvbi4NCg0KUmVnYXJk
cywNCg0KSWxhbi4NCg0K

2014-05-06 10:47:43

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

On Fri, 2014-05-02 at 16:40 +0300, Luca Coelho wrote:

> + * @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY: Notify that a channel switch
> + * has been started on an interface, regardless of the initiator
> + * (ie. whether it was requested from a remote device or
> + * initiated on our own). It indicates that
> + * %NL80211_ATTR_IFINDEX will be on %NL80211_ATTR_WIPHY_FREQ
> + * after %NL80211_ATTR_CH_SWITCH_COUNT TBTT's.

You don't seem to have that now, but it may not always be needed, and we
could add it later (unless userspace needs to rely on having the
attribute - otherwise it has to check for it)

Actually though, maybe you do need it, so that you can determine whether
it makes sense to try anything smart - if the count is 1 or you received
an action frame then you probably don't have time at all?

> + * @NL80211_CMD_CH_SWITCH_FAILED_NOTIFY: Notify that a channel switch
> + * has failed on %NL80211_ATTR_IFINDEX. This notification can
> + * only be sent after a @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY has
> + * been issued.

(+Ilan, Andrei)

I'm not really sure why you have this - it seems that in this case the
interface will be stopped so you'll get a STOP_AP or DISCONNECT or
whatever other notification. This may not actually be completely true
right now (I seem to remember a fix in this area but can't seem to find
it in my tree!!) but we'll have to fix that part anyway. Not really sure
why then that couldn't be used instead of this notification? The
userspace code is going to have to worry about that anyway, I'd think.

johannes


2014-05-02 13:41:15

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 3/4] mac80211: send channel switch failed notifications

From: Luciano Coelho <[email protected]>

Send a channel switch failed notification to userspace when an ongoing
channel swith fails.

Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/cfg.c | 30 +++++++++++++++++++++++-------
net/mac80211/iface.c | 6 +++++-
net/mac80211/mlme.c | 9 ++++++++-
3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 53a2cfe..f8958e0 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1101,9 +1101,13 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
old_probe_resp = sdata_dereference(sdata->u.ap.probe_resp, sdata);

/* abort any running channel switch */
- sdata->vif.csa_active = false;
- kfree(sdata->u.ap.next_beacon);
- sdata->u.ap.next_beacon = NULL;
+ if (sdata->vif.csa_active) {
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
+ sdata->vif.csa_active = false;
+ kfree(sdata->u.ap.next_beacon);
+ sdata->u.ap.next_beacon = NULL;
+ }

/* turn off carrier for this interface and dependent VLANs */
list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list)
@@ -3030,8 +3034,11 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
sdata->radar_required = sdata->csa_radar_required;
err = ieee80211_vif_change_channel(sdata, &changed);
mutex_unlock(&local->mtx);
- if (WARN_ON(err < 0))
+ if (WARN_ON(err < 0)) {
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
return;
+ }

if (!local->use_chanctx) {
local->_oper_chandef = sdata->csa_chandef;
@@ -3045,21 +3052,30 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
kfree(sdata->u.ap.next_beacon);
sdata->u.ap.next_beacon = NULL;

- if (err < 0)
+ if (err < 0) {
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
return;
+ }
changed |= err;
break;
case NL80211_IFTYPE_ADHOC:
err = ieee80211_ibss_finish_csa(sdata);
- if (err < 0)
+ if (err < 0) {
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
return;
+ }
changed |= err;
break;
#ifdef CONFIG_MAC80211_MESH
case NL80211_IFTYPE_MESH_POINT:
err = ieee80211_mesh_finish_csa(sdata);
- if (err < 0)
+ if (err < 0) {
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
return;
+ }
changed |= err;
break;
#endif
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 7fff3dc..578f6e6 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -838,7 +838,11 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,

cancel_work_sync(&sdata->recalc_smps);
sdata_lock(sdata);
- sdata->vif.csa_active = false;
+ if (sdata->vif.csa_active) {
+ sdata->vif.csa_active = false;
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
+ }
sdata_unlock(sdata);
cancel_work_sync(&sdata->csa_finalize_work);

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 139005d..beff8c8 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -958,6 +958,8 @@ static void ieee80211_chswitch_work(struct work_struct *work)
"vif channel switch failed, disconnecting\n");
ieee80211_queue_work(&sdata->local->hw,
&ifmgd->csa_connection_drop_work);
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
goto out;
}

@@ -2060,7 +2062,12 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY,
true, frame_buf);
ifmgd->flags &= ~IEEE80211_STA_CSA_RECEIVED;
- sdata->vif.csa_active = false;
+
+ if (sdata->vif.csa_active) {
+ cfg80211_ch_switch_failed_notify(sdata->dev,
+ &sdata->csa_chandef);
+ sdata->vif.csa_active = false;
+ }
ieee80211_wake_queues_by_reason(&sdata->local->hw,
IEEE80211_MAX_QUEUE_MAP,
IEEE80211_QUEUE_STOP_REASON_CSA);
--
1.9.2


2014-05-13 10:31:35

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and failed notifications

Finally got the time to come back to this...

On Wed, 2014-05-07 at 11:55 +0000, Peer, Ilan wrote:
>
>
> > -----Original Message-----
> > From: Johannes Berg [mailto:[email protected]]
> > Sent: Wednesday, May 07, 2014 14:20
> > To: Peer, Ilan
> > Cc: Luca Coelho; [email protected]; [email protected];
> > Otcheretianski, Andrei
> > Subject: Re: [PATCH 1/4] cfg80211/nl80211: add channel switch started and
> > failed notifications
> >
> > On Wed, 2014-05-07 at 10:27 +0000, Peer, Ilan wrote:
> >
> > > > In the first case, there's no notification, nor any need for it. In
> > > > the second case, the scenario you suggest doesn't apply, and STOP_AP
> > > > has to happen anyway because the state is completely messed up,
> > > > clients will be in the process of switching etc.
> > > >
> > > > > Other than that, a STOP_AP might introduce some races, as
> > > > > wpa_supplicant/hostap will not know if the stop_ap was due to the
> > > > > failed CS or due to some other reason.
> > > >
> > > > I don't see why that would matter - even if the STOP_AP *was* for
> > > > some other reason, but happened in the middle of the CS flow, the
> > > > reaction would presumably be the same?
> > > >
> > >
> > > It can be beneficial to know that the STOP_AP was called due to
> > > failure of CS, as wpa_supplicant/hostap can tear down the AP and then
> > > (if possible) set it up again on the new channel or another channel.
> >
> > Maybe that's more of an argument for adding a sort of "reason" or "cause" to
> > the STOP_AP? (Btw, no tear down needed in this case - that's already done)
> >
>
> Yes :)
>
> > OTOH, what else are you thinking of doing? Why would you ever, under any
> > circumstances, not restart the AP if it was stopped by the device?
>
> Actually this is the default today: on STOP_AP notification a GO interface is simply deleted, as the assumption is that something unrecoverable happened. In case of CS failure, it would be *nice* to know if the failure is recoverable or not.

Okay, so this is what I'll do:

* remove the failed notifications
* in a new patch, add a reason attribute to the STOP_AP notification;

--
Luca.


2014-05-02 13:41:16

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 4/4] cfg80211/nl80211: allow any interface to send channel switch notifications

From: Luciano Coelho <[email protected]>

For multi-vif channel switches, we want to send
NL80211_CMD_CH_SWITCH_NOTIFY to the userspace to let it decide whether
other interfaces need to be moved as well. This is needed when we run
out of available channels or when we want a P2P GO interface to follow
the channel of a station, for example.

Modify the code so that all interfaces can send CSA notifications.
Additionally, send notifications for STA CSA as well.

Signed-off-by: Luciano Coelho <[email protected]>
---
include/uapi/linux/nl80211.h | 4 +++-
net/mac80211/mlme.c | 1 +
net/wireless/nl80211.c | 6 ------
3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index d7c5b61..635267d 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -636,7 +636,9 @@
* @NL80211_CMD_CH_SWITCH_NOTIFY: An AP or GO may decide to switch channels
* independently of the userspace SME, send this event indicating
* %NL80211_ATTR_IFINDEX is now on %NL80211_ATTR_WIPHY_FREQ and the
- * attributes determining channel width.
+ * attributes determining channel width. This indication may also be
+ * sent when a remotely-initiated switch (eg. when a STA receives a CSA
+ * from the remote AP) is completed;
*
* @NL80211_CMD_CH_SWITCH_STARTED_NOTIFY: Notify that a channel switch
* has been started on an interface, regardless of the initiator
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index beff8c8..59c59da 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -984,6 +984,7 @@ static void ieee80211_chswitch_work(struct work_struct *work)

ieee80211_bss_info_change_notify(sdata, changed);

+ cfg80211_ch_switch_notify(sdata->dev, &sdata->reserved_chandef);
out:
sdata->vif.csa_active = false;
ifmgd->flags &= ~IEEE80211_STA_CSA_RECEIVED;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 804c3c9..aea60bb 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -11252,12 +11252,6 @@ void cfg80211_ch_switch_notify(struct net_device *dev,

trace_cfg80211_ch_switch_notify(dev, chandef);

- if (WARN_ON(wdev->iftype != NL80211_IFTYPE_AP &&
- wdev->iftype != NL80211_IFTYPE_P2P_GO &&
- wdev->iftype != NL80211_IFTYPE_ADHOC &&
- wdev->iftype != NL80211_IFTYPE_MESH_POINT))
- return;
-
wdev->chandef = *chandef;
wdev->preset_chandef = *chandef;
nl80211_ch_switch_notify(rdev, dev, chandef, GFP_KERNEL,
--
1.9.2