In order to give the usermode an ability to configure
scan channel times, and as it required for the beacon
reports in the 802.11k standard.
Add to the scan command min/max channel times. Add
min/max passive channel times, since a single scan
can contain both passive and active channels due to
regulatory constraints.
Signed-off-by: Victor Goldenshtein <[email protected]>
---
v3:
Redefine default scan times in milliseconds instead of jiffies.
New ieee80211_msec_to_tu() helper function.
include/linux/ieee80211.h | 9 +++++++++
include/linux/nl80211.h | 26 ++++++++++++++++++++++++++
include/net/cfg80211.h | 11 +++++++++++
net/wireless/core.h | 1 +
net/wireless/nl80211.c | 31 ++++++++++++++++++++++++++++++-
net/wireless/scan.c | 38 ++++++++++++++++++++++++++++++++++++++
6 files changed, 115 insertions(+), 1 deletions(-)
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index ce9af89..9522b7e 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1856,6 +1856,15 @@ static inline unsigned long ieee80211_tu_to_usec(unsigned long tu)
}
/**
+ * ieee80211_msec_to_tu - convert milliseconds to time units (TU)
+ * @msec: the milliseconds
+ */
+static inline unsigned long ieee80211_msec_to_tu(unsigned long msec)
+{
+ return msec * 1000 / 1024;
+}
+
+/**
* ieee80211_check_tim - check if AID bit is set in TIM
* @tim: the TIM IE
* @tim_len: length of the TIM IE
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index a6959f7..10d517b 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1222,6 +1222,24 @@ enum nl80211_commands {
* @NL80211_ATTR_BG_SCAN_PERIOD: Background scan period in seconds
* or 0 to disable background scan.
*
+ * @NL80211_ATTR_SCAN_MIN_CH_TIME: Minimum active scan time (in TUs),
+ * u32 attribute to setup minimum time to wait on each channel, if received
+ * at least one probe_resp/beacon during this period will continue waiting
+ * @NL80211_ATTR_SCAN_MAX_CH_TIME, otherwise will move to next channel.
+ * @NL80211_ATTR_SCAN_MAX_CH_TIME: Maximum active scan time (in TUs),
+ * u32 attribute to setup maximum time to wait on the channel.
+ * @NL80211_ATTR_SCAN_PSV_MIN_CH_TIME: Minimum passive scan time (in TUs),
+ * u32 attribute (similar to @NL80211_ATTR_SCAN_MIN_CH_TIME).
+ * @NL80211_ATTR_SCAN_PSV_MAX_CH_TIME: Maximum passive scan time (in TUs),
+ * u32 attribute (similar to @NL80211_ATTR_SCAN_MAX_CH_TIME).
+ * Note:
+ * The above channel time attributes are for the %NL80211_CMD_TRIGGER_SCAN
+ * command. The attributes are optional, the driver will use default
+ * channel time values if the attribute is not set.
+ * If one of the min times will be greater than max or set to zero,
+ * -EINVAL will be returned. For the software scan only the min times
+ * are relevant.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1473,6 +1491,11 @@ enum nl80211_attrs {
NL80211_ATTR_BG_SCAN_PERIOD,
+ NL80211_ATTR_SCAN_MIN_CH_TIME,
+ NL80211_ATTR_SCAN_MAX_CH_TIME,
+ NL80211_ATTR_SCAN_PSV_MIN_CH_TIME,
+ NL80211_ATTR_SCAN_PSV_MAX_CH_TIME,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -2867,11 +2890,14 @@ enum nl80211_ap_sme_features {
* @NL80211_FEATURE_HT_IBSS: This driver supports IBSS with HT datarates.
* @NL80211_FEATURE_INACTIVITY_TIMER: This driver takes care of freeing up
* the connected inactive stations in AP mode.
+ * @NL80211_FEATURE_SCAN_TIMES: This driver supports min/max scan times
+ * configuration for the %NL80211_CMD_TRIGGER_SCAN command.
*/
enum nl80211_feature_flags {
NL80211_FEATURE_SK_TX_STATUS = 1 << 0,
NL80211_FEATURE_HT_IBSS = 1 << 1,
NL80211_FEATURE_INACTIVITY_TIMER = 1 << 2,
+ NL80211_FEATURE_SCAN_TIMES = 1 << 3,
};
/**
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0289d4c..56fd3d9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -920,6 +920,12 @@ struct cfg80211_ssid {
* @dev: the interface
* @aborted: (internal) scan request was notified as aborted
* @no_cck: used to send probe requests at non CCK rate in 2GHz band
+ * @min_ch_time: minimum time to wait on each channel for active scans
+ * @max_ch_time: maximum time to wait on each channel for active scans
+ * @min_passive_ch_time: minimum time to wait on each channel for passive scans
+ * @max_passive_ch_time: maximum time to wait on each channel for passive scans
+ * Note: If the above channel times are not set, the default channel times
+ * will be used. The channel times are in TUs.
*/
struct cfg80211_scan_request {
struct cfg80211_ssid *ssids;
@@ -936,6 +942,11 @@ struct cfg80211_scan_request {
bool aborted;
bool no_cck;
+ u32 min_ch_time;
+ u32 max_ch_time;
+ u32 min_passive_ch_time;
+ u32 max_passive_ch_time;
+
/* keep last */
struct ieee80211_channel *channels[0];
};
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 8523f38..04e2cd1 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -416,6 +416,7 @@ void cfg80211_sme_scan_done(struct net_device *dev);
void cfg80211_sme_rx_auth(struct net_device *dev, const u8 *buf, size_t len);
void cfg80211_sme_disassoc(struct net_device *dev,
struct cfg80211_internal_bss *bss);
+int cfg80211_trigger_scan(struct cfg80211_registered_device *rdev);
void __cfg80211_scan_done(struct work_struct *wk);
void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak);
void __cfg80211_sched_scan_results(struct work_struct *wk);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 206465d..5058395 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -206,6 +206,10 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_NOACK_MAP] = { .type = NLA_U16 },
[NL80211_ATTR_INACTIVITY_TIMEOUT] = { .type = NLA_U16 },
[NL80211_ATTR_BG_SCAN_PERIOD] = { .type = NLA_U16 },
+ [NL80211_ATTR_SCAN_MIN_CH_TIME] = { .type = NLA_U32 },
+ [NL80211_ATTR_SCAN_MAX_CH_TIME] = { .type = NLA_U32 },
+ [NL80211_ATTR_SCAN_PSV_MIN_CH_TIME] = { .type = NLA_U32 },
+ [NL80211_ATTR_SCAN_PSV_MAX_CH_TIME] = { .type = NLA_U32 },
};
/* policy for the key attributes */
@@ -3964,6 +3968,31 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
request->ie_len);
}
+ if (info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]) {
+ request->min_ch_time =
+ nla_get_u32(info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]);
+ if (!request->min_ch_time)
+ return -EINVAL;
+ }
+ if (info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME]) {
+ request->max_ch_time =
+ nla_get_u32(info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME]);
+ if (request->min_ch_time > request->max_ch_time)
+ return -EINVAL;
+ }
+ if (info->attrs[NL80211_ATTR_SCAN_PSV_MIN_CH_TIME]) {
+ request->min_passive_ch_time =
+ nla_get_u32(info->attrs[NL80211_ATTR_SCAN_PSV_MIN_CH_TIME]);
+ if (!request->min_passive_ch_time)
+ return -EINVAL;
+ }
+ if (info->attrs[NL80211_ATTR_SCAN_PSV_MAX_CH_TIME]) {
+ request->max_passive_ch_time =
+ nla_get_u32(info->attrs[NL80211_ATTR_SCAN_PSV_MAX_CH_TIME]);
+ if (request->min_passive_ch_time > request->max_passive_ch_time)
+ return -EINVAL;
+ }
+
for (i = 0; i < IEEE80211_NUM_BANDS; i++)
if (wiphy->bands[i])
request->rates[i] =
@@ -3995,7 +4024,7 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
request->wiphy = &rdev->wiphy;
rdev->scan_req = request;
- err = rdev->ops->scan(&rdev->wiphy, dev, request);
+ err = cfg80211_trigger_scan(rdev);
if (!err) {
nl80211_send_scan_start(rdev, dev);
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index af2b1ca..45edd68 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -20,6 +20,30 @@
#define IEEE80211_SCAN_RESULT_EXPIRE (30 * HZ)
+#define IEEE80211_CH_TIME_MSEC 30
+#define IEEE80211_PSV_CH_TIME_MSEC 125
+
+int cfg80211_trigger_scan(struct cfg80211_registered_device *rdev)
+{
+ struct cfg80211_scan_request *request;
+
+ ASSERT_RDEV_LOCK(rdev);
+
+ request = rdev->scan_req;
+
+ if (!request)
+ return -EINVAL;
+
+ if (!request->min_passive_ch_time)
+ request->min_passive_ch_time =
+ ieee80211_msec_to_tu(IEEE80211_PSV_CH_TIME_MSEC);
+ if (!request->min_ch_time)
+ request->min_ch_time =
+ ieee80211_msec_to_tu(IEEE80211_CH_TIME_MSEC);
+
+ return rdev->ops->scan(&rdev->wiphy, request->dev, request);
+}
+
void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak)
{
struct cfg80211_scan_request *request;
@@ -1023,6 +1047,20 @@ int cfg80211_wext_siwscan(struct net_device *dev,
if (wiphy->bands[i])
creq->rates[i] = (1 << wiphy->bands[i]->n_bitrates) - 1;
+ if (wreq->min_channel_time) {
+ creq->min_ch_time = wreq->min_channel_time;
+ if (wreq->scan_type == IW_SCAN_TYPE_ACTIVE)
+ creq->min_passive_ch_time =
+ ieee80211_msec_to_tu(IEEE80211_PSV_CH_TIME_MSEC);
+ else
+ creq->min_passive_ch_time = wreq->min_channel_time;
+ } else {
+ creq->min_ch_time =
+ ieee80211_msec_to_tu(IEEE80211_CH_TIME_MSEC);
+ creq->min_passive_ch_time =
+ ieee80211_msec_to_tu(IEEE80211_PSV_CH_TIME_MSEC);
+ }
+
rdev->scan_req = creq;
err = rdev->ops->scan(wiphy, dev, creq);
if (err) {
--
1.7.5.4
Use the user scan times if those were set, otherwise
use the default values.
This patch handles both hw_scan and non-offload scan.
Signed-off-by: Victor Goldenshtein <[email protected]>
---
net/mac80211/scan.c | 31 ++++++++++++++++++++-----------
1 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 169da07..548b564 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -26,8 +26,6 @@
#include "mesh.h"
#define IEEE80211_PROBE_DELAY (HZ / 33)
-#define IEEE80211_CHANNEL_TIME (HZ / 33)
-#define IEEE80211_PASSIVE_CHANNEL_TIME (HZ / 8)
static void ieee80211_rx_bss_free(struct cfg80211_bss *cbss)
{
@@ -421,7 +419,7 @@ 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;
+ *next_delay = TU_TO_JIFFIES(local->scan_req->min_ch_time);
local->next_scan_state = SCAN_DECISION;
}
@@ -456,6 +454,13 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
local->hw_scan_req->ssids = req->ssids;
local->hw_scan_req->n_ssids = req->n_ssids;
+ local->hw_scan_req->max_ch_time = req->max_ch_time;
+ local->hw_scan_req->min_ch_time = req->min_ch_time;
+ local->hw_scan_req->max_passive_ch_time =
+ req->max_passive_ch_time;
+ local->hw_scan_req->min_passive_ch_time =
+ req->min_passive_ch_time;
+
ies = (u8 *)local->hw_scan_req +
sizeof(*local->hw_scan_req) +
req->n_channels * sizeof(req->channels[0]);
@@ -502,10 +507,10 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
if ((req->channels[0]->flags &
IEEE80211_CHAN_PASSIVE_SCAN) ||
!local->scan_req->n_ssids) {
- next_delay = IEEE80211_PASSIVE_CHANNEL_TIME;
+ next_delay = TU_TO_JIFFIES(req->min_passive_ch_time);
} else {
ieee80211_scan_state_send_probe(local, &next_delay);
- next_delay = IEEE80211_CHANNEL_TIME;
+ next_delay = TU_TO_JIFFIES(req->min_ch_time);
}
/* Now, just wait a bit and we are all done! */
@@ -540,15 +545,16 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
}
static unsigned long
-ieee80211_scan_get_channel_time(struct ieee80211_channel *chan)
+ieee80211_scan_get_channel_time(struct cfg80211_scan_request *scan_req,
+ struct ieee80211_channel *chan)
{
/*
* TODO: channel switching also consumes quite some time,
* add that delay as well to get a better estimation
*/
if (chan->flags & IEEE80211_CHAN_PASSIVE_SCAN)
- return IEEE80211_PASSIVE_CHANNEL_TIME;
- return IEEE80211_PROBE_DELAY + IEEE80211_CHANNEL_TIME;
+ return TU_TO_JIFFIES(scan_req->min_passive_ch_time);
+ return IEEE80211_PROBE_DELAY + TU_TO_JIFFIES(scan_req->min_ch_time);
}
static void ieee80211_scan_state_decision(struct ieee80211_local *local,
@@ -609,12 +615,14 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
*/
bad_latency = time_after(jiffies +
- ieee80211_scan_get_channel_time(next_chan),
+ ieee80211_scan_get_channel_time(local->scan_req,
+ next_chan),
local->leave_oper_channel_time +
usecs_to_jiffies(pm_qos_request(PM_QOS_NETWORK_LATENCY)));
listen_int_exceeded = time_after(jiffies +
- ieee80211_scan_get_channel_time(next_chan),
+ ieee80211_scan_get_channel_time(local->scan_req,
+ next_chan),
local->leave_oper_channel_time +
usecs_to_jiffies(min_beacon_int * 1024) *
local->hw.conf.listen_interval);
@@ -662,7 +670,8 @@ 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;
+ *next_delay =
+ TU_TO_JIFFIES(local->scan_req->min_passive_ch_time);
local->next_scan_state = SCAN_DECISION;
return;
}
--
1.7.5.4
> + * @NL80211_ATTR_SCAN_PSV_MAX_CH_TIME: Maximum passive scan time (in TUs),
> + * u32 attribute (similar to @NL80211_ATTR_SCAN_MAX_CH_TIME).
> + * Note:
> + * The above channel time attributes are for the %NL80211_CMD_TRIGGER_SCAN
> + * command. The attributes are optional, the driver will use default
> + * channel time values if the attribute is not set.
> + * If one of the min times will be greater than max or set to zero,
> + * -EINVAL will be returned. For the software scan only the min times
> + * are relevant.
What if the driver doesn't support the timings?
> + if (info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]) {
> + if (info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME]) {
> + if (info->attrs[NL80211_ATTR_SCAN_PSV_MIN_CH_TIME]) {
> + if (info->attrs[NL80211_ATTR_SCAN_PSV_MAX_CH_TIME]) {
I think you should ignore the attributes if NL80211_FEATURE_SCAN_TIMES
is not set. That will enforce that anyone implementing it sets the
feature flag correctly, if they ever test their code at all anyway :-)
> +int cfg80211_trigger_scan(struct cfg80211_registered_device *rdev)
> +{
> + struct cfg80211_scan_request *request;
> +
> + ASSERT_RDEV_LOCK(rdev);
> +
> + request = rdev->scan_req;
> +
> + if (!request)
> + return -EINVAL;
> +
> + if (!request->min_passive_ch_time)
> + request->min_passive_ch_time =
> + ieee80211_msec_to_tu(IEEE80211_PSV_CH_TIME_MSEC);
> + if (!request->min_ch_time)
> + request->min_ch_time =
> + ieee80211_msec_to_tu(IEEE80211_CH_TIME_MSEC);
I pointed this out before -- this is problematic. Userspace could
request a max time of 10, and then this would set the min time to 30,
which means min > max which is completely stupid. We should prevent such
settings.
Maybe we need to enforce that if MIN is set, the respective MAX is also
set by userspace. That way, there are no surprises if userspace sets
only MAX but we change the default values at some point.
> @@ -1023,6 +1047,20 @@ int cfg80211_wext_siwscan(struct net_device *dev,
> if (wiphy->bands[i])
> creq->rates[i] = (1 << wiphy->bands[i]->n_bitrates) - 1;
>
> + if (wreq->min_channel_time) {
> + creq->min_ch_time = wreq->min_channel_time;
are you sure that wext uses TUs?
johannes
On Wed, 2012-06-20 at 13:38 +0300, Goldenshtein, Victor wrote:
> Will add this check, so we will read scan attributes only if the
> NL80211_FEATURE_SCAN_TIMES is set by the driver.
Thx.
> > If I set *ONLY* max_passive_ch_time = 10 via nl80211, what will happen?
> >
>
> There is no MAX times for the software scan, so the default values
> will be used ,no problem here.
What if it's hardware scan, not software scan?
> The hw_scan has it's own specific default values, and it should
> validate that his default min_passive_channel_time is <=
> max_passive_ch_time from the user request.
I don't think you're understanding it yet ...
Say I have USER_max_passive_time but DEFAULT_min_passive_time -- that
can cause all kinds of weird things to happen, and forcing drivers to
validate it seems rather stupid.
> The other option is to enforce usermode to define always min *and*
> max, something like:
> if ((info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]) &&
> (info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME])) {
> request->min_ch_time =
> nla_get_u32(info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]);
> request->max_ch_time =
> nla_get_u32(info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME]);
> }
>
> Although, not sure that enforcing is the correct way ?
It makes a lot more sense than your way which can result in invalid
configurations being passed to the driver ...
johannes
On Wed, Jun 20, 2012 at 3:12 PM, Johannes Berg
<[email protected]> wrote:
>
> It makes a lot more sense than your way which can result in invalid
> configurations being passed to the driver ...
>
I will respin.
--
Thanks,
Victor.
On Wed, 2012-06-20 at 11:23 +0300, Goldenshtein, Victor wrote:
> The attributes are optional, so the scan will run as usual using the
> default scan times defined in cfg for software scan and default driver
> specific values for the hw_scan (if those exist).
Right.
> > I think you should ignore the attributes if NL80211_FEATURE_SCAN_TIMES
> > is not set. That will enforce that anyone implementing it sets the
> > feature flag correctly, if they ever test their code at all anyway :-)
> >
> >
>
> The porpoise of the NL80211_FEATURE_SCAN_TIMES (as we discussed
> before) is to advertise this capability to usermode which implements
> 802.11k.
Yes, but if we let the attributes through then the driver author can
test his code and see different timings, even though the flag isn't set,
and then get confused. It'd be easier for the driver authors and more
reliable if we require the flag to be set for even reading timings from
userspace. Otherwise the driver might honour them even if the flag isn't
set which is very odd.
> > I pointed this out before -- this is problematic. Userspace could
> > request a max time of 10, and then this would set the min time to 30,
> > which means min > max which is completely stupid. We should prevent such
> > settings.
> >
>
> This check is done in nl80211_trigger_scan(), please see:
>
> +» » if·(request->min_ch_time·>·request->max_ch_time)
> +» » » return·-EINVAL;
>
> and
>
> +» » if·(request->min_passive_ch_time·>·request->max_passive_ch_time)
> +» » » return·-EINVAL;
>
> in this case we return EINVAL, btw this also documented:
> + * If one of the min times will be greater than max or set to zero,
> + * -EINVAL will be returned. For the software scan only the min times
You're not getting it.
If I set *ONLY* max_passive_ch_time = 10 via nl80211, what will happen?
> > are you sure that wext uses TUs?
> >
>
> yes, from "struct iw_scan_req":
> __u32 min_channel_time; /* in TU */
> __u32 max_channel_time; /* in TU */
Ok, great.
johannes
On Wed, Jun 20, 2012 at 11:29 AM, Johannes Berg
<[email protected]> wrote:
> Yes, but if we let the attributes through then the driver author can
> test his code and see different timings, even though the flag isn't set,
> and then get confused. It'd be easier for the driver authors and more
> reliable if we require the flag to be set for even reading timings from
> userspace. Otherwise the driver might honour them even if the flag isn't
> set which is very odd.
>
Will add this check, so we will read scan attributes only if the
NL80211_FEATURE_SCAN_TIMES is set by the driver.
>
>> > I pointed this out before -- this is problematic. Userspace could
>> > request a max time of 10, and then this would set the min time to 30,
>> > which means min > max which is completely stupid. We should prevent such
>> > settings.
>> >
>>
>> This check is done in nl80211_trigger_scan(), please see:
>>
>> +» » if·(request->min_ch_time·>·request->max_ch_time)
>> +» » » return·-EINVAL;
>>
>> and
>>
>> +» » if·(request->min_passive_ch_time·>·request->max_passive_ch_time)
>> +» » » return·-EINVAL;
>>
>> in this case we return EINVAL, btw this also documented:
>> + * If one of the min times will be greater than max or set to zero,
>> + * -EINVAL will be returned. For the software scan only the min times
>
> You're not getting it.
>
> If I set *ONLY* max_passive_ch_time = 10 via nl80211, what will happen?
>
There is no MAX times for the software scan, so the default values
will be used ,no problem here.
The hw_scan has it's own specific default values, and it should
validate that his default min_passive_channel_time is <=
max_passive_ch_time from the user request.
The other option is to enforce usermode to define always min *and*
max, something like:
if ((info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]) &&
(info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME])) {
request->min_ch_time =
nla_get_u32(info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]);
request->max_ch_time =
nla_get_u32(info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME]);
}
Although, not sure that enforcing is the correct way ?
--
Thanks,
Victor.
On Wed, Jun 20, 2012 at 10:46 AM, Johannes Berg
<[email protected]> wrote:
>> + * @NL80211_ATTR_SCAN_PSV_MAX_CH_TIME: Maximum passive scan time (in TUs),
>> + * u32 attribute (similar to @NL80211_ATTR_SCAN_MAX_CH_TIME).
>> + * Note:
>> + * The above channel time attributes are for the %NL80211_CMD_TRIGGER_SCAN
>> + * command. The attributes are optional, the driver will use default
>> + * channel time values if the attribute is not set.
>> + * If one of the min times will be greater than max or set to zero,
>> + * -EINVAL will be returned. For the software scan only the min times
>> + * are relevant.
>
> What if the driver doesn't support the timings?
>
The attributes are optional, so the scan will run as usual using the
default scan times defined in cfg for software scan and default driver
specific values for the hw_scan (if those exist).
>> + if (info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]) {
>> + if (info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME]) {
>> + if (info->attrs[NL80211_ATTR_SCAN_PSV_MIN_CH_TIME]) {
>> + if (info->attrs[NL80211_ATTR_SCAN_PSV_MAX_CH_TIME]) {
>
> I think you should ignore the attributes if NL80211_FEATURE_SCAN_TIMES
> is not set. That will enforce that anyone implementing it sets the
> feature flag correctly, if they ever test their code at all anyway :-)
>
>
The porpoise of the NL80211_FEATURE_SCAN_TIMES (as we discussed
before) is to advertise this capability to usermode which implements
802.11k.
>> +int cfg80211_trigger_scan(struct cfg80211_registered_device *rdev)
>> +{
>> + struct cfg80211_scan_request *request;
>> +
>> + ASSERT_RDEV_LOCK(rdev);
>> +
>> + request = rdev->scan_req;
>> +
>> + if (!request)
>> + return -EINVAL;
>> +
>> + if (!request->min_passive_ch_time)
>> + request->min_passive_ch_time =
>> + ieee80211_msec_to_tu(IEEE80211_PSV_CH_TIME_MSEC);
>> + if (!request->min_ch_time)
>> + request->min_ch_time =
>> + ieee80211_msec_to_tu(IEEE80211_CH_TIME_MSEC);
>
> I pointed this out before -- this is problematic. Userspace could
> request a max time of 10, and then this would set the min time to 30,
> which means min > max which is completely stupid. We should prevent such
> settings.
>
This check is done in nl80211_trigger_scan(), please see:
+» » if·(request->min_ch_time·>·request->max_ch_time)
+» » » return·-EINVAL;
and
+» » if·(request->min_passive_ch_time·>·request->max_passive_ch_time)
+» » » return·-EINVAL;
in this case we return EINVAL, btw this also documented:
+ * If one of the min times will be greater than max or set to zero,
+ * -EINVAL will be returned. For the software scan only the min times
>
>> @@ -1023,6 +1047,20 @@ int cfg80211_wext_siwscan(struct net_device *dev,
>> if (wiphy->bands[i])
>> creq->rates[i] = (1 << wiphy->bands[i]->n_bitrates) - 1;
>>
>> + if (wreq->min_channel_time) {
>> + creq->min_ch_time = wreq->min_channel_time;
>
> are you sure that wext uses TUs?
>
yes, from "struct iw_scan_req":
__u32 min_channel_time; /* in TU */
__u32 max_channel_time; /* in TU */
--
Thanks,
Victor.