2014-01-06 07:58:22

by Jouni Malinen

[permalink] [raw]
Subject: [RFC 1/3] cfg80211: Allow BSS hint to be provided for connect

This clarifies the expected driver behavior on the older
NL80211_ATTR_MAC and NL80211_ATTR_WIPHY_FREQ attributes and adds a new
set of similar attributes with _HINT postfix to enable use of a
recommendation of the initial BSS to choose. This can be helpful for
some drivers that can avoid an additional full scan on connection
request if the information is provided to them (user space tools like
wpa_supplicant already has that information available based on earlier
scans).

In addition, this can be used to get more expected behavior for cases
where a specific BSS should be picked first based on operations like
Interworking network selection or WPS. These cases were already easily
addressed with drivers that leave BSS selection to user space, but there
was no convenient way to do this with drivers that take care of BSS
selection internally without using the NL80211_ATTR_MAC which is not
really desired since it is needed for other purposes to force the
association to remain with the same BSS.

Signed-off-by: Jouni Malinen <[email protected]>
---
include/net/cfg80211.h | 8 ++++++++
include/uapi/linux/nl80211.h | 20 ++++++++++++++++++--
net/wireless/nl80211.c | 12 ++++++++++++
3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 56c5977..8b5777a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1701,8 +1701,14 @@ struct cfg80211_ibss_params {
*
* @channel: The channel to use or %NULL if not specified (auto-select based
* on scan results)
+ * @channel_hint: The channel of the recommended BSS for initial connection or
+ * %NULL if not specified
* @bssid: The AP BSSID or %NULL if not specified (auto-select based on scan
* results)
+ * @bssid_hint: The recommended AP BSSID for initial connection to the BSS or
+ * %NULL if not specified. Unlike the @bssid parameter, the driver is
+ * allowed to ignore this @bssid_hint if it has knowledge of a better BSS
+ * to use.
* @ssid: SSID
* @ssid_len: Length of ssid in octets
* @auth_type: Authentication type (algorithm)
@@ -1725,7 +1731,9 @@ struct cfg80211_ibss_params {
*/
struct cfg80211_connect_params {
struct ieee80211_channel *channel;
+ struct ieee80211_channel *channel_hint;
u8 *bssid;
+ u8 *bssid_hint;
u8 *ssid;
size_t ssid_len;
enum nl80211_auth_type auth_type;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 91054fd..e57de33 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -418,8 +418,18 @@
* %NL80211_ATTR_SSID attribute, and can optionally specify the association
* IEs in %NL80211_ATTR_IE, %NL80211_ATTR_AUTH_TYPE, %NL80211_ATTR_USE_MFP,
* %NL80211_ATTR_MAC, %NL80211_ATTR_WIPHY_FREQ, %NL80211_ATTR_CONTROL_PORT,
- * %NL80211_ATTR_CONTROL_PORT_ETHERTYPE and
- * %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT.
+ * %NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
+ * %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT, %NL80211_ATTR_MAC_HINT, and
+ * %NL80211_ATTR_WIPHY_FREQ_HINT.
+ * If included, %NL80211_ATTR_MAC and %NL80211_ATTR_WIPHY_FREQ are
+ * restrictions on BSS selection, i.e., they effectively prevent roaming
+ * within the ESS. %NL80211_ATTR_MAC_HINT and %NL80211_ATTR_WIPHY_FREQ_HINT
+ * can be included to provide a recommendation of the initial BSS while
+ * allowing the driver to roam to other BSSes within the ESS and also to
+ * ignore this recommendation if the indicated BSS is not ideal. Only one
+ * set of BSSID,frequency parameters is used (i.e., either the enforcing
+ * %NL80211_ATTR_MAC,%NL80211_ATTR_WIPHY_FREQ or the less strict
+ * %NL80211_ATTR_MAC_HINT and %NL80211_ATTR_WIPHY_FREQ_HINT).
* Background scan period can optionally be
* specified in %NL80211_ATTR_BG_SCAN_PERIOD,
* if not specified default background scan configuration
@@ -1555,6 +1565,9 @@ enum nl80211_commands {
* data is in the format defined for the payload of the QoS Map Set element
* in IEEE Std 802.11-2012, 8.4.2.97.
*
+ * @NL80211_ATTR_MAC_HINT: MAC address recommendation as initial BSS
+ * @NL80211_ATTR_WIPHY_FREQ_HINT: frequency of the recommended initial BSS
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1883,6 +1896,9 @@ enum nl80211_attrs {

NL80211_ATTR_QOS_MAP,

+ NL80211_ATTR_MAC_HINT,
+ NL80211_ATTR_WIPHY_FREQ_HINT,
+
/* 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 81bd2ae..a3a6fb7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -384,6 +384,8 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_VENDOR_DATA] = { .type = NLA_BINARY },
[NL80211_ATTR_QOS_MAP] = { .type = NLA_BINARY,
.len = IEEE80211_QOS_MAP_LEN_MAX },
+ [NL80211_ATTR_MAC_HINT] = { .len = ETH_ALEN },
+ [NL80211_ATTR_WIPHY_FREQ] = { .type = NLA_U32 },
};

/* policy for the key attributes */
@@ -7021,6 +7023,9 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)

if (info->attrs[NL80211_ATTR_MAC])
connect.bssid = nla_data(info->attrs[NL80211_ATTR_MAC]);
+ else if (info->attrs[NL80211_ATTR_MAC_HINT])
+ connect.bssid_hint =
+ nla_data(info->attrs[NL80211_ATTR_MAC_HINT]);
connect.ssid = nla_data(info->attrs[NL80211_ATTR_SSID]);
connect.ssid_len = nla_len(info->attrs[NL80211_ATTR_SSID]);

@@ -7045,6 +7050,13 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
if (!connect.channel ||
connect.channel->flags & IEEE80211_CHAN_DISABLED)
return -EINVAL;
+ } else if (info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]) {
+ connect.channel_hint =
+ ieee80211_get_channel(wiphy,
+ nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]));
+ if (!connect.channel_hint ||
+ connect.channel_hint->flags & IEEE80211_CHAN_DISABLED)
+ return -EINVAL;
}

