This is the v2 series prepared by Amitkumar Karwar.
1) TX_ABORT flag is renamed to LOW_PRIORITY to have more generic
approach. Let the drivers decide behaviour for background scan.
2) An extra parameter is added to __cfg80211_bss_expire() to
avoid race condition pointed by Johannes.
Note that "PATCH v2 5/5 mwifiex: use LOW_PRIORITY flag ..."
does not apply to mac80211-next cleanly because some recent
mwifiex patches in wireless-testing have not been pulled yet.
=== original post (v1) by Sam Leffler ===
These add per-scan request controls for optional behaviours we've found
useful (they've been in Chrome OS for a long time). A patch for iw will
follow separately and the mods for wpa_supplicant are in our repo and
I'll push them to Jouni if these are accepted.
Amitkumar Karwar (1):
mwifiex: use LOW_PRIORITY flag provided in scan request
Sam Leffler (4):
{nl,cfg}80211: add a flags word to scan requests
cfg80211: add scan flag to indicate its priority
cfg80211: add support for flushing old scan results
mac80211: add support for tx to abort low priority scan requests
drivers/net/wireless/mwifiex/cfg80211.c | 3 ++-
drivers/net/wireless/mwifiex/scan.c | 6 +++++-
include/linux/nl80211.h | 19 +++++++++++++++++++
include/net/cfg80211.h | 17 +++++++++++++++++
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/scan.c | 22 ++++++++++++++++++----
net/wireless/nl80211.c | 12 ++++++++++++
net/wireless/scan.c | 31 +++++++++++++++++++++++++++----
net/wireless/sme.c | 1 +
9 files changed, 103 insertions(+), 10 deletions(-)
On Fri, Sep 28, 2012 at 4:06 AM, Johannes Berg
<[email protected]> wrote:
> On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote:
>
>> + if (associated && !tx_empty) {
>> + if (unlikely(local->scan_req->flags &
>> + CFG80211_SCAN_FLAG_LOW_PRIORITY))
>
> I don't really see value in the "unlikely()" here, that just clutters
> the code and probably has very little effect on the runtime behaviour,
> this is very infrequently executed.
>
>
>> + case SCAN_ABORT:
>> + aborted = true;
>> + goto out_complete;
>
> Maybe we should have different flags though ... I mean, this is the
> first implementation that I hear of that interprets a background scan as
> "ok to abort at any time"? It seems very unlikely that other
> implementations would do that. I *think* (like I said before, I don't
> really know) that ours (Intel's) will just shorten the dwell time, or
> similar instead.
Well previous implementations I've done have used this technique for
many years and you can find them in various products (assuming they
haven't been totally rewritten) :-)
-Sam
On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote:
> From: Amitkumar Karwar <[email protected]>
>
> The flag gives information about scan priority. This patch adds
> checks in the code to cancel/abort scan operation only for low
> priority scan requests.
I don't take mwifiex patches, so sending this in the same series isn't
all that useful.
Let's sort out the other stuff first, and then you can implement it in
the driver. :)
johannes
On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote:
> +static void __cfg80211_bss_expire(struct cfg80211_registered_device *dev,
> + int maxage, unsigned long expire_time);
Please no static forward declarations, unless you really can't move the
code (say because it's recursive). You can move the code up instead.
> /* must hold dev->bss_lock! */
> -void cfg80211_bss_expire(struct cfg80211_registered_device *dev)
> +static void __cfg80211_bss_expire(struct cfg80211_registered_device *dev,
> + int maxage, unsigned long expire_time)
I don't really see why you need maxage and expire_time?
> +void cfg80211_bss_expire(struct cfg80211_registered_device *dev)
> +{
> + __cfg80211_bss_expire(dev, IEEE80211_SCAN_RESULT_EXPIRE, 0);
Here, you could just pass "jiffies - SCAN_RESULT_EXPIRE" as the maxage
and get pretty much the same behaviour as before without a second
parameter.
johannes
From: Amitkumar Karwar <[email protected]>
The flag gives information about scan priority. This patch adds
checks in the code to cancel/abort scan operation only for low
priority scan requests.
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
Reviewed-by: Sam Leffler <[email protected]>
---
drivers/net/wireless/mwifiex/cfg80211.c | 3 ++-
drivers/net/wireless/mwifiex/scan.c | 6 +++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index 2691620..dc6d868 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -1808,7 +1808,8 @@ mwifiex_cfg80211_scan(struct wiphy *wiphy,
wiphy_dbg(wiphy, "info: received scan request on %s\n", dev->name);
- if (atomic_read(&priv->wmm.tx_pkts_queued) >=
+ if ((request->flags & CFG80211_SCAN_FLAG_LOW_PRIORITY) &&
+ atomic_read(&priv->wmm.tx_pkts_queued) >=
MWIFIEX_MIN_TX_PENDING_TO_CANCEL_SCAN) {
dev_dbg(priv->adapter->dev, "scan rejected due to traffic\n");
return -EBUSY;
diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
index 9e077e5..b3a1d2f 100644
--- a/drivers/net/wireless/mwifiex/scan.c
+++ b/drivers/net/wireless/mwifiex/scan.c
@@ -1770,12 +1770,16 @@ int mwifiex_ret_802_11_scan(struct mwifiex_private *priv,
priv->user_scan_cfg = NULL;
}
} else {
- if (!mwifiex_wmm_lists_empty(adapter)) {
+ if (!mwifiex_wmm_lists_empty(adapter) &&
+ (priv->scan_request && (priv->scan_request->flags &
+ CFG80211_SCAN_FLAG_LOW_PRIORITY))) {
spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
flags);
adapter->scan_delay_cnt = 1;
mod_timer(&priv->scan_delay_timer, jiffies +
msecs_to_jiffies(MWIFIEX_SCAN_DELAY_MSEC));
+ dev_dbg(priv->adapter->dev,
+ "%s: deferring scan\n", __func__);
} else {
/* Get scan command from scan_pending_q and put to
cmd_pending_q */
--
1.7.0.2
From: Sam Leffler <[email protected]>
Add NL80211_SCAN_FLAG_LOW_PRIORITY flag support. It tells drivers
that this is a low priority scan request, so that they can take
necessary action.
Signed-off-by: Sam Leffler <[email protected]>
Tested-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
include/linux/nl80211.h | 3 +++
include/net/cfg80211.h | 3 +++
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 109f75d..e512baf 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -3079,8 +3079,11 @@ enum nl80211_connect_failed_reason {
* Scan request control flags are used to control the handling
* of NL80211_CMD_TRIGGER_SCAN and NL80211_CMD_START_SCHED_SCAN
* requests.
+ *
+ * @NL80211_SCAN_FLAG_LOW_PRIORITY: scan request has low priority
*/
enum nl80211_scan_flags {
+ NL80211_SCAN_FLAG_LOW_PRIORITY = 1<<0,
};
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 2c0c14a..ed659db 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -993,8 +993,11 @@ struct cfg80211_ssid {
/**
* enum cfg80211_scan_flags - scan request control flags
+ *
+ * @CFG80211_SCAN_FLAG_LOW_PRIORITY: scan request has low priority
*/
enum cfg80211_scan_flags {
+ CFG80211_SCAN_FLAG_LOW_PRIORITY = NL80211_SCAN_FLAG_LOW_PRIORITY,
};
/**
--
1.7.0.2
From: Sam Leffler <[email protected]>
Add an NL80211_SCAN_FLAG_FLUSH flag that causes old bss cache
entries to be flushed on scan completion. This is useful for
collecting guaranteed fresh scan/survey results (e.g. on resume).
Flushing only happens on successful completion of a scan;
i.e. it does not happen if the scan is aborted/canceled.
Signed-off-by: Sam Leffler <[email protected]>
Tested-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
include/linux/nl80211.h | 2 ++
include/net/cfg80211.h | 4 ++++
net/wireless/nl80211.c | 1 +
net/wireless/scan.c | 31 +++++++++++++++++++++++++++----
net/wireless/sme.c | 1 +
5 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index e512baf..71400e7 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -3081,9 +3081,11 @@ enum nl80211_connect_failed_reason {
* requests.
*
* @NL80211_SCAN_FLAG_LOW_PRIORITY: scan request has low priority
+ * @NL80211_SCAN_FLAG_FLUSH: flush cache before scanning
*/
enum nl80211_scan_flags {
NL80211_SCAN_FLAG_LOW_PRIORITY = 1<<0,
+ NL80211_SCAN_FLAG_FLUSH = 1<<1,
};
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ed659db..cc63a3a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -995,9 +995,11 @@ struct cfg80211_ssid {
* enum cfg80211_scan_flags - scan request control flags
*
* @CFG80211_SCAN_FLAG_LOW_PRIORITY: scan request has low priority
+ * @CFG80211_SCAN__FLAG_FLUSH: flush cache before scanning
*/
enum cfg80211_scan_flags {
CFG80211_SCAN_FLAG_LOW_PRIORITY = NL80211_SCAN_FLAG_LOW_PRIORITY,
+ CFG80211_SCAN_FLAG_FLUSH = NL80211_SCAN_FLAG_FLUSH,
};
/**
@@ -1012,6 +1014,7 @@ enum cfg80211_scan_flags {
* @flags: bit field of flags controlling operation
* @rates: bitmap of rates to advertise for each band
* @wiphy: the wiphy this was for
+ * @scan_start: time (in jiffies) when the scan started
* @wdev: the wireless device to scan for
* @aborted: (internal) scan request was notified as aborted
* @no_cck: used to send probe requests at non CCK rate in 2GHz band
@@ -1030,6 +1033,7 @@ struct cfg80211_scan_request {
/* internal */
struct wiphy *wiphy;
+ unsigned long scan_start;
bool aborted;
bool no_cck;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e4312f6..7133477 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4347,6 +4347,7 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
request->wdev = wdev;
request->wiphy = &rdev->wiphy;
+ request->scan_start = jiffies;
rdev->scan_req = request;
err = rdev->ops->scan(&rdev->wiphy, request);
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 9730c98..392e5a9 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -20,6 +20,9 @@
#define IEEE80211_SCAN_RESULT_EXPIRE (30 * HZ)
+static void __cfg80211_bss_expire(struct cfg80211_registered_device *dev,
+ int maxage, unsigned long expire_time);
+
void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak)
{
struct cfg80211_scan_request *request;
@@ -47,8 +50,15 @@ void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak)
if (request->aborted)
nl80211_send_scan_aborted(rdev, wdev);
- else
+ else {
+ if (request->flags & CFG80211_SCAN_FLAG_FLUSH) {
+ /* flush entries from previous scans */
+ spin_lock_bh(&rdev->bss_lock);
+ __cfg80211_bss_expire(rdev, 0, request->scan_start);
+ spin_unlock_bh(&rdev->bss_lock);
+ }
nl80211_send_scan_done(rdev, wdev);
+ }
#ifdef CONFIG_CFG80211_WEXT
if (wdev->netdev && !request->aborted) {
@@ -198,7 +208,8 @@ static void __cfg80211_unlink_bss(struct cfg80211_registered_device *dev,
}
/* must hold dev->bss_lock! */
-void cfg80211_bss_expire(struct cfg80211_registered_device *dev)
+static void __cfg80211_bss_expire(struct cfg80211_registered_device *dev,
+ int maxage, unsigned long expire_time)
{
struct cfg80211_internal_bss *bss, *tmp;
bool expired = false;
@@ -206,8 +217,14 @@ void cfg80211_bss_expire(struct cfg80211_registered_device *dev)
list_for_each_entry_safe(bss, tmp, &dev->bss_list, list) {
if (atomic_read(&bss->hold))
continue;
- if (!time_after(jiffies, bss->ts + IEEE80211_SCAN_RESULT_EXPIRE))
- continue;
+ if (maxage) {
+ if (!time_after(jiffies, bss->ts + maxage))
+ continue;
+ } else {
+ if (!time_after(expire_time, bss->ts))
+ continue;
+ }
+
__cfg80211_unlink_bss(dev, bss);
expired = true;
}
@@ -216,6 +233,11 @@ void cfg80211_bss_expire(struct cfg80211_registered_device *dev)
dev->bss_generation++;
}
+void cfg80211_bss_expire(struct cfg80211_registered_device *dev)
+{
+ __cfg80211_bss_expire(dev, IEEE80211_SCAN_RESULT_EXPIRE, 0);
+}
+
const u8 *cfg80211_find_ie(u8 eid, const u8 *ies, int len)
{
while (len > 2 && ies[0] != eid) {
@@ -962,6 +984,7 @@ int cfg80211_wext_siwscan(struct net_device *dev,
creq->ssids = (void *)&creq->channels[n_channels];
creq->n_channels = n_channels;
creq->n_ssids = 1;
+ creq->scan_start = jiffies;
/* translate "Scan on frequencies" request */
i = 0;
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 6f39cb8..dde1d47 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -138,6 +138,7 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
request->wdev = wdev;
request->wiphy = &rdev->wiphy;
+ request->scan_start = jiffies;
rdev->scan_req = request;
--
1.7.0.2
On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote:
> + if (associated && !tx_empty) {
> + if (unlikely(local->scan_req->flags &
> + CFG80211_SCAN_FLAG_LOW_PRIORITY))
I don't really see value in the "unlikely()" here, that just clutters
the code and probably has very little effect on the runtime behaviour,
this is very infrequently executed.
> + case SCAN_ABORT:
> + aborted = true;
> + goto out_complete;
Maybe we should have different flags though ... I mean, this is the
first implementation that I hear of that interprets a background scan as
"ok to abort at any time"? It seems very unlikely that other
implementations would do that. I *think* (like I said before, I don't
really know) that ours (Intel's) will just shorten the dwell time, or
similar instead.
johannes
On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote:
> From: Sam Leffler <[email protected]>
>
> Add a flags word to direct and scheduled scan requests; it will
> be used for control of optional behaviours such as flushing the
> bss cache prior to doing a scan.
Why for scheduled scan as well?
> + * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32)
One thing that might be useful is to advertise which flags are even
supported at all by a driver, if we add different ones? We might then
ignore the flags that we don't support anyway, but at least userspace
would know that it can't expect flushing (for example) on an older
kernel version and might have to use some workarounds or whatever.
> +/**
> + * enum nl80211_scan_flags - scan request control flags
> + *
> + * Scan request control flags are used to control the handling
> + * of NL80211_CMD_TRIGGER_SCAN and NL80211_CMD_START_SCHED_SCAN
> + * requests.
> + */
> +enum nl80211_scan_flags {
> +};
> +
> /**
> + * enum cfg80211_scan_flags - scan request control flags
> + */
> +enum cfg80211_scan_flags {
> +};
That doesn't make a lot of sense? A single enum seems sufficient?
> + nla_put_u32(msg, NL80211_ATTR_SCAN_FLAGS, req->flags);
Missing error check, also, is there nothing that re-publishes
information about scheduled scans?
johannes
On Fri, Sep 28, 2012 at 4:01 AM, Johannes Berg
<[email protected]> wrote:
> On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote:
>> From: Sam Leffler <[email protected]>
>>
>> Add a flags word to direct and scheduled scan requests; it will
>> be used for control of optional behaviours such as flushing the
>> bss cache prior to doing a scan.
>
> Why for scheduled scan as well?
Because I thought the scheduled scan mechanism is used to trigger
periodic scans w/o involving user space and so might be used to do
bgscan's. Please enlighten me.
>
>> + * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32)
>
> One thing that might be useful is to advertise which flags are even
> supported at all by a driver, if we add different ones? We might then
> ignore the flags that we don't support anyway, but at least userspace
> would know that it can't expect flushing (for example) on an older
> kernel version and might have to use some workarounds or whatever.
Yes I considered this. I don't know what the model is for nl80211 in
this regard (think we've talked about this in the past). The low
priority scan flag is somewhat advisory so applications should be able
to deal w/ it being ignored. The flush flag however may be cause some
misbehaviour--e.g. you wakeup, scan, and then act on stale results
that may cause you try and associate to an ap that's out of range in
which case you'll end up doing the equivalent to the flush in user
space to make sure the right thing happens.
>
>> +/**
>> + * enum nl80211_scan_flags - scan request control flags
>> + *
>> + * Scan request control flags are used to control the handling
>> + * of NL80211_CMD_TRIGGER_SCAN and NL80211_CMD_START_SCHED_SCAN
>> + * requests.
>> + */
>> +enum nl80211_scan_flags {
>> +};
>> +
>
>> /**
>> + * enum cfg80211_scan_flags - scan request control flags
>> + */
>> +enum cfg80211_scan_flags {
>> +};
>
> That doesn't make a lot of sense? A single enum seems sufficient?
I thought cfg80211's name space was separate from nl80211's? Was just
trying to follow what I found elsewhere...
>
>> + nla_put_u32(msg, NL80211_ATTR_SCAN_FLAGS, req->flags);
>
> Missing error check, also, is there nothing that re-publishes
> information about scheduled scans?
>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
SGkgSm9oYW5uZXMsDQoNClRoYW5rcyBmb3IgeW91ciBjb21tZW50cy4NCg0KPiA+ICAvKiBtdXN0
IGhvbGQgZGV2LT5ic3NfbG9jayEgKi8NCj4gPiAtdm9pZCBjZmc4MDIxMV9ic3NfZXhwaXJlKHN0
cnVjdCBjZmc4MDIxMV9yZWdpc3RlcmVkX2RldmljZSAqZGV2KQ0KPiA+ICtzdGF0aWMgdm9pZCBf
X2NmZzgwMjExX2Jzc19leHBpcmUoc3RydWN0IGNmZzgwMjExX3JlZ2lzdGVyZWRfZGV2aWNlICpk
ZXYsDQo+ID4gKwkJCQkgIGludCBtYXhhZ2UsIHVuc2lnbmVkIGxvbmcgZXhwaXJlX3RpbWUpDQo+
IA0KPiBJIGRvbid0IHJlYWxseSBzZWUgd2h5IHlvdSBuZWVkIG1heGFnZSBhbmQgZXhwaXJlX3Rp
bWU/DQo+IA0KPiA+ICt2b2lkIGNmZzgwMjExX2Jzc19leHBpcmUoc3RydWN0IGNmZzgwMjExX3Jl
Z2lzdGVyZWRfZGV2aWNlICpkZXYpDQo+ID4gK3sNCj4gPiArCV9fY2ZnODAyMTFfYnNzX2V4cGly
ZShkZXYsIElFRUU4MDIxMV9TQ0FOX1JFU1VMVF9FWFBJUkUsIDApOw0KPiANCj4gSGVyZSwgeW91
IGNvdWxkIGp1c3QgcGFzcyAiamlmZmllcyAtIFNDQU5fUkVTVUxUX0VYUElSRSIgYXMgdGhlIG1h
eGFnZQ0KPiBhbmQgZ2V0IHByZXR0eSBtdWNoIHRoZSBzYW1lIGJlaGF2aW91ciBhcyBiZWZvcmUg
d2l0aG91dCBhIHNlY29uZA0KPiBwYXJhbWV0ZXIuDQoNCl9fY2ZnODAyMTFfYnNzX2V4cGlyZShk
ZXYsIGppZmZpZXMgLSBJRUVFODAyMTFfU0NBTl9SRVNVTFRfRVhQSVJFKTsNCg0KVGhpcyBpcyBz
aW1pbGFyIHRvIHRoZSBjb2RlIGluIHYxIChiZWxvdyk6DQoNCl9fY2ZnODAyMTFfYnNzX2V4cGly
ZShyZGV2LCBqaWZmaWVzIC0gcmVxdWVzdC0+c2Nhbl9zdGFydCk7DQoNCkl0IG1heSBzdGlsbCBo
YXZlIHRoZSBzbWFsbCByYWNlIGNvbmRpdGlvbiB5b3UgY29tbWVudGVkIGluIHYxPw0KUGxlYXNl
IGxldCBtZSBrbm93IGlmIEkgbWlzdW5kZXJzdG9vZCBpdC4NCg0KVGhhbmtzLA0KQmluZw0K
From: Sam Leffler <[email protected]>
Use NL80211_SCAN_FLAG_LOW_PRIORITY flag in mac80211's scan state
machine to prematurely terminate scan operations if outbound
traffic collides. This is useful for marking background scans so
they don't affect throughput.
Signed-off-by: Sam Leffler <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/scan.c | 22 ++++++++++++++++++----
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 8c80455..5113a4c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -821,6 +821,7 @@ enum {
* @SCAN_SUSPEND: Suspend the scan and go back to operating channel to
* send out data
* @SCAN_RESUME: Resume the scan and scan the next channel
+ * @SCAN_ABORT: Abort the scan and go back to operating channel
*/
enum mac80211_scan_state {
SCAN_DECISION,
@@ -828,6 +829,7 @@ enum mac80211_scan_state {
SCAN_SEND_PROBE,
SCAN_SUSPEND,
SCAN_RESUME,
+ SCAN_ABORT,
};
struct ieee80211_local {
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index c4cdbde..f95bc52 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -462,6 +462,7 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
sizeof(*local->hw_scan_req) +
req->n_channels * sizeof(req->channels[0]);
local->hw_scan_req->ie = ies;
+ local->hw_scan_req->flags = req->flags;
local->hw_scan_band = 0;
@@ -562,6 +563,7 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
unsigned long min_beacon_int = 0;
struct ieee80211_sub_if_data *sdata;
struct ieee80211_channel *next_chan;
+ enum mac80211_scan_state next_scan_state;
/*
* check if at least one STA interface is associated,
@@ -620,10 +622,19 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
usecs_to_jiffies(min_beacon_int * 1024) *
local->hw.conf.listen_interval);
- if (associated && (!tx_empty || bad_latency || listen_int_exceeded))
- local->next_scan_state = SCAN_SUSPEND;
- else
- local->next_scan_state = SCAN_SET_CHANNEL;
+ if (associated && !tx_empty) {
+ if (unlikely(local->scan_req->flags &
+ CFG80211_SCAN_FLAG_LOW_PRIORITY))
+ next_scan_state = SCAN_ABORT;
+ else
+ next_scan_state = SCAN_SUSPEND;
+ } else if (associated && (bad_latency || listen_int_exceeded)) {
+ next_scan_state = SCAN_SUSPEND;
+ } else {
+ next_scan_state = SCAN_SET_CHANNEL;
+ }
+
+ local->next_scan_state = next_scan_state;
*next_delay = 0;
}
@@ -794,6 +805,9 @@ void ieee80211_scan_work(struct work_struct *work)
case SCAN_RESUME:
ieee80211_scan_state_resume(local, &next_delay);
break;
+ case SCAN_ABORT:
+ aborted = true;
+ goto out_complete;
}
} while (next_delay == 0);
--
1.7.0.2
From: Sam Leffler <[email protected]>
Add a flags word to direct and scheduled scan requests; it will
be used for control of optional behaviours such as flushing the
bss cache prior to doing a scan.
Signed-off-by: Sam Leffler <[email protected]>
Tested-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
include/linux/nl80211.h | 14 ++++++++++++++
include/net/cfg80211.h | 10 ++++++++++
net/wireless/nl80211.c | 11 +++++++++++
3 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 7df9b50..109f75d 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1273,6 +1273,8 @@ enum nl80211_commands {
* the connection request from a station. nl80211_connect_failed_reason
* enum has different reasons of connection failure.
*
+ * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32)
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1530,6 +1532,8 @@ enum nl80211_attrs {
NL80211_ATTR_CONN_FAILED_REASON,
+ NL80211_ATTR_SCAN_FLAGS,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -3069,4 +3073,14 @@ enum nl80211_connect_failed_reason {
NL80211_CONN_FAIL_BLOCKED_CLIENT,
};
+/**
+ * enum nl80211_scan_flags - scan request control flags
+ *
+ * Scan request control flags are used to control the handling
+ * of NL80211_CMD_TRIGGER_SCAN and NL80211_CMD_START_SCHED_SCAN
+ * requests.
+ */
+enum nl80211_scan_flags {
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ab78b53..2c0c14a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -992,6 +992,12 @@ struct cfg80211_ssid {
};
/**
+ * enum cfg80211_scan_flags - scan request control flags
+ */
+enum cfg80211_scan_flags {
+};
+
+/**
* struct cfg80211_scan_request - scan request description
*
* @ssids: SSIDs to scan for (active scan only)
@@ -1000,6 +1006,7 @@ struct cfg80211_ssid {
* @n_channels: total number of channels to scan
* @ie: optional information element(s) to add into Probe Request or %NULL
* @ie_len: length of ie in octets
+ * @flags: bit field of flags controlling operation
* @rates: bitmap of rates to advertise for each band
* @wiphy: the wiphy this was for
* @wdev: the wireless device to scan for
@@ -1012,6 +1019,7 @@ struct cfg80211_scan_request {
u32 n_channels;
const u8 *ie;
size_t ie_len;
+ u32 flags;
u32 rates[IEEE80211_NUM_BANDS];
@@ -1044,6 +1052,7 @@ struct cfg80211_match_set {
* @interval: interval between each scheduled scan cycle
* @ie: optional information element(s) to add into Probe Request or %NULL
* @ie_len: length of ie in octets
+ * @flags: bit field of flags controlling operation
* @match_sets: sets of parameters to be matched for a scan result
* entry to be considered valid and to be passed to the host
* (others are filtered out).
@@ -1061,6 +1070,7 @@ struct cfg80211_sched_scan_request {
u32 interval;
const u8 *ie;
size_t ie_len;
+ u32 flags;
struct cfg80211_match_set *match_sets;
int n_match_sets;
s32 rssi_thold;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f1047ae..e4312f6 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -355,6 +355,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_BG_SCAN_PERIOD] = { .type = NLA_U16 },
[NL80211_ATTR_WDEV] = { .type = NLA_U64 },
[NL80211_ATTR_USER_REG_HINT_TYPE] = { .type = NLA_U32 },
+ [NL80211_ATTR_SCAN_FLAGS] = { .type = NLA_U32 },
};
/* policy for the key attributes */
@@ -4337,6 +4338,10 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
}
}
+ if (info->attrs[NL80211_ATTR_SCAN_FLAGS])
+ request->flags = nla_get_u32(
+ info->attrs[NL80211_ATTR_SCAN_FLAGS]);
+
request->no_cck =
nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]);
@@ -4568,6 +4573,10 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
request->ie_len);
}
+ if (info->attrs[NL80211_ATTR_SCAN_FLAGS])
+ request->flags = nla_get_u32(
+ info->attrs[NL80211_ATTR_SCAN_FLAGS]);
+
request->dev = dev;
request->wiphy = &rdev->wiphy;
request->interval = interval;
@@ -7622,6 +7631,8 @@ static int nl80211_add_scan_req(struct sk_buff *msg,
nla_put(msg, NL80211_ATTR_IE, req->ie_len, req->ie))
goto nla_put_failure;
+ nla_put_u32(msg, NL80211_ATTR_SCAN_FLAGS, req->flags);
+
return 0;
nla_put_failure:
return -ENOBUFS;
--
1.7.0.2
PiBPbiBUaHUsIDIwMTItMDktMjcgYXQgMTE6NTkgLTA3MDAsIEJpbmcgWmhhbyB3cm90ZToNCj4g
PiBGcm9tOiBBbWl0a3VtYXIgS2Fyd2FyIDxha2Fyd2FyQG1hcnZlbGwuY29tPg0KPiA+DQo+ID4g
VGhlIGZsYWcgZ2l2ZXMgaW5mb3JtYXRpb24gYWJvdXQgc2NhbiBwcmlvcml0eS4gVGhpcyBwYXRj
aCBhZGRzDQo+ID4gY2hlY2tzIGluIHRoZSBjb2RlIHRvIGNhbmNlbC9hYm9ydCBzY2FuIG9wZXJh
dGlvbiBvbmx5IGZvciBsb3cNCj4gPiBwcmlvcml0eSBzY2FuIHJlcXVlc3RzLg0KPiANCj4gSSBk
b24ndCB0YWtlIG13aWZpZXggcGF0Y2hlcywgc28gc2VuZGluZyB0aGlzIGluIHRoZSBzYW1lIHNl
cmllcyBpc24ndA0KPiBhbGwgdGhhdCB1c2VmdWwuDQo+IA0KPiBMZXQncyBzb3J0IG91dCB0aGUg
b3RoZXIgc3R1ZmYgZmlyc3QsIGFuZCB0aGVuIHlvdSBjYW4gaW1wbGVtZW50IGl0IGluDQo+IHRo
ZSBkcml2ZXIuIDopDQoNClRoYXQgbWFrZXMgc2Vuc2UuIFdlIHdpbGwgd29yayBvbiB0aGUgY2Zn
fG5sfG1hYzgwMjExIHN0dWZmIGZpcnN0Lg0KDQpUaGFua3MsDQpCaW5nDQo=
On Fri, 2012-09-28 at 12:47 -0700, Bing Zhao wrote:
> > Here, you could just pass "jiffies - SCAN_RESULT_EXPIRE" as the maxage
> > and get pretty much the same behaviour as before without a second
> > parameter.
>
> __cfg80211_bss_expire(dev, jiffies - IEEE80211_SCAN_RESULT_EXPIRE);
>
> This is similar to the code in v1 (below):
>
> __cfg80211_bss_expire(rdev, jiffies - request->scan_start);
Well, they look similar, but these are different calculations. The first
yields an absolute time, while the second yields a relative time. With
these, say we call them T_abs and T_rel, you'd do the following
comparisons
remove = (timestamp < T_abs)
or
remove = (timestamp < jiffies - T_rel)
respectively. The "problem" that I pointed out is that in the latter
case jiffies keeps running, so you would really have (resolving T_rel)
remove = (timestamp < jiffies_1 - (jiffies_2 - scan_start))
= (timestamp < jiffies_1 - jiffies_2 + scan_start)
when you really want
remove = timestamp < scan_start
If jiffies_1 == jiffies_2, then the two are the same, obviously, but
since you're doing calculations against a running counter it's not
guaranteed. With the absolute timestamp based calculations (T_abs above)
you don't run into that problem.
> It may still have the small race condition you commented in v1?
> Please let me know if I misunderstood it.
Well I should also say that the race condition is really fairly
theoretical, unless you have a very small HZ and calculate T_rel just
before the roll-over, in which case jiffies_1 and jiffies_2 above could
actually differ by a fair amount (tens of milliseconds in wall time.)
Regardless, I think if we're going to have absolute time here in any way
(today we only have relative time for age-based expiry) we should do
calculations and comparisons in absolute rather than relative time.
Hth!
johannes
On Fri, 2012-09-28 at 13:36 -0700, Sam Leffler wrote:
> On Fri, Sep 28, 2012 at 4:01 AM, Johannes Berg
> <[email protected]> wrote:
> > On Thu, 2012-09-27 at 11:59 -0700, Bing Zhao wrote:
> >> From: Sam Leffler <[email protected]>
> >>
> >> Add a flags word to direct and scheduled scan requests; it will
> >> be used for control of optional behaviours such as flushing the
> >> bss cache prior to doing a scan.
> >
> > Why for scheduled scan as well?
>
> Because I thought the scheduled scan mechanism is used to trigger
> periodic scans w/o involving user space and so might be used to do
> bgscan's. Please enlighten me.
It could be used for that, but I don't see any handling for scheduled
scans in this code. And this also goes into the question about
advertising the features, since scheduled scan is always implemented by
the driver -- there's little value in implementing it in mac80211 since
then it's software anyway and wpa_s can just do the triggering. Then the
question will be: does the driver support it or not, and should we
advertise whether it's supported or not.
Also for scheduled scan, I'm not sure how the flushing would work? You
don't seem to have implemented that.
> >> + * @NL80211_ATTR_SCAN_FLAGS: scan request control flags (u32)
> >
> > One thing that might be useful is to advertise which flags are even
> > supported at all by a driver, if we add different ones? We might then
> > ignore the flags that we don't support anyway, but at least userspace
> > would know that it can't expect flushing (for example) on an older
> > kernel version and might have to use some workarounds or whatever.
>
> Yes I considered this. I don't know what the model is for nl80211 in
> this regard (think we've talked about this in the past). The low
> priority scan flag is somewhat advisory so applications should be able
> to deal w/ it being ignored. The flush flag however may be cause some
> misbehaviour--e.g. you wakeup, scan, and then act on stale results
> that may cause you try and associate to an ap that's out of range in
> which case you'll end up doing the equivalent to the flush in user
> space to make sure the right thing happens.
I'd say there are different possible implementations here. One way would
be to advertise the nl80211 scan flags attribute with the device
information and set all the supported bits, at least the ones that are
important like the flush one, and document the advisory ones as such.
For other more generic features we'd probably use nl80211_feature_flags,
but here I'd suggest the above, putting the attribute into the device
information with the supported bits set, which today would be only the
flush one which is handled in cfg80211 so we don't (yet) need driver
information for it if low-prio is advisory.
There's one complication though with scheduled scan, since that'd
require a different feature attribute unless we want to mandate that all
flags must always be supported in both or neither, which I'm not sure is
realistic.
How if "flush" supposed to behave in scheduled scan anyway?
> >> +/**
> >> + * enum nl80211_scan_flags - scan request control flags
> >> + * enum cfg80211_scan_flags - scan request control flags
> > That doesn't make a lot of sense? A single enum seems sufficient?
>
> I thought cfg80211's name space was separate from nl80211's? Was just
> trying to follow what I found elsewhere...
Yeah there are a few (old) examples of this (e.g. band enum), but they
aren't really separate any more, we have the namespaces more like this
(in maths/latex notation):
nl80211 $\subset$ cfg80211 $\subset$ mac80211
At some point maybe I'll clean up the old examples. I think there might
also be a few where the cfg80211 value is BIT() of the nl80211 value,
but I'm not really sure.
johannes
On Fri, 2012-09-28 at 13:38 -0700, Sam Leffler wrote:
> > Maybe we should have different flags though ... I mean, this is the
> > first implementation that I hear of that interprets a background scan as
> > "ok to abort at any time"? It seems very unlikely that other
> > implementations would do that. I *think* (like I said before, I don't
> > really know) that ours (Intel's) will just shorten the dwell time, or
> > similar instead.
>
> Well previous implementations I've done have used this technique for
> many years and you can find them in various products (assuming they
> haven't been totally rewritten) :-)
:-)
You convinced me before though with the abort/not-abort notification
being sufficient, so I think we can do with the low-priority flag and
any kind of implementation.
johannes