diff mbox

[v2,1/3] cfg80211: Allow a scan request for a specific BSSID

Message ID 1456517569-27235-1-git-send-email-jouni@qca.qualcomm.com (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show

Commit Message

Jouni Malinen Feb. 26, 2016, 8:12 p.m. UTC
This allows scans for a specific BSSID to be optimized by the user space
application by requesting the driver to set the Probe Request frame
BSSID field (Address 3) to the specified BSSID instead of the wildcard
BSSID. This prevents other APs from replying which reduces airtime need
and latency in getting the response from the target AP through.

This is an optimization and as such, it is acceptable for some of the
drivers not to support the mechanism. If not supported, the wildcard
BSSID will be used and more responses may be received.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---

Notes:
    v2: Updated two forgotten rdev_scan() callers

 include/net/cfg80211.h       | 2 ++
 include/uapi/linux/nl80211.h | 4 +++-
 net/wireless/nl80211.c       | 6 ++++++
 net/wireless/scan.c          | 2 ++
 net/wireless/sme.c           | 2 ++
 5 files changed, 15 insertions(+), 1 deletion(-)

Comments

Johannes Berg March 3, 2016, 3:11 p.m. UTC | #1
On Fri, 2016-02-26 at 22:12 +0200, Jouni Malinen wrote:
> This allows scans for a specific BSSID to be optimized by the user
> space
> application by requesting the driver to set the Probe Request frame
> BSSID field (Address 3) to the specified BSSID instead of the
> wildcard
> BSSID. This prevents other APs from replying which reduces airtime
> need
> and latency in getting the response from the target AP through.
> 
> This is an optimization and as such, it is acceptable for some of the
> drivers not to support the mechanism. If not supported, the wildcard
> BSSID will be used and more responses may be received.
> 
Applied all three.

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luca Coelho March 4, 2016, 8:07 a.m. UTC | #2
Looks good.  A couple of small comments.

On Fri, 2016-02-26 at 22:12 +0200, Jouni Malinen wrote:
> This allows scans for a specific BSSID to be optimized by the user space
> application by requesting the driver to set the Probe Request frame
> BSSID field (Address 3) to the specified BSSID instead of the wildcard
> BSSID. This prevents other APs from replying which reduces airtime need
> and latency in getting the response from the target AP through.
> 
> This is an optimization and as such, it is acceptable for some of the
> drivers not to support the mechanism. If not supported, the wildcard
> BSSID will be used and more responses may be received.

Receiving more responses than what we asked for is always possible,
because we don't filter out other things we may receive at the same
time.  It's like requesting for a specific SSID, it just means that the
SSID will be used in the probe requests, but it doesn't guarantee that
other results will not be returned.


> Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
> ---
> 
> Notes:
>     v2: Updated two forgotten rdev_scan() callers
> 
>  include/net/cfg80211.h       | 2 ++
>  include/uapi/linux/nl80211.h | 4 +++-
>  net/wireless/nl80211.c       | 6 ++++++
>  net/wireless/scan.c          | 2 ++
>  net/wireless/sme.c           | 2 ++
>  5 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 9e1b24c..14c0c43 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1455,6 +1455,7 @@ struct cfg80211_ssid {
>   * @mac_addr_mask: MAC address mask used with randomisation, bits that
>   *	are 0 in the mask should be randomised, bits that are 1 should
>   *	be taken from the @mac_addr
> + * @bssid: BSSID to scan for (most commonly, the wildcard BSSID)
>   */
>  struct cfg80211_scan_request {
>  	struct cfg80211_ssid *ssids;
> @@ -1471,6 +1472,7 @@ struct cfg80211_scan_request {
>  
>  	u8 mac_addr[ETH_ALEN] __aligned(2);
>  	u8 mac_addr_mask[ETH_ALEN] __aligned(2);
> +	u8 bssid[ETH_ALEN] __aligned(2);
>  
>  	/* internal */
>  	struct wiphy *wiphy;
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 5a30a75..23bf066 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -322,7 +322,9 @@
>   * @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.
> + *	probe requests at CCK rate or not. %NL80211_ATTR_MAC can be used to
> + *	specify a BSSID to scan for; if not included, the wildcard BSSID will
> + *	be used.

Maybe you could be explicit here about the possibility of receiving
more results than requested?

--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jouni Malinen March 4, 2016, 9:15 a.m. UTC | #3
On Fri, Mar 04, 2016 at 10:07:56AM +0200, Luca Coelho wrote:
> On Fri, 2016-02-26 at 22:12 +0200, Jouni Malinen wrote:

> > This is an optimization and as such, it is acceptable for some of the

> > drivers not to support the mechanism. If not supported, the wildcard

> > BSSID will be used and more responses may be received.

> 

> Receiving more responses than what we asked for is always possible,

> because we don't filter out other things we may receive at the same

> time.  It's like requesting for a specific SSID, it just means that the

> SSID will be used in the probe requests, but it doesn't guarantee that

> other results will not be returned.


Sure. This "more responses" is referring to not being able to get the
benefit from saved airtime; not on what we might add into the cfg80211
BSS table. We don't even have an explicit mechanism for reporting
results from a single scan iteration.

> > diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h

> > @@ -322,7 +322,9 @@

> >   * @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.

> > + *	probe requests at CCK rate or not. %NL80211_ATTR_MAC can be used to

> > + *	specify a BSSID to scan for; if not included, the wildcard BSSID will

> > + *	be used.

> 

> Maybe you could be explicit here about the possibility of receiving

> more results than requested?


Which "receiving" are you referring to here? I don't see much point in
documenting the airtime optimization here and as far as receiving
results through nl80211 is concerned, the only option for it today is to
use NL80211_CMD_GET_SCAN and that returns the current contents of the
BSS table rather than returns from any specific scan, so I'm not sure
what exactly you would like to add here.

I was going to note that this is similar to how NL80211_ATTR_SCAN_SSIDS
is documented, but actually, that one is not mentioned at all with
NL80211_CMD_TRIGGER_SCAN currently.. Nor is the details of what
NL80211_CMD_GET_SCAN returns.
 
-- 
Jouni Malinen                                            PGP id EFC895FA
Luca Coelho March 4, 2016, 9:39 a.m. UTC | #4
On Fri, 2016-03-04 at 09:15 +0000, Malinen, Jouni wrote:
> On Fri, Mar 04, 2016 at 10:07:56AM +0200, Luca Coelho wrote:
> > On Fri, 2016-02-26 at 22:12 +0200, Jouni Malinen wrote:
> > > This is an optimization and as such, it is acceptable for some of
> > > the
> > > drivers not to support the mechanism. If not supported, the
> > > wildcard
> > > BSSID will be used and more responses may be received.
> > 
> > Receiving more responses than what we asked for is always possible,
> > because we don't filter out other things we may receive at the same
> > time.  It's like requesting for a specific SSID, it just means that
> > the
> > SSID will be used in the probe requests, but it doesn't guarantee
> > that
> > other results will not be returned.
> 
> Sure. This "more responses" is referring to not being able to get the
> benefit from saved airtime; not on what we might add into the
> cfg80211
> BSS table. We don't even have an explicit mechanism for reporting
> results from a single scan iteration.

Right.


> > > diff --git a/include/uapi/linux/nl80211.h
> > > b/include/uapi/linux/nl80211.h
> > > @@ -322,7 +322,9 @@
> > >   * @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.
> > > + *	probe requests at CCK rate or not. %NL80211_ATTR_MAC
> > > can be used to
> > > + *	specify a BSSID to scan for; if not included, the
> > > wildcard BSSID will
> > > + *	be used.
> > 
> > Maybe you could be explicit here about the possibility of receiving
> > more results than requested?
> 
> Which "receiving" are you referring to here? I don't see much point
> in
> documenting the airtime optimization here and as far as receiving
> results through nl80211 is concerned, the only option for it today is
> to
> use NL80211_CMD_GET_SCAN and that returns the current contents of the
> BSS table rather than returns from any specific scan, so I'm not sure
> what exactly you would like to add here.
> 
> I was going to note that this is similar to how
> NL80211_ATTR_SCAN_SSIDS
> is documented, but actually, that one is not mentioned at all with
> NL80211_CMD_TRIGGER_SCAN currently.. Nor is the details of what
> NL80211_CMD_GET_SCAN returns.

Okay, fair enough.  It's true that understanding how the scan API works
from the nl80211 documentation is pretty hard today.  That's what I
wanted to address, but this should probably be another task in itself.
;)

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9e1b24c..14c0c43 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1455,6 +1455,7 @@  struct cfg80211_ssid {
  * @mac_addr_mask: MAC address mask used with randomisation, bits that
  *	are 0 in the mask should be randomised, bits that are 1 should
  *	be taken from the @mac_addr
+ * @bssid: BSSID to scan for (most commonly, the wildcard BSSID)
  */
 struct cfg80211_scan_request {
 	struct cfg80211_ssid *ssids;
@@ -1471,6 +1472,7 @@  struct cfg80211_scan_request {
 
 	u8 mac_addr[ETH_ALEN] __aligned(2);
 	u8 mac_addr_mask[ETH_ALEN] __aligned(2);
+	u8 bssid[ETH_ALEN] __aligned(2);
 
 	/* internal */
 	struct wiphy *wiphy;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 5a30a75..23bf066 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -322,7 +322,9 @@ 
  * @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.
+ *	probe requests at CCK rate or not. %NL80211_ATTR_MAC 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
  *	NL80211_CMD_GET_SCAN and on the "scan" multicast group)
  * @NL80211_CMD_SCAN_ABORTED: scan was aborted, for unspecified reasons,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 98c9242..1b43f78 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5996,6 +5996,12 @@  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])
+		memcpy(request->bssid, nla_data(info->attrs[NL80211_ATTR_MAC]),
+		       ETH_ALEN);
+	else
+		eth_broadcast_addr(request->bssid);
+
 	request->wdev = wdev;
 	request->wiphy = &rdev->wiphy;
 	request->scan_start = jiffies;
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 14d5369..50ea8e3 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1293,6 +1293,8 @@  int cfg80211_wext_siwscan(struct net_device *dev,
 		if (wiphy->bands[i])
 			creq->rates[i] = (1 << wiphy->bands[i]->n_bitrates) - 1;
 
+	eth_broadcast_addr(creq->bssid);
+
 	rdev->scan_req = creq;
 	err = rdev_scan(rdev, creq);
 	if (err) {
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 5445581..65882d2 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -119,6 +119,8 @@  static int cfg80211_conn_scan(struct wireless_dev *wdev)
 		wdev->conn->params.ssid_len);
 	request->ssids[0].ssid_len = wdev->conn->params.ssid_len;
 
+	eth_broadcast_addr(request->bssid);
+
 	request->wdev = wdev;
 	request->wiphy = &rdev->wiphy;
 	request->scan_start = jiffies;