if (connect.privacy && info->attrs[NL80211_ATTR_KEYS]) {
--
1.7.9.5


--
Jouni Malinen PGP id EFC895FA


2014-01-14 18:20:08

by Jouni Malinen

[permalink] [raw]
Subject: Re: [RFC 1/3] cfg80211: Allow BSS hint to be provided for connect

On Tue, Jan 07, 2014 at 04:56:24PM +0100, Johannes Berg wrote:
> > struct cfg80211_connect_params {
> > struct ieee80211_channel *channel;
> > + struct ieee80211_channel *channel_hint;
> > u8 *bssid;
> > + u8 *bssid_hint;
> > u8 *ssid;
>
> Those should probably all be const, but we can do that separately.

These are easy (see below). The "u8 *ie" below would require more work
since brcm80211 uses it in a non-const-friendly-way (in its own IE
parse?!)..

> > + } else if (info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]) {
> > + connect.channel_hint =
> > + ieee80211_get_channel(wiphy,
> > + nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]));
> > + if (!connect.channel_hint ||
> > + connect.channel_hint->flags & IEEE80211_CHAN_DISABLED)
> > + return -EINVAL;
>
> I'm starting to wonder if this pattern should be abstracted out, but
> again, we can do that separately.

Something like this could work.. Only compile tested, though.


diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 27dd032..9237b26 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1732,9 +1732,9 @@ struct cfg80211_ibss_params {
struct cfg80211_connect_params {
struct ieee80211_channel *channel;
struct ieee80211_channel *channel_hint;
- u8 *bssid;
- u8 *bssid_hint;
- u8 *ssid;
+ const u8 *bssid;
+ const u8 *bssid_hint;
+ const u8 *ssid;
size_t ssid_len;
enum nl80211_auth_type auth_type;
u8 *ie;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 55f8e83..1385276 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -859,6 +859,19 @@ static int nl80211_key_allowed(struct wireless_dev *wdev)
return 0;
}

+static struct ieee80211_channel * nl80211_get_valid_chan(struct wiphy *wiphy,
+ struct nlattr *tb)
+{
+ struct ieee80211_channel *chan;
+
+ if (tb == NULL)
+ return NULL;
+ chan = ieee80211_get_channel(wiphy, nla_get_u32(tb));
+ if (!chan || chan->flags & IEEE80211_CHAN_DISABLED)
+ return NULL;
+ return chan;
+}
+
static int nl80211_put_iftypes(struct sk_buff *msg, u32 attr, u16 ifmodes)
{
struct nlattr *nl_modes = nla_nest_start(msg, attr);
@@ -6227,9 +6240,9 @@ static int nl80211_authenticate(struct sk_buff *skb, struct genl_info *info)
return -EOPNOTSUPP;

bssid = nla_data(info->attrs[NL80211_ATTR_MAC]);
- chan = ieee80211_get_channel(&rdev->wiphy,
- nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]));
- if (!chan || (chan->flags & IEEE80211_CHAN_DISABLED))
+ chan = nl80211_get_valid_chan(&rdev->wiphy,
+ info->attrs[NL80211_ATTR_WIPHY_FREQ]);
+ if (!chan)
return -EINVAL;

