2016-11-23 22:08:02

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 1/2] nl80211: Use different attrs for BSSID and random MAC addr in scan req

From: Vamsi Krishna <[email protected]>

NL80211_ATTR_MAC was used to set both the specific BSSID to be scanned
and the random MAC address to be used when privacy is enabled. When both
the features are enabled, both the BSSID and the local MAC address were
getting same value causing Probe Request frames to go with unintended
DA. Hence, this has been fixed by using a different NL80211_ATTR_BSSID
attribute to set the specific BSSID (which was the more recent addition
in cfg80211) for a scan.

Backwards compatibility with old userspace software is maintained to
some extend by allowing NL80211_ATTR_MAC to be used to set the specific
BSSID when scanning without enabling random MAC address use.

Scanning with random source MAC address was introduced by commit
ad2b26abc157 ("cfg80211: allow drivers to support random MAC addresses
for scan") and the issue was introduced with the addition of the second
user for the same attribute in commit 818965d39177 ("cfg80211: Allow a
scan request for a specific BSSID").

Fixes: 818965d39177 ("cfg80211: Allow a scan request for a specific BSSID")
Signed-off-by: Vamsi Krishna <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
include/uapi/linux/nl80211.h | 8 +++++++-
net/wireless/nl80211.c | 16 +++++++++++++++-
2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 259c9c7..984a35ac 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -323,7 +323,7 @@
* @NL80211_CMD_GET_SCAN: get scan results
* @NL80211_CMD_TRIGGER_SCAN: trigger a new scan with the given parameters
* %NL80211_ATTR_TX_NO_CCK_RATE is used to decide whether to send the
- * probe requests at CCK rate or not. %NL80211_ATTR_MAC can be used to
+ * probe requests at CCK rate or not. %NL80211_ATTR_BSSID can be used to
* specify a BSSID to scan for; if not included, the wildcard BSSID will
* be used.
* @NL80211_CMD_NEW_SCAN_RESULTS: scan notification (as a reply to
@@ -1977,6 +1977,10 @@ enum nl80211_commands {
* @NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED: Indicates whether or not multicast
* packets should be send out as unicast to all stations (flag attribute).
*
+ * @NL80211_ATTR_BSSID: The BSSID of the AP (various uses). Note that
+ * %NL80211_ATTR_MAC has also been used in various commands/events for
+ * specifying the BSSID.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2381,6 +2385,8 @@ enum nl80211_attrs {

NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED,

+ NL80211_ATTR_BSSID,
+
/* 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 e4f718e..8db5cb1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -404,6 +404,7 @@ enum nl80211_multicast_groups {
.len = FILS_MAX_KEK_LEN },
[NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
+ [NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
};

/* policy for the key attributes */
@@ -6703,7 +6704,20 @@ 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_MAC])
+ /* Initial implementation used NL80211_ATTR_MAC to set the specific
+ * BSSID to scan for. This was problematic because that same attribute
+ * was already used for another purpose (local random MAC address). The
+ * NL80211_ATTR_BSSID attribute was added to fix this. For backwards
+ * compatibility with older userspace components, also use the
+ * NL80211_ATTR_MAC value here if it can be determined to be used for
+ * the specific BSSID use case instead of the random MAC address
+ * (NL80211_ATTR_SCAN_FLAGS is used to enable random MAC address use).
+ */
+ if (info->attrs[NL80211_ATTR_BSSID])
+ memcpy(request->bssid,
+ nla_data(info->attrs[NL80211_ATTR_BSSID]), ETH_ALEN);
+ else if (!info->attrs[NL80211_ATTR_SCAN_FLAGS] &&
+ info->attrs[NL80211_ATTR_MAC])
memcpy(request->bssid, nla_data(info->attrs[NL80211_ATTR_MAC]),
ETH_ALEN);
else
--
1.9.1


2016-11-28 14:15:00

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] nl80211: Use different attrs for BSSID and random MAC addr in scan req


> > + * @NL80211_ATTR_BSSID: The BSSID of the AP (various uses). Note
> > that
>
> The BSSID may also be used for other things, like P2P GO, right? Also
> "various uses" is probably unnecessary? Every command using this
> attribute should describe it's use in their description.
>
>
> >
> > + * %NL80211_ATTR_MAC has also been used in various
> > commands/events for
> > + * specifying the BSSID.
>
> This can be a bit confusing.  Maybe you can specify which commands
> *used* to use NL80211_ATTR_MAC but now use NL80211_ATTR_BSSID?

I'd actually avoid that, let's not make the "compatibility quirks" part
of the documentation people read through normally ... In the code,
that's fine, but here, I think less so.

With that, perhaps just rephrase this to

"The BSSID of the AP. Note that %NL80211_ATTR_MAC is also used in
various commands/events for specifying the BSSID."

> > - if (info->attrs[NL80211_ATTR_MAC])
> > + /* Initial implementation used NL80211_ATTR_MAC to set the
> > specific
> > +  * BSSID to scan for. This was problematic because that
> > same attribute
> > +  * was already used for another purpose (local random MAC
> > address). The
> > +  * NL80211_ATTR_BSSID attribute was added to fix this. For
> > backwards
> > +  * compatibility with older userspace components, also use
> > the
> > +  * NL80211_ATTR_MAC value here if it can be determined to
> > be used for
> > +  * the specific BSSID use case instead of the random MAC
> > address
> > +  * (NL80211_ATTR_SCAN_FLAGS is used to enable random MAC
> > address use).
> > +  */
>
> You should probably add this information to the
> NL80211_CMD_TRIGGER_SCAN description.

It may need an update to refer to ATTR_BSSID, but again I don't think
all the compatibility discussion should be there.

>
> >
> > + if (info->attrs[NL80211_ATTR_BSSID])
> > + memcpy(request->bssid,
> > +        nla_data(info->attrs[NL80211_ATTR_BSSID]),
> > ETH_ALEN);
> > + else if (!info->attrs[NL80211_ATTR_SCAN_FLAGS] &&
>
> You should actually check that the SCAN_FLAGS attribute either
> doesn't
> exist (as you already do) or, if it exists, that it doesn't have the
> NL80211_SCAN_FLAG_RANDOM_ADDR flags.
>

Agree with that, that would make sense.

johannes

2016-11-28 14:20:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs

> It seems there has already
> been taken a shot at this which may be used/extended [1].
>

That's a good point - it's somewhat similar.

This is obviously a different context - offloaded BSS selection vs.
scheduled scan (for host BSS selection), but perhaps the attribute &
definitions could be reused?

johannes

2016-11-25 09:21:21

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs

IMHO "report better BSSs" is vague in this context.  It would better to
use something more concrete like "add relative RSSI attribute to sched
scan".


On Thu, 2016-11-24 at 00:07 +0200, Jouni Malinen wrote:
> From: vamsi krishna <[email protected]>

> Enhance sched scan to support option of finding a better BSS while in
> connected state. Firmware scans the medium and reports when it finds
a
> known BSS which has a significantly better RSSI than the current

"Significantly" is dependent on the value you use. :)

> connected BSS.

> Signed-off-by: vamsi krishna <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> ---
>  include/net/cfg80211.h       | 17 +++++++++++++++++
>  include/uapi/linux/nl80211.h | 17 +++++++++++++++++
>  net/wireless/nl80211.c       | 32 ++++++++++++++++++++++++++++++--
>  3 files changed, 64 insertions(+), 2 deletions(-)

> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index ef42749..c62c42a 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1626,6 +1626,20 @@ struct cfg80211_sched_scan_plan {
>   * cycle.  The driver may ignore this parameter and start
>   * immediately (or at any other time), if this feature is not
>   * supported.
> + * @relative_rssi: Relative RSSI threshold to restrict scan result
reporting in
> + * connected state to cases where a matching BSS is
determined to have a
> + * significantly better RSSI than the current connected BSS.

What happens with this attribute if we're not connected? Also, I think
you should specify the attribute type (int?) and the unit here.


> + * @relative_rssi_5g_pref: The amount of RSSI preference that is given to a
> + * 5 GHz BSS over 2.4 GHz BSS while looking for better BSSs in connected
> + * state.
> + * If the current connected BSS is in the 2.4 GHz band, other BSSs in the
> + * 2.4 GHz band to be reported should have better RSSI by @relative_rssi
> + * and other BSSs in the 5 GHz band to be reported should have better RSSI
> + * by (@relative_rssi - @relative_rssi_5g_pref).
> + * If the current connected BSS is in the 5 GHz band, other BSSs in the
> + * 2.4 GHz band to be reported should have better RSSI by
> + * (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
> + * band to be reported should have better RSSI by by @relative_rssi.

Could there be cases where you want the opposite? Meaning,
relative_rssi_5g_pref being negative?

> */
> struct cfg80211_sched_scan_request {
> struct cfg80211_ssid *ssids;
> @@ -1645,6 +1659,9 @@ struct cfg80211_sched_scan_request {
> u8 mac_addr[ETH_ALEN] __aligned(2);
> u8 mac_addr_mask[ETH_ALEN] __aligned(2);
>
> + int relative_rssi;
> + int relative_rssi_5g_pref;

I'm not sure it was your intention, but for flexibility, it's good that
both these attributes are ints so that you can find relative values on
both sides (greater than and smaller than).

It doesn't apply to the "better BSS" case, but maybe people find a
different use for it in the future?


> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 984a35ac..34b16a4 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1981,6 +1981,16 @@ enum nl80211_commands {
> * %NL80211_ATTR_MAC has also been used in various commands/events for
> * specifying the BSSID.
> *
> + * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI: Relative RSSI threshold by which
> + * other BSSs has to be better than the current connected BSS so that they
> + * get reported to user space. This will give an opportunity to userspace
> + * to consider connecting to other matching BSSs which have better RSSI
> + * than the current connected BSS.

The userspace can always do this, but the advantage here is that it
doesn't need to be woken up and check the results itself, since it
would only receive results that matter.


> + *
> + * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF: The amount of RSSI preference
> + * to be given to 5 GHz APs over 2.4 GHz APs while searching for better
> + * BSSs than the current connected BSS.
> + *
> * @NUM_NL80211_ATTR: total number of nl80211_attrs available
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2387,6 +2397,9 @@ enum nl80211_attrs {
>
> NL80211_ATTR_BSSID,
>
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
> +
> /* add attributes here, update the policy in nl80211.c */
>
> __NL80211_ATTR_AFTER_LAST,
> @@ -4698,6 +4711,9 @@ enum nl80211_feature_flags {
> * configuration (AP/mesh) with VHT rates.
> * @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
> * with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
> + * @NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS: The driver supports sched_scan

Should be "RELATIVE_RSSI" instead of "BETTER_BSS".

[...]

> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 8db5cb1..af018a5 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -405,6 +405,8 @@ enum nl80211_multicast_groups {
> [NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
> [NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
> [NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
> + [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] = { .type = NLA_U32 },
> + [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF] = { .type = NLA_U32 },

This should probably be NLA_S32 (if we want to allow negative values as
well).


[...]

>
@@ -7156,6 +7166,14 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
> request->delay =
> nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_DELAY]);
>
> + if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI])
> + request->relative_rssi = nla_get_s32(
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
> +
> + if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF])
> + request->relative_rssi_5g_pref = nla_get_s32(
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]);

Good, I see you use s32 here. :)

[...]

> @@ -9649,7 +9667,8 @@ static int nl80211_send_wowlan_tcp(struct sk_buff *msg,
> return 0;
> }
>
> -static int nl80211_send_wowlan_nd(struct sk_buff *msg,
> +static int nl80211_send_wowlan_nd(struct wiphy *wiphy,
> + struct sk_buff *msg,
> struct cfg80211_sched_scan_request *req)
> {
> struct nlattr *nd, *freqs, *matches, *match, *scan_plans, *scan_plan;
> @@ -9670,6 +9689,15 @@ static int nl80211_send_wowlan_nd(struct sk_buff *msg,
> if (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_DELAY, req->delay))
> return -ENOBUFS;
>
> + if (wiphy_ext_feature_isset(
> + wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS) &&
> + (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> + req->relative_rssi) ||
> + nla_put_u32(msg,
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
> + req->relative_rssi_5g_pref)))
> + return -ENOBUFS;
> +

Why did you add this to nl80211_send_wowlan_nd() function?


[...]

--
Luca.

2016-11-28 14:34:14

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs

On Mon, 2016-11-28 at 15:20 +0100, Johannes Berg wrote:
> > It seems there has already
> > been taken a shot at this which may be used/extended [1].
> >
>
> That's a good point - it's somewhat similar.
>
> This is obviously a different context - offloaded BSS selection vs.
> scheduled scan (for host BSS selection), but perhaps the attribute &
> definitions could be reused?

Yes, similar but not quite the same. The existing case is for finding
BSSs that are worth waking the host up for (while disconnected), so it
needs to be better than the RSSI passed (absolute number). Now this is
about relative RSSI as compared to the current connection, so
RELATIVE_RSSI is different than RSSI and I think the same attribute
should not be used, to avoid confusion.

--
Luca.

2016-11-28 14:17:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs

On Thu, 2016-11-24 at 00:07 +0200, Jouni Malinen wrote:

> + int bbr;

bbr? Also, bool?
 
johannes

2016-11-25 08:44:33

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 1/2] nl80211: Use different attrs for BSSID and random MAC addr in scan req

On Thu, 2016-11-24 at 00:07 +0200, Jouni Malinen wrote:
> From: Vamsi Krishna <[email protected]>
>
> NL80211_ATTR_MAC was used to set both the specific BSSID to be scanned
> and the random MAC address to be used when privacy is enabled. When both
> the features are enabled, both the BSSID and the local MAC address were
> getting same value causing Probe Request frames to go with unintended
> DA. Hence, this has been fixed by using a different NL80211_ATTR_BSSID
> attribute to set the specific BSSID (which was the more recent addition
> in cfg80211) for a scan.
>
> Backwards compatibility with old userspace software is maintained to
> some extend by allowing NL80211_ATTR_MAC to be used to set the specific

typo, extent

> BSSID when scanning without enabling random MAC address use.

Hopefully nobody expects the broken functionality to be the correct
behavior, so we can slightly break backwards compatibility.


> Scanning with random source MAC address was introduced by commit
> ad2b26abc157 ("cfg80211: allow drivers to support random MAC addresses
> for scan") and the issue was introduced with the addition of the second
> user for the same attribute in commit 818965d39177 ("cfg80211: Allow a
> scan request for a specific BSSID").
>
> Fixes: 818965d39177 ("cfg80211: Allow a scan request for a specific BSSID")
> Signed-off-by: Vamsi Krishna <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> ---




> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 259c9c7..984a35ac 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h

[...]

> @@ -1977,6 +1977,10 @@ enum nl80211_commands {
> * @NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED: Indicates whether or not multicast
> * packets should be send out as unicast to all stations (flag attribute).
> *
> + * @NL80211_ATTR_BSSID: The BSSID of the AP (various uses). Note that

The BSSID may also be used for other things, like P2P GO, right? Also
"various uses" is probably unnecessary? Every command using this
attribute should describe it's use in their description.


> + * %NL80211_ATTR_MAC has also been used in various commands/events for
> + * specifying the BSSID.

This can be a bit confusing. Maybe you can specify which commands
*used* to use NL80211_ATTR_MAC but now use NL80211_ATTR_BSSID?

[...]

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index e4f718e..8db5cb1 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c

[...]

> @@ -6703,7 +6704,20 @@ 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_MAC])
> + /* Initial implementation used NL80211_ATTR_MAC to set the specific
> + * BSSID to scan for. This was problematic because that same attribute
> + * was already used for another purpose (local random MAC address). The
> + * NL80211_ATTR_BSSID attribute was added to fix this. For backwards
> + * compatibility with older userspace components, also use the
> + * NL80211_ATTR_MAC value here if it can be determined to be used for
> + * the specific BSSID use case instead of the random MAC address
> + * (NL80211_ATTR_SCAN_FLAGS is used to enable random MAC address use).
> + */

You should probably add this information to the
NL80211_CMD_TRIGGER_SCAN description.


> + if (info->attrs[NL80211_ATTR_BSSID])
> + memcpy(request->bssid,
> + nla_data(info->attrs[NL80211_ATTR_BSSID]), ETH_ALEN);
> + else if (!info->attrs[NL80211_ATTR_SCAN_FLAGS] &&

You should actually check that the SCAN_FLAGS attribute either doesn't
exist (as you already do) or, if it exists, that it doesn't have the
NL80211_SCAN_FLAG_RANDOM_ADDR flags.


--
Cheers,
Luca.

2016-11-27 20:51:08

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs

On 23-11-2016 23:07, Jouni Malinen wrote:
> From: vamsi krishna <[email protected]>
>
> Enhance sched scan to support option of finding a better BSS while in
> connected state. Firmware scans the medium and reports when it finds a
> known BSS which has a significantly better RSSI than the current
> connected BSS.

I suppose the intent here is to be notified about roaming candidates and
hence the "connected state" prerequisite. It seems there has already
been taken a shot at this which may be used/extended [1].

Regards,
Arend

[1] http://lxr.free-electrons.com/source/include/uapi/linux/nl80211.h#L2945
[2] http://lxr.free-electrons.com/source/include/uapi/linux/nl80211.h#L4826

> Signed-off-by: vamsi krishna <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> ---
> include/net/cfg80211.h | 17 +++++++++++++++++
> include/uapi/linux/nl80211.h | 17 +++++++++++++++++
> net/wireless/nl80211.c | 32 ++++++++++++++++++++++++++++++--
> 3 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index ef42749..c62c42a 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1626,6 +1626,20 @@ struct cfg80211_sched_scan_plan {
> * cycle. The driver may ignore this parameter and start
> * immediately (or at any other time), if this feature is not
> * supported.
> + * @relative_rssi: Relative RSSI threshold to restrict scan result reporting in
> + * connected state to cases where a matching BSS is determined to have a
> + * significantly better RSSI than the current connected BSS.
> + * @relative_rssi_5g_pref: The amount of RSSI preference that is given to a
> + * 5 GHz BSS over 2.4 GHz BSS while looking for better BSSs in connected
> + * state.
> + * If the current connected BSS is in the 2.4 GHz band, other BSSs in the
> + * 2.4 GHz band to be reported should have better RSSI by @relative_rssi
> + * and other BSSs in the 5 GHz band to be reported should have better RSSI
> + * by (@relative_rssi - @relative_rssi_5g_pref).
> + * If the current connected BSS is in the 5 GHz band, other BSSs in the
> + * 2.4 GHz band to be reported should have better RSSI by
> + * (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
> + * band to be reported should have better RSSI by by @relative_rssi.
> */
> struct cfg80211_sched_scan_request {
> struct cfg80211_ssid *ssids;
> @@ -1645,6 +1659,9 @@ struct cfg80211_sched_scan_request {
> u8 mac_addr[ETH_ALEN] __aligned(2);
> u8 mac_addr_mask[ETH_ALEN] __aligned(2);
>
> + int relative_rssi;
> + int relative_rssi_5g_pref;

I suppose s8 would be sufficient here, right?

> /* internal */
> struct wiphy *wiphy;
> struct net_device *dev;
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 984a35ac..34b16a4 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1981,6 +1981,16 @@ enum nl80211_commands {
> * %NL80211_ATTR_MAC has also been used in various commands/events for
> * specifying the BSSID.
> *
> + * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI: Relative RSSI threshold by which
> + * other BSSs has to be better than the current connected BSS so that they
> + * get reported to user space. This will give an opportunity to userspace
> + * to consider connecting to other matching BSSs which have better RSSI
> + * than the current connected BSS.
> + *
> + * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF: The amount of RSSI preference
> + * to be given to 5 GHz APs over 2.4 GHz APs while searching for better
> + * BSSs than the current connected BSS.
> + *
> * @NUM_NL80211_ATTR: total number of nl80211_attrs available
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2387,6 +2397,9 @@ enum nl80211_attrs {
>
> NL80211_ATTR_BSSID,
>
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
> +
> /* add attributes here, update the policy in nl80211.c */
>
> __NL80211_ATTR_AFTER_LAST,
> @@ -4698,6 +4711,9 @@ enum nl80211_feature_flags {
> * configuration (AP/mesh) with VHT rates.
> * @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
> * with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
> + * @NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS: The driver supports sched_scan
> + * for reporting BSSs with better RSSI than the current connected BSS
> + * (%NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI).
> *
> * @NUM_NL80211_EXT_FEATURES: number of extended features.
> * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
> @@ -4713,6 +4729,7 @@ enum nl80211_ext_feature_index {
> NL80211_EXT_FEATURE_BEACON_RATE_HT,
> NL80211_EXT_FEATURE_BEACON_RATE_VHT,
> NL80211_EXT_FEATURE_FILS_STA,
> + NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS,
>
> /* add new features before the definition below */
> NUM_NL80211_EXT_FEATURES,
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 8db5cb1..af018a5 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -405,6 +405,8 @@ enum nl80211_multicast_groups {
> [NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
> [NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
> [NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
> + [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] = { .type = NLA_U32 },
> + [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF] = { .type = NLA_U32 },

Related to earlier comment this could be NLA_U8.

> };
>
> /* policy for the key attributes */
> @@ -6856,6 +6858,7 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
> size_t ie_len;
> struct nlattr *tb[NL80211_SCHED_SCAN_MATCH_ATTR_MAX + 1];
> s32 default_match_rssi = NL80211_SCAN_RSSI_THOLD_OFF;
> + int bbr;
>
> if (!is_valid_ie_attr(attrs[NL80211_ATTR_IE]))
> return ERR_PTR(-EINVAL);
> @@ -6950,6 +6953,13 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
> if (!n_plans || n_plans > wiphy->max_sched_scan_plans)
> return ERR_PTR(-EINVAL);
>
> + bbr = wiphy_ext_feature_isset(
> + wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS);
> + if (!bbr &&
> + (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] ||
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]))
> + return ERR_PTR(-EINVAL);
> +
> request = kzalloc(sizeof(*request)
> + sizeof(*request->ssids) * n_ssids
> + sizeof(*request->match_sets) * n_match_sets
> @@ -7156,6 +7166,14 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
> request->delay =
> nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_DELAY]);
>
> + if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI])
> + request->relative_rssi = nla_get_s32(
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
> +
> + if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF])
> + request->relative_rssi_5g_pref = nla_get_s32(
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]);
> +
> err = nl80211_parse_sched_scan_plans(wiphy, n_plans, request, attrs);
> if (err)
> goto out_free;
> @@ -9649,7 +9667,8 @@ static int nl80211_send_wowlan_tcp(struct sk_buff *msg,
> return 0;
> }
>
> -static int nl80211_send_wowlan_nd(struct sk_buff *msg,
> +static int nl80211_send_wowlan_nd(struct wiphy *wiphy,
> + struct sk_buff *msg,
> struct cfg80211_sched_scan_request *req)
> {
> struct nlattr *nd, *freqs, *matches, *match, *scan_plans, *scan_plan;
> @@ -9670,6 +9689,15 @@ static int nl80211_send_wowlan_nd(struct sk_buff *msg,
> if (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_DELAY, req->delay))
> return -ENOBUFS;
>
> + if (wiphy_ext_feature_isset(
> + wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS) &&
> + (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> + req->relative_rssi) ||
> + nla_put_u32(msg,
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
> + req->relative_rssi_5g_pref)))
> + return -ENOBUFS;
> +
> freqs = nla_nest_start(msg, NL80211_ATTR_SCAN_FREQUENCIES);
> if (!freqs)
> return -ENOBUFS;
> @@ -9783,7 +9811,7 @@ static int nl80211_get_wowlan(struct sk_buff *skb, struct genl_info *info)
> goto nla_put_failure;
>
> if (nl80211_send_wowlan_nd(
> - msg,
> + &rdev->wiphy, msg,
> rdev->wiphy.wowlan_config->nd_config))
> goto nla_put_failure;
>
>

2016-11-23 22:08:10

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs

From: vamsi krishna <[email protected]>

Enhance sched scan to support option of finding a better BSS while in
connected state. Firmware scans the medium and reports when it finds a
known BSS which has a significantly better RSSI than the current
connected BSS.

Signed-off-by: vamsi krishna <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
include/net/cfg80211.h | 17 +++++++++++++++++
include/uapi/linux/nl80211.h | 17 +++++++++++++++++
net/wireless/nl80211.c | 32 ++++++++++++++++++++++++++++++--
3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ef42749..c62c42a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1626,6 +1626,20 @@ struct cfg80211_sched_scan_plan {
* cycle. The driver may ignore this parameter and start
* immediately (or at any other time), if this feature is not
* supported.
+ * @relative_rssi: Relative RSSI threshold to restrict scan result reporting in
+ * connected state to cases where a matching BSS is determined to have a
+ * significantly better RSSI than the current connected BSS.
+ * @relative_rssi_5g_pref: The amount of RSSI preference that is given to a
+ * 5 GHz BSS over 2.4 GHz BSS while looking for better BSSs in connected
+ * state.
+ * If the current connected BSS is in the 2.4 GHz band, other BSSs in the
+ * 2.4 GHz band to be reported should have better RSSI by @relative_rssi
+ * and other BSSs in the 5 GHz band to be reported should have better RSSI
+ * by (@relative_rssi - @relative_rssi_5g_pref).
+ * If the current connected BSS is in the 5 GHz band, other BSSs in the
+ * 2.4 GHz band to be reported should have better RSSI by
+ * (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
+ * band to be reported should have better RSSI by by @relative_rssi.
*/
struct cfg80211_sched_scan_request {
struct cfg80211_ssid *ssids;
@@ -1645,6 +1659,9 @@ struct cfg80211_sched_scan_request {
u8 mac_addr[ETH_ALEN] __aligned(2);
u8 mac_addr_mask[ETH_ALEN] __aligned(2);

+ int relative_rssi;
+ int relative_rssi_5g_pref;
+
/* internal */
struct wiphy *wiphy;
struct net_device *dev;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 984a35ac..34b16a4 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1981,6 +1981,16 @@ enum nl80211_commands {
* %NL80211_ATTR_MAC has also been used in various commands/events for
* specifying the BSSID.
*
+ * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI: Relative RSSI threshold by which
+ * other BSSs has to be better than the current connected BSS so that they
+ * get reported to user space. This will give an opportunity to userspace
+ * to consider connecting to other matching BSSs which have better RSSI
+ * than the current connected BSS.
+ *
+ * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF: The amount of RSSI preference
+ * to be given to 5 GHz APs over 2.4 GHz APs while searching for better
+ * BSSs than the current connected BSS.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2387,6 +2397,9 @@ enum nl80211_attrs {

NL80211_ATTR_BSSID,

+ NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
+ NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -4698,6 +4711,9 @@ enum nl80211_feature_flags {
* configuration (AP/mesh) with VHT rates.
* @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
* with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
+ * @NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS: The driver supports sched_scan
+ * for reporting BSSs with better RSSI than the current connected BSS
+ * (%NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI).
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4713,6 +4729,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_BEACON_RATE_HT,
NL80211_EXT_FEATURE_BEACON_RATE_VHT,
NL80211_EXT_FEATURE_FILS_STA,
+ NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS,

/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8db5cb1..af018a5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -405,6 +405,8 @@ enum nl80211_multicast_groups {
[NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
[NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
+ [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] = { .type = NLA_U32 },
+ [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF] = { .type = NLA_U32 },
};

/* policy for the key attributes */
@@ -6856,6 +6858,7 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
size_t ie_len;
struct nlattr *tb[NL80211_SCHED_SCAN_MATCH_ATTR_MAX + 1];
s32 default_match_rssi = NL80211_SCAN_RSSI_THOLD_OFF;
+ int bbr;

if (!is_valid_ie_attr(attrs[NL80211_ATTR_IE]))
return ERR_PTR(-EINVAL);
@@ -6950,6 +6953,13 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
if (!n_plans || n_plans > wiphy->max_sched_scan_plans)
return ERR_PTR(-EINVAL);

+ bbr = wiphy_ext_feature_isset(
+ wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS);
+ if (!bbr &&
+ (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] ||
+ attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]))
+ return ERR_PTR(-EINVAL);
+
request = kzalloc(sizeof(*request)
+ sizeof(*request->ssids) * n_ssids
+ sizeof(*request->match_sets) * n_match_sets
@@ -7156,6 +7166,14 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
request->delay =
nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_DELAY]);

+ if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI])
+ request->relative_rssi = nla_get_s32(
+ attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
+
+ if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF])
+ request->relative_rssi_5g_pref = nla_get_s32(
+ attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]);
+
err = nl80211_parse_sched_scan_plans(wiphy, n_plans, request, attrs);
if (err)
goto out_free;
@@ -9649,7 +9667,8 @@ static int nl80211_send_wowlan_tcp(struct sk_buff *msg,
return 0;
}

-static int nl80211_send_wowlan_nd(struct sk_buff *msg,
+static int nl80211_send_wowlan_nd(struct wiphy *wiphy,
+ struct sk_buff *msg,
struct cfg80211_sched_scan_request *req)
{
struct nlattr *nd, *freqs, *matches, *match, *scan_plans, *scan_plan;
@@ -9670,6 +9689,15 @@ static int nl80211_send_wowlan_nd(struct sk_buff *msg,
if (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_DELAY, req->delay))
return -ENOBUFS;

+ if (wiphy_ext_feature_isset(
+ wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS) &&
+ (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
+ req->relative_rssi) ||
+ nla_put_u32(msg,
+ NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
+ req->relative_rssi_5g_pref)))
+ return -ENOBUFS;
+
freqs = nla_nest_start(msg, NL80211_ATTR_SCAN_FREQUENCIES);
if (!freqs)
return -ENOBUFS;
@@ -9783,7 +9811,7 @@ static int nl80211_get_wowlan(struct sk_buff *skb, struct genl_info *info)
goto nla_put_failure;

if (nl80211_send_wowlan_nd(
- msg,
+ &rdev->wiphy, msg,
rdev->wiphy.wowlan_config->nd_config))
goto nla_put_failure;

--
1.9.1

2016-12-02 21:48:23

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] cfg80211: Add support to sched scan to report better BSSs

On Fri, Nov 25, 2016 at 11:21:15AM +0200, Luca Coelho wrote:
> On Thu, 2016-11-24 at 00:07 +0200, Jouni Malinen wrote:
> > From: vamsi krishna <[email protected]>
> > @@ -9670,6 +9689,15 @@ static int nl80211_send_wowlan_nd(struct sk_buff=
*msg,
> > if (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_DELAY, req->delay))
> > return -ENOBUFS;
> > =20
> > + if (wiphy_ext_feature_isset(
> > + wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_BETTER_BSS) &&
> > + (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> > + req->relative_rssi) ||
> > + nla_put_u32(msg,
> > + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
> > + req->relative_rssi_5g_pref)))
> > + return -ENOBUFS;
> > +
>=20
> Why did you add this to nl80211_send_wowlan_nd() function?

Rest of the feedback will be addressed in PATCH v2, but I'm not sure
what to do with this part. I don't think we have any use case for this,
i.e., the addition here for NL80211_CMD_GET_WOWLAN response attributes
is based on how other sched_scan attributes were already included in the
response. Any new attribute atted to nl80211_parse_sched_scan() (like
these two new attributes) get parsed into
rdev->wiphy.wowlan_config->nd_config in nl80211_wowlan_nd(), so they can
end up being set here when using NL80211_CMD_SET_WOWLAN.
=20
--=20
Jouni Malinen PGP id EFC895FA=