Support configuring an RSSI threshold in dBm (s32) when scanning,
below which a BSS won't be reported by the cfg80211 driver.
Signed-off-by: Thomas Pedersen <[email protected]>
---
include/linux/nl80211.h | 6 ++++++
include/net/cfg80211.h | 7 +++++++
net/wireless/nl80211.c | 17 +++++++++++++++++
3 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index a6959f7..dc051a8 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1222,6 +1222,10 @@ enum nl80211_commands {
* @NL80211_ATTR_BG_SCAN_PERIOD: Background scan period in seconds
* or 0 to disable background scan.
*
+ * @NL80211_ATTR_SCAN_RSSI: rssi threshold (in s32 dBm) below which a BSS is
+ * not reported in scan results. Will be disabled if 0 or not specified.
+ * Supported in %NL80211_CMD_START_SCHED_SCAN and %NL80211_TRIGGER_SCAN.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1473,6 +1477,8 @@ enum nl80211_attrs {
NL80211_ATTR_BG_SCAN_PERIOD,
+ NL80211_ATTR_SCAN_RSSI,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0289d4c..39d293b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -920,6 +920,7 @@ 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
+ * @rssi: don't report scan results below this threshold
*/
struct cfg80211_scan_request {
struct cfg80211_ssid *ssids;
@@ -935,6 +936,7 @@ struct cfg80211_scan_request {
struct net_device *dev;
bool aborted;
bool no_cck;
+ s32 rssi;
/* keep last */
struct ieee80211_channel *channels[0];
@@ -966,6 +968,7 @@ struct cfg80211_match_set {
* @wiphy: the wiphy this was for
* @dev: the interface
* @channels: channels to scan
+ * @rssi: don't report scan results below this threshold
*/
struct cfg80211_sched_scan_request {
struct cfg80211_ssid *ssids;
@@ -976,6 +979,7 @@ struct cfg80211_sched_scan_request {
size_t ie_len;
struct cfg80211_match_set *match_sets;
int n_match_sets;
+ s32 rssi;
/* internal */
struct wiphy *wiphy;
@@ -1794,6 +1798,8 @@ struct cfg80211_ops {
* responds to probe-requests in hardware.
* @WIPHY_FLAG_OFFCHAN_TX: Device supports direct off-channel TX.
* @WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL: Device supports remain-on-channel call.
+ * @WIPHY_FLAG_SUPPORTS_RSSI_SCAN: Device supports filtering scan results by
+ * RSSI (in dBm).
*/
enum wiphy_flags {
WIPHY_FLAG_CUSTOM_REGULATORY = BIT(0),
@@ -1817,6 +1823,7 @@ enum wiphy_flags {
WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD = BIT(19),
WIPHY_FLAG_OFFCHAN_TX = BIT(20),
WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL = BIT(21),
+ WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
};
/**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 206465d..6886ca1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -206,6 +206,7 @@ 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_RSSI] = { .type = NLA_U32 },
};
/* policy for the key attributes */
@@ -3846,6 +3847,10 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
if (!rdev->ops->scan)
return -EOPNOTSUPP;
+ if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_RSSI_SCAN) &&
+ info->attrs[NL80211_ATTR_SCAN_RSSI])
+ return -EOPNOTSUPP;
+
if (rdev->scan_req)
return -EBUSY;
@@ -3991,6 +3996,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_RSSI])
+ request->rssi =
+ nla_get_u32(info->attrs[NL80211_ATTR_SCAN_RSSI]);
+
request->dev = dev;
request->wiphy = &rdev->wiphy;
@@ -4027,6 +4036,10 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
!rdev->ops->sched_scan_start)
return -EOPNOTSUPP;
+ if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_RSSI_SCAN) &&
+ info->attrs[NL80211_ATTR_SCAN_RSSI])
+ return -EOPNOTSUPP;
+
if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
return -EINVAL;
@@ -4212,6 +4225,10 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
request->ie_len);
}
+ if (info->attrs[NL80211_ATTR_SCAN_RSSI])
+ request->rssi =
+ nla_get_u32(info->attrs[NL80211_ATTR_SCAN_RSSI]);
+
request->dev = dev;
request->wiphy = &rdev->wiphy;
request->interval = interval;
--
1.7.4.1
On 06/07/2012 06:50 PM, Dan Williams wrote:
> On Wed, 2012-06-06 at 19:43 -0700, Thomas Pedersen wrote:
>> Support configuring an RSSI threshold in dBm (s32) when scanning,
>> below which a BSS won't be reported by the cfg80211 driver.
>
> What's the point of this? Shouldn't the driver report all available
> APs, even if the signal is small, and then userspace (or whatever is
> handling scan results) just ignores ones it doesn't care about?
When you do this on the firmware you can reduce host wakeups and hence
save power.
Kalle
On 06/07/2012 05:43 AM, Thomas Pedersen wrote:
> Signed-off-by: Thomas Pedersen <[email protected]>
A small description would be nice even when the patch is obvious.
Kalle
On 06/07/2012 05:43 AM, Thomas Pedersen wrote:
> Support configuring an RSSI threshold in dBm (s32) when scanning,
> below which a BSS won't be reported by the cfg80211 driver.
>
> Signed-off-by: Thomas Pedersen <[email protected]>
[...]
> + * @NL80211_ATTR_SCAN_RSSI: rssi threshold (in s32 dBm) below which a BSS is
> + * not reported in scan results. Will be disabled if 0 or not specified.
> + * Supported in %NL80211_CMD_START_SCHED_SCAN and %NL80211_TRIGGER_SCAN.
> + *
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> */
> @@ -1473,6 +1477,8 @@ enum nl80211_attrs {
>
> NL80211_ATTR_BG_SCAN_PERIOD,
>
> + NL80211_ATTR_SCAN_RSSI,
NL80211_ATTR_SCAN_RSSI_THRESHOLD? Or is limit a better term? Or
something else?
My english sucks anyway...
> @@ -935,6 +936,7 @@ struct cfg80211_scan_request {
> struct net_device *dev;
> bool aborted;
> bool no_cck;
> + s32 rssi;
rssi_threshold?
>
> /* keep last */
> struct ieee80211_channel *channels[0];
> @@ -966,6 +968,7 @@ struct cfg80211_match_set {
> * @wiphy: the wiphy this was for
> * @dev: the interface
> * @channels: channels to scan
> + * @rssi: don't report scan results below this threshold
> */
> struct cfg80211_sched_scan_request {
> struct cfg80211_ssid *ssids;
> @@ -976,6 +979,7 @@ struct cfg80211_sched_scan_request {
> size_t ie_len;
> struct cfg80211_match_set *match_sets;
> int n_match_sets;
> + s32 rssi;
rssi_threshold?
> @@ -1794,6 +1798,8 @@ struct cfg80211_ops {
> * responds to probe-requests in hardware.
> * @WIPHY_FLAG_OFFCHAN_TX: Device supports direct off-channel TX.
> * @WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL: Device supports remain-on-channel call.
> + * @WIPHY_FLAG_SUPPORTS_RSSI_SCAN: Device supports filtering scan results by
> + * RSSI (in dBm).
> */
> enum wiphy_flags {
> WIPHY_FLAG_CUSTOM_REGULATORY = BIT(0),
> @@ -1817,6 +1823,7 @@ enum wiphy_flags {
> WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD = BIT(19),
> WIPHY_FLAG_OFFCHAN_TX = BIT(20),
> WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL = BIT(21),
> + WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
> };
Is this flag really needed? For me this looks like an optimisation more
than a functional change. If the driver supports this, that's great and
we can save some power. But if the driver does not support it does it
really make any difference for the user space? Would user space act
differently if this feature is not supported by the driver?
(Just asking to understand this better.)
Kalle
On 06/07/2012 09:38 PM, Pedersen, Thomas wrote:
>>> + WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
>>> > > };
>> >
>> > Is this flag really needed? For me this looks like an optimisation more
>> > than a functional change. If the driver supports this, that's great and
>> > we can save some power. But if the driver does not support it does it
>> > really make any difference for the user space? Would user space act
>> > differently if this feature is not supported by the driver?
>
> Well, this allows cfg80211 to return an error if this feature is
> requested but not supported by the driver / fw.
But do we want to return an error when the driver doesn't support this?
I was thinking that driver should just ignore the attribute in that case
and let user space filter the results.
Kalle
On Wed, 2012-06-06 at 19:43 -0700, Thomas Pedersen wrote:
> Support configuring an RSSI threshold in dBm (s32) when scanning,
> below which a BSS won't be reported by the cfg80211 driver.
What's the point of this? Shouldn't the driver report all available
APs, even if the signal is small, and then userspace (or whatever is
handling scan results) just ignores ones it doesn't care about?
Dan
> Signed-off-by: Thomas Pedersen <[email protected]>
> ---
> include/linux/nl80211.h | 6 ++++++
> include/net/cfg80211.h | 7 +++++++
> net/wireless/nl80211.c | 17 +++++++++++++++++
> 3 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index a6959f7..dc051a8 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -1222,6 +1222,10 @@ enum nl80211_commands {
> * @NL80211_ATTR_BG_SCAN_PERIOD: Background scan period in seconds
> * or 0 to disable background scan.
> *
> + * @NL80211_ATTR_SCAN_RSSI: rssi threshold (in s32 dBm) below which a BSS is
> + * not reported in scan results. Will be disabled if 0 or not specified.
> + * Supported in %NL80211_CMD_START_SCHED_SCAN and %NL80211_TRIGGER_SCAN.
> + *
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> */
> @@ -1473,6 +1477,8 @@ enum nl80211_attrs {
>
> NL80211_ATTR_BG_SCAN_PERIOD,
>
> + NL80211_ATTR_SCAN_RSSI,
> +
> /* add attributes here, update the policy in nl80211.c */
>
> __NL80211_ATTR_AFTER_LAST,
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 0289d4c..39d293b 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -920,6 +920,7 @@ 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
> + * @rssi: don't report scan results below this threshold
> */
> struct cfg80211_scan_request {
> struct cfg80211_ssid *ssids;
> @@ -935,6 +936,7 @@ struct cfg80211_scan_request {
> struct net_device *dev;
> bool aborted;
> bool no_cck;
> + s32 rssi;
>
> /* keep last */
> struct ieee80211_channel *channels[0];
> @@ -966,6 +968,7 @@ struct cfg80211_match_set {
> * @wiphy: the wiphy this was for
> * @dev: the interface
> * @channels: channels to scan
> + * @rssi: don't report scan results below this threshold
> */
> struct cfg80211_sched_scan_request {
> struct cfg80211_ssid *ssids;
> @@ -976,6 +979,7 @@ struct cfg80211_sched_scan_request {
> size_t ie_len;
> struct cfg80211_match_set *match_sets;
> int n_match_sets;
> + s32 rssi;
>
> /* internal */
> struct wiphy *wiphy;
> @@ -1794,6 +1798,8 @@ struct cfg80211_ops {
> * responds to probe-requests in hardware.
> * @WIPHY_FLAG_OFFCHAN_TX: Device supports direct off-channel TX.
> * @WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL: Device supports remain-on-channel call.
> + * @WIPHY_FLAG_SUPPORTS_RSSI_SCAN: Device supports filtering scan results by
> + * RSSI (in dBm).
> */
> enum wiphy_flags {
> WIPHY_FLAG_CUSTOM_REGULATORY = BIT(0),
> @@ -1817,6 +1823,7 @@ enum wiphy_flags {
> WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD = BIT(19),
> WIPHY_FLAG_OFFCHAN_TX = BIT(20),
> WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL = BIT(21),
> + WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
> };
>
> /**
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 206465d..6886ca1 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -206,6 +206,7 @@ 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_RSSI] = { .type = NLA_U32 },
> };
>
> /* policy for the key attributes */
> @@ -3846,6 +3847,10 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> if (!rdev->ops->scan)
> return -EOPNOTSUPP;
>
> + if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_RSSI_SCAN) &&
> + info->attrs[NL80211_ATTR_SCAN_RSSI])
> + return -EOPNOTSUPP;
> +
> if (rdev->scan_req)
> return -EBUSY;
>
> @@ -3991,6 +3996,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_RSSI])
> + request->rssi =
> + nla_get_u32(info->attrs[NL80211_ATTR_SCAN_RSSI]);
> +
> request->dev = dev;
> request->wiphy = &rdev->wiphy;
>
> @@ -4027,6 +4036,10 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
> !rdev->ops->sched_scan_start)
> return -EOPNOTSUPP;
>
> + if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_RSSI_SCAN) &&
> + info->attrs[NL80211_ATTR_SCAN_RSSI])
> + return -EOPNOTSUPP;
> +
> if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
> return -EINVAL;
>
> @@ -4212,6 +4225,10 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
> request->ie_len);
> }
>
> + if (info->attrs[NL80211_ATTR_SCAN_RSSI])
> + request->rssi =
> + nla_get_u32(info->attrs[NL80211_ATTR_SCAN_RSSI]);
> +
> request->dev = dev;
> request->wiphy = &rdev->wiphy;
> request->interval = interval;
On Thu, Jun 07, 2012 at 09:53:20PM +0300, Kalle Valo wrote:
> On 06/07/2012 09:38 PM, Pedersen, Thomas wrote:
> >>> + WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
> >>> > > };
> >> >
> >> > Is this flag really needed? For me this looks like an optimisation more
> >> > than a functional change. If the driver supports this, that's great and
> >> > we can save some power. But if the driver does not support it does it
> >> > really make any difference for the user space? Would user space act
> >> > differently if this feature is not supported by the driver?
> >
> > Well, this allows cfg80211 to return an error if this feature is
> > requested but not supported by the driver / fw.
>
> But do we want to return an error when the driver doesn't support this?
> I was thinking that driver should just ignore the attribute in that case
> and let user space filter the results.
>
> Kalle
Sure, we can just let userspace unconditionally filter the results when
we do something like:
iw wlan0 scan rssi -40
Johannes, does this look OK to you?
Thomas
On Sat, Jun 09, 2012 at 10:09:52AM +0200, Johannes Berg wrote:
> On Wed, 2012-06-06 at 19:43 -0700, Thomas Pedersen wrote:
>
> > + if (sdata->local->scan_req->rssi &&
> > + rx_status->signal < sdata->local->scan_req->rssi)
> > + return RX_DROP_MONITOR;
>
> I believe that should be RX_CONTINUE
Thanks. Since this is considered a performance optimization, it makes
little sense to even implement it in mac80211?
Thomas
On 06/10/2012 01:03 AM, Pedersen, Thomas wrote:
> On Sat, Jun 09, 2012 at 10:09:52AM +0200, Johannes Berg wrote:
>> On Wed, 2012-06-06 at 19:43 -0700, Thomas Pedersen wrote:
>>
>>> + if (sdata->local->scan_req->rssi &&
>>> + rx_status->signal < sdata->local->scan_req->rssi)
>>> + return RX_DROP_MONITOR;
>>
>> I believe that should be RX_CONTINUE
>
> Thanks. Since this is considered a performance optimization, it makes
> little sense to even implement it in mac80211?
It doesn't do any harm either and on the positive side the feature will
be tested with all mac80211 drivers. But other than that there's no real
advantage.
Well, there's a theoretical advantage once we implement partial scan
results which would avoid waking up user space unnecessary. But I doubt
it makes any difference in practise.
But I still vote to take this patch, just to make the feature supported
with a wider range of drivers.
Kalle
Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/main.c | 3 +++
net/mac80211/scan.c | 4 ++++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index f5548e9..3f49d1b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -873,6 +873,9 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
if (local->hw.wiphy->flags & WIPHY_FLAG_SUPPORTS_TDLS)
local->hw.wiphy->flags |= WIPHY_FLAG_TDLS_EXTERNAL_SETUP;
+ if (local->hw.wiphy->signal_type == CFG80211_SIGNAL_TYPE_MBM)
+ local->hw.wiphy->flags |= WIPHY_FLAG_SUPPORTS_RSSI_SCAN;
+
result = wiphy_register(local->hw.wiphy);
if (result < 0)
goto fail_wiphy_register;
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 8282284..1b92405 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -212,6 +212,10 @@ ieee80211_scan_rx(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb)
if (baselen > skb->len)
return RX_DROP_MONITOR;
+ if (sdata->local->scan_req->rssi &&
+ rx_status->signal < sdata->local->scan_req->rssi)
+ return RX_DROP_MONITOR;
+
ieee802_11_parse_elems(elements, skb->len - baselen, &elems);
if (elems.ds_params && elems.ds_params_len == 1)
--
1.7.4.1
On Sat, 2012-06-09 at 10:09 +0200, Johannes Berg wrote:
> On Thu, 2012-06-07 at 12:18 -0700, Pedersen, Thomas wrote:
> > On Thu, Jun 07, 2012 at 09:53:20PM +0300, Kalle Valo wrote:
> > > On 06/07/2012 09:38 PM, Pedersen, Thomas wrote:
> > > >>> + WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
> > > >>> > > };
> > > >> >
> > > >> > Is this flag really needed? For me this looks like an optimisation more
> > > >> > than a functional change. If the driver supports this, that's great and
> > > >> > we can save some power. But if the driver does not support it does it
> > > >> > really make any difference for the user space? Would user space act
> > > >> > differently if this feature is not supported by the driver?
> > > >
> > > > Well, this allows cfg80211 to return an error if this feature is
> > > > requested but not supported by the driver / fw.
> > >
> > > But do we want to return an error when the driver doesn't support this?
> > > I was thinking that driver should just ignore the attribute in that case
> > > and let user space filter the results.
> > >
> > > Kalle
> >
> > Sure, we can just let userspace unconditionally filter the results when
> > we do something like:
> >
> > iw wlan0 scan rssi -40
> >
> > Johannes, does this look OK to you?
>
> I don't see a need to filter in iw, but I agree with Kalle that we
> shouldn't impose any restrictions on a performance optimisation.
Yeah, I also agree that the flag is probably unnecessary. We can't
prevent results that are below the threshold anyway, because results
from other scans might be cached in the scan results. So the userspace
must always be ready to receive results that need to be dropped.
--
Cheers,
Luca.
On Thu, 2012-06-07 at 12:18 -0700, Pedersen, Thomas wrote:
> On Thu, Jun 07, 2012 at 09:53:20PM +0300, Kalle Valo wrote:
> > On 06/07/2012 09:38 PM, Pedersen, Thomas wrote:
> > >>> + WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
> > >>> > > };
> > >> >
> > >> > Is this flag really needed? For me this looks like an optimisation more
> > >> > than a functional change. If the driver supports this, that's great and
> > >> > we can save some power. But if the driver does not support it does it
> > >> > really make any difference for the user space? Would user space act
> > >> > differently if this feature is not supported by the driver?
> > >
> > > Well, this allows cfg80211 to return an error if this feature is
> > > requested but not supported by the driver / fw.
> >
> > But do we want to return an error when the driver doesn't support this?
> > I was thinking that driver should just ignore the attribute in that case
> > and let user space filter the results.
> >
> > Kalle
>
> Sure, we can just let userspace unconditionally filter the results when
> we do something like:
>
> iw wlan0 scan rssi -40
>
> Johannes, does this look OK to you?
I don't see a need to filter in iw, but I agree with Kalle that we
shouldn't impose any restrictions on a performance optimisation.
johannes
On Thu, Jun 07, 2012 at 10:23:57AM +0300, Kalle Valo wrote:
> On 06/07/2012 05:43 AM, Thomas Pedersen wrote:
> > Support configuring an RSSI threshold in dBm (s32) when scanning,
> > below which a BSS won't be reported by the cfg80211 driver.
> >
> > Signed-off-by: Thomas Pedersen <[email protected]>
>
> [...]
>
> > + * @NL80211_ATTR_SCAN_RSSI: rssi threshold (in s32 dBm) below which a BSS is
> > + * not reported in scan results. Will be disabled if 0 or not specified.
> > + * Supported in %NL80211_CMD_START_SCHED_SCAN and %NL80211_TRIGGER_SCAN.
> > + *
> > * @NL80211_ATTR_MAX: highest attribute number currently defined
> > * @__NL80211_ATTR_AFTER_LAST: internal use
> > */
> > @@ -1473,6 +1477,8 @@ enum nl80211_attrs {
> >
> > NL80211_ATTR_BG_SCAN_PERIOD,
> >
> > + NL80211_ATTR_SCAN_RSSI,
>
> NL80211_ATTR_SCAN_RSSI_THRESHOLD? Or is limit a better term? Or
> something else?
>
> My english sucks anyway...
No you're probably right, thanks :)
> > @@ -935,6 +936,7 @@ struct cfg80211_scan_request {
> > struct net_device *dev;
> > bool aborted;
> > bool no_cck;
> > + s32 rssi;
>
> rssi_threshold?
Yes.
> >
> > /* keep last */
> > struct ieee80211_channel *channels[0];
> > @@ -966,6 +968,7 @@ struct cfg80211_match_set {
> > * @wiphy: the wiphy this was for
> > * @dev: the interface
> > * @channels: channels to scan
> > + * @rssi: don't report scan results below this threshold
> > */
> > struct cfg80211_sched_scan_request {
> > struct cfg80211_ssid *ssids;
> > @@ -976,6 +979,7 @@ struct cfg80211_sched_scan_request {
> > size_t ie_len;
> > struct cfg80211_match_set *match_sets;
> > int n_match_sets;
> > + s32 rssi;
>
> rssi_threshold?
Ditto.
> > @@ -1794,6 +1798,8 @@ struct cfg80211_ops {
> > * responds to probe-requests in hardware.
> > * @WIPHY_FLAG_OFFCHAN_TX: Device supports direct off-channel TX.
> > * @WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL: Device supports remain-on-channel call.
> > + * @WIPHY_FLAG_SUPPORTS_RSSI_SCAN: Device supports filtering scan results by
> > + * RSSI (in dBm).
> > */
> > enum wiphy_flags {
> > WIPHY_FLAG_CUSTOM_REGULATORY = BIT(0),
> > @@ -1817,6 +1823,7 @@ enum wiphy_flags {
> > WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD = BIT(19),
> > WIPHY_FLAG_OFFCHAN_TX = BIT(20),
> > WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL = BIT(21),
> > + WIPHY_FLAG_SUPPORTS_RSSI_SCAN = BIT(22),
> > };
>
> Is this flag really needed? For me this looks like an optimisation more
> than a functional change. If the driver supports this, that's great and
> we can save some power. But if the driver does not support it does it
> really make any difference for the user space? Would user space act
> differently if this feature is not supported by the driver?
Well, this allows cfg80211 to return an error if this feature is
requested but not supported by the driver / fw.
Thomas
On Wed, 2012-06-06 at 19:43 -0700, Thomas Pedersen wrote:
> Support configuring an RSSI threshold in dBm (s32) when scanning,
> below which a BSS won't be reported by the cfg80211 driver.
>
> Signed-off-by: Thomas Pedersen <[email protected]>
> ---
> include/linux/nl80211.h | 6 ++++++
> include/net/cfg80211.h | 7 +++++++
> net/wireless/nl80211.c | 17 +++++++++++++++++
> 3 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index a6959f7..dc051a8 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -1222,6 +1222,10 @@ enum nl80211_commands {
> * @NL80211_ATTR_BG_SCAN_PERIOD: Background scan period in seconds
> * or 0 to disable background scan.
> *
> + * @NL80211_ATTR_SCAN_RSSI: rssi threshold (in s32 dBm) below which a BSS is
> + * not reported in scan results. Will be disabled if 0 or not specified.
> + * Supported in %NL80211_CMD_START_SCHED_SCAN and %NL80211_TRIGGER_SCAN.
> + *
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> */
BTW, I actually had discussed this feature with Victor internally some
time ago. And, IIRC, I also talked about it briefly with Johannes.
My idea was that we could maybe make this more generic? There are lots
of different values that could be used as thresholds or filter options.
Couldn't we make this API generic as well? The first thing that comes to
my mind is the existing NL80211_ATTR_SCHED_SCAN_MATCH. It is
essentially the same, we only want to report certain matches and filter
everything else out...
Does it make sense?
--
Luca.
On Wed, 2012-06-06 at 19:43 -0700, Thomas Pedersen wrote:
> + if (sdata->local->scan_req->rssi &&
> + rx_status->signal < sdata->local->scan_req->rssi)
> + return RX_DROP_MONITOR;
I believe that should be RX_CONTINUE
johannes