ssid = nla_data(info->attrs[NL80211_ATTR_SSID]);
@@ -6382,9 +6395,9 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)

bssid = nla_data(info->attrs[NL80211_ATTR_MAC]);

- chan = ieee80211_get_channel(&rdev->wiphy,
- nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]));
- if (!chan || (chan->flags & IEEE80211_CHAN_DISABLED))
+ chan = nl80211_get_valid_chan(&rdev->wiphy,
+ info->attrs[NL80211_ATTR_WIPHY_FREQ]);
+ if (!chan)
return -EINVAL;

ssid = nla_data(info->attrs[NL80211_ATTR_SSID]);
@@ -7041,18 +7054,14 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
}

if (info->attrs[NL80211_ATTR_WIPHY_FREQ]) {
- connect.channel =
- ieee80211_get_channel(wiphy,
- nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]));
- if (!connect.channel ||
- connect.channel->flags & IEEE80211_CHAN_DISABLED)
+ connect.channel = nl80211_get_valid_chan(
+ wiphy, info->attrs[NL80211_ATTR_WIPHY_FREQ]);
+ if (!connect.channel)
return -EINVAL;
} else if (info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]) {
- connect.channel_hint =
- ieee80211_get_channel(wiphy,
- nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]));
- if (!connect.channel_hint ||
- connect.channel_hint->flags & IEEE80211_CHAN_DISABLED)
+ connect.channel_hint = nl80211_get_valid_chan(
+ wiphy, info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]);
+ if (!connect.channel_hint)
return -EINVAL;
}


--
Jouni Malinen PGP id EFC895FA

2014-01-07 15:56:27

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 1/3] cfg80211: Allow BSS hint to be provided for connect

On Mon, 2014-01-06 at 09:51 +0200, Jouni Malinen wrote:
> This clarifies the expected driver behavior on the older
> NL80211_ATTR_MAC and NL80211_ATTR_WIPHY_FREQ attributes and adds a new
> set of similar attributes with _HINT postfix to enable use of a
> recommendation of the initial BSS to choose. This can be helpful for
> some drivers that can avoid an additional full scan on connection
> request if the information is provided to them (user space tools like
> wpa_supplicant already has that information available based on earlier
> scans).
>
> In addition, this can be used to get more expected behavior for cases
> where a specific BSS should be picked first based on operations like
> Interworking network selection or WPS. These cases were already easily
> addressed with drivers that leave BSS selection to user space, but there
> was no convenient way to do this with drivers that take care of BSS
> selection internally without using the NL80211_ATTR_MAC which is not
> really desired since it is needed for other purposes to force the
> association to remain with the same BSS.

Makes sense.

> struct cfg80211_connect_params {
> struct ieee80211_channel *channel;
> + struct ieee80211_channel *channel_hint;
> u8 *bssid;
> + u8 *bssid_hint;
> u8 *ssid;

Those should probably all be const, but we can do that separately.


> return -EINVAL;
> + } else if (info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]) {
> + connect.channel_hint =
> + ieee80211_get_channel(wiphy,
> + nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ_HINT]));
> + if (!connect.channel_hint ||
> + connect.channel_hint->flags & IEEE80211_CHAN_DISABLED)
> + return -EINVAL;

I'm starting to wonder if this pattern should be abstracted out, but
again, we can do that separately.

johannes