Hi,
This patchset adds a new scan parameter. It allows
specyfing for how long each channel should be
visited during scan.
I've tested this with ath9k (which doesn't have
hw_scan) and ath10k (does have hw_scan). It's not
super time-precise, but seems reasonable.
The main motivation behind this is to allow easier
ACS implementation in hostapd.
This could also perhaps be useful for spectral
scan and anyone interested in having ability to
tune scan times (e.g. very fast initial channel
scanning on mobile devices, long passive scans).
I'm wondering if it would be okay to set swscan
probe delay to 0 if chan_time is non-zero? Or does
probe delay need to be non-zero? Or maybe it
should also be configurable via a parameter?
Pozdrawiam / Best regards,
Michal Kazior.
Michal Kazior (3):
nl/cfg80211: add chan_time for scan request
mac80211: add support for scan chan_time parameter
ath10k: respect chan_time parameter in scan request
drivers/net/wireless/ath/ath10k/mac.c | 6 ++++++
include/net/cfg80211.h | 3 +++
include/uapi/linux/nl80211.h | 7 +++++++
net/mac80211/scan.c | 22 ++++++++++++++++++++--
net/wireless/nl80211.c | 4 ++++
5 files changed, 40 insertions(+), 2 deletions(-)
--
1.7.9.5
Respect the new scan request parameter.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index da5c333..cd20a20 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2150,6 +2150,12 @@ static int ath10k_hw_scan(struct ieee80211_hw *hw,
arg.vdev_id = arvif->vdev_id;
arg.scan_id = ATH10K_SCAN_ID;
+ if (req->chan_time) {
+ arg.dwell_time_active = req->chan_time;
+ arg.dwell_time_passive = req->chan_time;
+ arg.max_scan_time = 2 * req->chan_time * req->n_channels;
+ }
+
if (!req->no_cck)
arg.scan_ctrl_flags |= WMI_SCAN_ADD_CCK_RATES;
--
1.7.9.5
This allows mac80211-based drivers that support
hw_scan() to get chan_time paremter.
This also makes mac80211-based drivers that don't
support hw_scan() to respect the chan_time
parameter.
The effective minimum channel time for swscan
drivers is the probe delay. hw_scan drivers may
have their own limits.
Signed-off-by: Michal Kazior <[email protected]>
---
net/mac80211/scan.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 08afe74..e8b1aca 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -443,7 +443,18 @@ static void ieee80211_scan_state_send_probe(struct ieee80211_local *local,
* After sending probe requests, wait for probe responses
* on the channel.
*/
- *next_delay = IEEE80211_CHANNEL_TIME;
+ if (local->scan_req->chan_time) {
+ /*
+ * Account probe delay to compute the delay needed to reach
+ * chan_time on a given channel.
+ */
+ *next_delay = msecs_to_jiffies(local->scan_req->chan_time);
+ if (*next_delay > IEEE80211_PROBE_DELAY)
+ *next_delay -= IEEE80211_PROBE_DELAY;
+ else
+ *next_delay = 0;
+ } else
+ *next_delay = IEEE80211_CHANNEL_TIME;
local->next_scan_state = SCAN_DECISION;
}
@@ -485,6 +496,7 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
req->n_channels * sizeof(req->channels[0]);
local->hw_scan_req->ie = ies;
local->hw_scan_req->flags = req->flags;
+ local->hw_scan_req->chan_time = req->chan_time;
local->hw_scan_band = 0;
@@ -532,6 +544,9 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
next_delay = IEEE80211_CHANNEL_TIME;
}
+ if (req->chan_time)
+ next_delay = msecs_to_jiffies(req->chan_time);
+
/* Now, just wait a bit and we are all done! */
ieee80211_queue_delayed_work(&local->hw, &local->scan_work,
next_delay);
@@ -696,7 +711,10 @@ static void ieee80211_scan_state_set_channel(struct ieee80211_local *local,
*/
if (chan->flags & IEEE80211_CHAN_PASSIVE_SCAN ||
!local->scan_req->n_ssids) {
- *next_delay = IEEE80211_PASSIVE_CHANNEL_TIME;
+ if (local->scan_req->chan_time)
+ *next_delay = msecs_to_jiffies(local->scan_req->chan_time);
+ else
+ *next_delay = IEEE80211_PASSIVE_CHANNEL_TIME;
local->next_scan_state = SCAN_DECISION;
return;
}
--
1.7.9.5
The new scan parameter allows user to specify how
long driver should remain on each scanned channel.
The default is to leave it up to driver to decide.
If specified it is preferred for driver to respect
this setting although it may ignore it if it can't
support it.
This can be useful to do very quick/slow scans
and/or gether survey data, e.g. for automatic
channel selection.
Signed-off-by: Michal Kazior <[email protected]>
---
include/net/cfg80211.h | 3 +++
include/uapi/linux/nl80211.h | 7 +++++++
net/wireless/nl80211.c | 4 ++++
3 files changed, 14 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index aeaf6df..2d46112 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1323,6 +1323,8 @@ struct cfg80211_ssid {
* @aborted: (internal) scan request was notified as aborted
* @notified: (internal) scan request was notified as done or aborted
* @no_cck: used to send probe requests at non CCK rate in 2GHz band
+ * @chan_time: how many msec driver should stay on each channel during scan.
+ * Setting 0 leaves the decision up to the driver.
*/
struct cfg80211_scan_request {
struct cfg80211_ssid *ssids;
@@ -1332,6 +1334,7 @@ struct cfg80211_scan_request {
const u8 *ie;
size_t ie_len;
u32 flags;
+ int chan_time;
u32 rates[IEEE80211_NUM_BANDS];
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index eb68735..4ea1d9b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1469,6 +1469,11 @@ enum nl80211_commands {
*
* @NL80211_ATTR_COALESCE_RULE: Coalesce rule information.
*
+ * @NL80211_ATTR_SCAN_CHAN_TIME: Specifies how many msec should a driver spend
+ * on each channel during scanning. This is optional and the default is
+ * leave the decision up to the driver. This setting may, but preferrably
+ * shouldn't, be ignored by driver.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1771,6 +1776,8 @@ enum nl80211_attrs {
NL80211_ATTR_COALESCE_RULE,
+ NL80211_ATTR_SCAN_CHAN_TIME,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 03d4ef9..27a8fef 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5298,6 +5298,10 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
request->no_cck =
nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]);
+ if (info->attrs[NL80211_ATTR_SCAN_CHAN_TIME])
+ request->chan_time =
+ nla_get_u32(info->attrs[NL80211_ATTR_SCAN_CHAN_TIME]);
+
request->wdev = wdev;
request->wiphy = &rdev->wiphy;
request->scan_start = jiffies;
--
1.7.9.5
On 2 August 2013 13:25, Johannes Berg <[email protected]> wrote:
> On Thu, 2013-08-01 at 11:14 +0200, Michal Kazior wrote:
>
>> > This seems a bit iffy - you don't differentiate between active/passive
>> > scans?
>>
>> Is there a reason this should be differentiated?
>
> Well they don't usually have the same timing, passive channel dwell time
> usually needs to be longer than on active channels.
>
>> > This also interferes a bit with some other scan optimisations that could
>> > be done at a low firmware level, so I think we should be careful and
>> > actually say that this is really more intended for measurement use cases
>> > and not for normal scans?
>> >
>> > Or maybe we should have a separate measurement command with similar
>> > semantics? This all doesn't seem very clear to me yet :)
>>
>> Motivation behind this patchset is to simplify ACS implementation and
>> depend on scan. This also allows easier fallback from survey-based ACS
>> to BSS-based one.
>
> Sure, I understand.
>
>> Other use cases are a side-effect so perhaps clarifying the intent in
>> the docs is enough.
>
> I'm just saying that this is a fundamentally different usage than actual
> scanning. Let's say for the sake of the argument I find a way to scan
> channels quicker and implement that in my device (hw_scan). I don't
> think this is too far-fetched, imagine I have an 80MHz capable device
> and can send probe request 4x transmissions on all the subchannels, if
> somehow I manage to separate the receive then I could scan 4x as
> quickly. I don't think this particular thing is possible but I could
> imagine scan optimisations like it.
>
> Now this comes along - and suddenly the API requires that you stay on
> each specified channel for the given period of time. Any such
> "non-traditional" scan optimisations would have to be given up, so in a
> sense this fundamentally alters the semantics of the scan primitive,
> where before it was "give me the list of networks (with these SSIDs) on
> these channels" it now becomes "go to each channel, stay there for this
> amount of time and ..."
>
> That's what I'm worried about - the implicit semantic change here.
I think I understand your point now.
I'm thinking of using series of passive scans for ACS now, so perhaps
the whole chan_time stuff is unnecessary. What are your thoughts about
it?
Pozdrawiam / Best regards,
Michał Kazior.
On Thu, 2013-08-01 at 11:14 +0200, Michal Kazior wrote:
> > This seems a bit iffy - you don't differentiate between active/passive
> > scans?
>
> Is there a reason this should be differentiated?
Well they don't usually have the same timing, passive channel dwell time
usually needs to be longer than on active channels.
> > This also interferes a bit with some other scan optimisations that could
> > be done at a low firmware level, so I think we should be careful and
> > actually say that this is really more intended for measurement use cases
> > and not for normal scans?
> >
> > Or maybe we should have a separate measurement command with similar
> > semantics? This all doesn't seem very clear to me yet :)
>
> Motivation behind this patchset is to simplify ACS implementation and
> depend on scan. This also allows easier fallback from survey-based ACS
> to BSS-based one.
Sure, I understand.
> Other use cases are a side-effect so perhaps clarifying the intent in
> the docs is enough.
I'm just saying that this is a fundamentally different usage than actual
scanning. Let's say for the sake of the argument I find a way to scan
channels quicker and implement that in my device (hw_scan). I don't
think this is too far-fetched, imagine I have an 80MHz capable device
and can send probe request 4x transmissions on all the subchannels, if
somehow I manage to separate the receive then I could scan 4x as
quickly. I don't think this particular thing is possible but I could
imagine scan optimisations like it.
Now this comes along - and suddenly the API requires that you stay on
each specified channel for the given period of time. Any such
"non-traditional" scan optimisations would have to be given up, so in a
sense this fundamentally alters the semantics of the scan primitive,
where before it was "give me the list of networks (with these SSIDs) on
these channels" it now becomes "go to each channel, stay there for this
amount of time and ..."
That's what I'm worried about - the implicit semantic change here.
johannes
On Mon, 2013-07-29 at 10:39 +0200, Michal Kazior wrote:
> + * @NL80211_ATTR_SCAN_CHAN_TIME: Specifies how many msec should a driver spend
> + * on each channel during scanning. This is optional and the default is
> + * leave the decision up to the driver. This setting may, but preferrably
typo: preferably :)
> + * shouldn't, be ignored by driver.
This seems a bit iffy - you don't differentiate between active/passive
scans?
Also maybe there should be a bit saying "I support scan timing" or even
the min/max times?
This also interferes a bit with some other scan optimisations that could
be done at a low firmware level, so I think we should be careful and
actually say that this is really more intended for measurement use cases
and not for normal scans?
Or maybe we should have a separate measurement command with similar
semantics? This all doesn't seem very clear to me yet :)
johannes
Hi Johannes and Michal ,
In this context I would also like to mention that there are also some
use cases ( Ex WiFi Positioning) , where good performance shall
result from using signals from as many Access Points as possible, a
fixed dwell / chan time in the host driver shall result in the poor
discovery efficiency of the Access Points,negatively impacting WiFi
positioning performance.
I would say such applications shall benefit by configuring a chan (dwell)_time.
Also,an attribute corresponding to the mode of the scan ( active /
passive ) , along with chan_time would benefit such applications with
more configurable options. Isn't it ?
Please correct me .
Regards,
Sunil
On Fri, Aug 2, 2013 at 6:00 PM, Sunil Dutt <[email protected]> wrote:
> Hi Johannes and Michal ,
> In this context I would also like to mention that there are also some use
> cases ( Ex WiFi Positioning) , where good performance shall result from
> using signals from as many Access Points as possible, a fixed dwell / chan
> time in the host driver shall result in the poor discovery efficiency of the
> Access Points,negatively impacting WiFi positioning performance.
>
> I would say such applications shall benefit by configuring a chan
> (dwell)_time.
> Also,an attribute corresponding to the mode of the scan ( active / passive )
> , along with chan_time would benefit such applications with more
> configurable options. Isn't it ?
> Please correct me .
>
> Regards,
> Sunil
>
>
>
>
> On Fri, Aug 2, 2013 at 4:55 PM, Johannes Berg <[email protected]>
> wrote:
>>
>> On Thu, 2013-08-01 at 11:14 +0200, Michal Kazior wrote:
>>
>> > > This seems a bit iffy - you don't differentiate between active/passive
>> > > scans?
>> >
>> > Is there a reason this should be differentiated?
>>
>> Well they don't usually have the same timing, passive channel dwell time
>> usually needs to be longer than on active channels.
>>
>> > > This also interferes a bit with some other scan optimisations that
>> > > could
>> > > be done at a low firmware level, so I think we should be careful and
>> > > actually say that this is really more intended for measurement use
>> > > cases
>> > > and not for normal scans?
>> > >
>> > > Or maybe we should have a separate measurement command with similar
>> > > semantics? This all doesn't seem very clear to me yet :)
>> >
>> > Motivation behind this patchset is to simplify ACS implementation and
>> > depend on scan. This also allows easier fallback from survey-based ACS
>> > to BSS-based one.
>>
>> Sure, I understand.
>>
>> > Other use cases are a side-effect so perhaps clarifying the intent in
>> > the docs is enough.
>>
>> I'm just saying that this is a fundamentally different usage than actual
>> scanning. Let's say for the sake of the argument I find a way to scan
>> channels quicker and implement that in my device (hw_scan). I don't
>> think this is too far-fetched, imagine I have an 80MHz capable device
>> and can send probe request 4x transmissions on all the subchannels, if
>> somehow I manage to separate the receive then I could scan 4x as
>> quickly. I don't think this particular thing is possible but I could
>> imagine scan optimisations like it.
>>
>> Now this comes along - and suddenly the API requires that you stay on
>> each specified channel for the given period of time. Any such
>> "non-traditional" scan optimisations would have to be given up, so in a
>> sense this fundamentally alters the semantics of the scan primitive,
>> where before it was "give me the list of networks (with these SSIDs) on
>> these channels" it now becomes "go to each channel, stay there for this
>> amount of time and ..."
>>
>> That's what I'm worried about - the implicit semantic change here.
>>
>> 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
>
>
On 1 August 2013 09:40, Johannes Berg <[email protected]> wrote:
> On Mon, 2013-07-29 at 10:39 +0200, Michal Kazior wrote:
>
>> + * @NL80211_ATTR_SCAN_CHAN_TIME: Specifies how many msec should a driver spend
>> + * on each channel during scanning. This is optional and the default is
>> + * leave the decision up to the driver. This setting may, but preferrably
>
> typo: preferably :)
>
>> + * shouldn't, be ignored by driver.
>
> This seems a bit iffy - you don't differentiate between active/passive
> scans?
Is there a reason this should be differentiated?
> Also maybe there should be a bit saying "I support scan timing" or even
> the min/max times?
Sounds good. I can add it.
> This also interferes a bit with some other scan optimisations that could
> be done at a low firmware level, so I think we should be careful and
> actually say that this is really more intended for measurement use cases
> and not for normal scans?
>
> Or maybe we should have a separate measurement command with similar
> semantics? This all doesn't seem very clear to me yet :)
Motivation behind this patchset is to simplify ACS implementation and
depend on scan. This also allows easier fallback from survey-based ACS
to BSS-based one.
Other use cases are a side-effect so perhaps clarifying the intent in
the docs is enough.
Pozdrawiam / Best regards,
Micha? Kazior.
> > That's what I'm worried about - the implicit semantic change here.
>
> I think I understand your point now.
>
> I'm thinking of using series of passive scans for ACS now, so perhaps
> the whole chan_time stuff is unnecessary. What are your thoughts about
> it?
Well, still then you're relying on the fact that passive channels scans
will stick to each channel for ~110ms or so. This is true today, but
it's not really specified.
I don't think that using the scan command would be a big deal, but maybe
this 'measurement' behaviour should be explicitly enabled? Maybe other
devices even have to do something special to collect the data.
johannes
On Mon, 2013-07-29 at 10:39 +0200, Michal Kazior wrote:
> I'm wondering if it would be okay to set swscan
> probe delay to 0 if chan_time is non-zero? Or does
> probe delay need to be non-zero? Or maybe it
> should also be configurable via a parameter?
probe delay needs to be non-zero for NAV adjustment.
johannes