Introducing a new feature that the driver can use to
indicate the driver/firmware supports configuration of BSS
selection criteria upon CONNECT command. This can be useful
when multiple BSS-es are found belonging to the same ESS,
ie. Infra-BSS with same SSID. The criteria can then be used to
offload selection of a preferred BSS.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
V7:
- add validatioin using nla_ok().
---
include/net/cfg80211.h | 34 ++++++++++++++
include/uapi/linux/nl80211.h | 52 ++++++++++++++++++++++
net/wireless/core.c | 4 ++
net/wireless/nl80211.c | 104 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 194 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9e1b24c..1dfb890 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1857,6 +1857,33 @@ struct cfg80211_ibss_params {
};
/**
+ * struct cfg80211_bss_select_adjust - BSS selection with RSSI adjustment.
+ *
+ * @band: band of BSS which should match for RSSI level adjustment.
+ * @delta: value of RSSI level adjustment.
+ */
+struct cfg80211_bss_select_adjust {
+ enum nl80211_band band;
+ s8 delta;
+};
+
+/**
+ * struct cfg80211_bss_selection - connection parameters for BSS selection.
+ *
+ * @behaviour: requested BSS selection behaviour.
+ * @param: parameters for requestion behaviour.
+ * @band_pref: preferred band for %NL80211_BSS_SELECT_ATTR_BAND_PREF.
+ * @adjust: parameters for %NL80211_BSS_SELECT_ATTR_RSSI_ADJUST.
+ */
+struct cfg80211_bss_selection {
+ enum nl80211_bss_select_attr behaviour;
+ union {
+ enum nl80211_band band_pref;
+ struct cfg80211_bss_select_adjust adjust;
+ } param;
+};
+
+/**
* struct cfg80211_connect_params - Connection parameters
*
* This structure provides information needed to complete IEEE 802.11
@@ -1893,6 +1920,7 @@ struct cfg80211_ibss_params {
* @vht_capa_mask: The bits of vht_capa which are to be used.
* @pbss: if set, connect to a PCP instead of AP. Valid for DMG
* networks.
+ * @bss_select: criteria to be used for BSS selection.
*/
struct cfg80211_connect_params {
struct ieee80211_channel *channel;
@@ -1916,6 +1944,7 @@ struct cfg80211_connect_params {
struct ieee80211_vht_cap vht_capa;
struct ieee80211_vht_cap vht_capa_mask;
bool pbss;
+ struct cfg80211_bss_selection bss_select;
};
/**
@@ -3184,6 +3213,9 @@ struct wiphy_vendor_command {
* low rssi when a frame is heard on different channel, then it should set
* this variable to the maximal offset for which it can compensate.
* This value should be set in MHz.
+ * @bss_select_support: bitmask indicating the BSS selection criteria supported
+ * by the driver in the .connect() callback. The bit position maps to the
+ * attribute indices defined in &enum nl80211_bss_select_attr.
*/
struct wiphy {
/* assign these fields before you register the wiphy */
@@ -3306,6 +3338,8 @@ struct wiphy {
u8 max_num_csa_counters;
u8 max_adj_channel_rssi_comp;
+ u32 bss_select_support;
+
char priv[0] __aligned(NETDEV_ALIGN);
};
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 7758969..1f25fbf5 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1793,6 +1793,12 @@ enum nl80211_commands {
* in a PBSS. Specified in %NL80211_CMD_CONNECT to request
* connecting to a PCP, and in %NL80211_CMD_START_AP to start
* a PCP instead of AP. Relevant for DMG networks only.
+ * @NL80211_ATTR_BSS_SELECT: nested attribute for driver supporting the
+ * BSS selection feature. When used with %NL80211_CMD_GET_WIPHY it contains
+ * NLA_FLAG type according &enum nl80211_bss_select_attr to indicate what
+ * BSS selection behaviours are supported. When used with %NL80211_CMD_CONNECT
+ * it contains the behaviour and parameters for BSS selection to be done by
+ * driver and/or firmware.
*
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
@@ -2169,6 +2175,7 @@ enum nl80211_attrs {
NL80211_ATTR_SCHED_SCAN_PLANS,
NL80211_ATTR_PBSS,
+ NL80211_ATTR_BSS_SELECT,
/* add attributes here, update the policy in nl80211.c */
@@ -3617,6 +3624,7 @@ enum nl80211_band {
NL80211_BAND_2GHZ,
NL80211_BAND_5GHZ,
NL80211_BAND_60GHZ,
+ NUM_NL80211_BAND,
};
/**
@@ -4657,4 +4665,48 @@ enum nl80211_sched_scan_plan {
__NL80211_SCHED_SCAN_PLAN_AFTER_LAST - 1
};
+/**
+ * struct nl80211_bss_select_rssi_adjust - RSSI adjustment parameters.
+ *
+ * @band: band of BSS that must match for RSSI value adjustment.
+ * @delta: value used to adjust the RSSI value of matching BSS.
+ */
+struct nl80211_bss_select_rssi_adjust {
+ enum nl80211_band band;
+ __s8 delta;
+} __attribute__((packed));
+
+/**
+ * enum nl80211_bss_select_attr - attributes for bss selection.
+ *
+ * @__NL80211_BSS_SELECT_ATTR_INVALID: reserved.
+ * @NL80211_BSS_SELECT_ATTR_RSSI: Flag indicating only RSSI-based BSS selection
+ * is requested.
+ * @NL80211_BSS_SELECT_ATTR_BAND_PREF: attribute indicating BSS
+ * selection should be done such that the specified band is preferred.
+ * When there are multiple BSS-es in the preferred band, the driver
+ * shall use RSSI-based BSS selection as a second step. The value of
+ * this attribute is according to &enum nl80211_band (u32).
+ * @NL80211_BSS_SELECT_ATTR_RSSI_ADJUST: When present the RSSI level for
+ * BSS-es in the specified band is to be adjusted before doing
+ * RSSI-based BSS selection. The attribute value is a packed structure
+ * value as specified by &struct nl80211_bss_select_rssi_adjust.
+ * @NL80211_BSS_SELECT_ATTR_MAX: highest bss select attribute number.
+ * @__NL80211_BSS_SELECT_ATTR_AFTER_LAST: internal use.
+ *
+ * One and only one of these attributes are found within %NL80211_ATTR_BSS_SELECT
+ * for %NL80211_CMD_CONNECT. It specifies the required BSS selection behaviour
+ * which the driver shall use.
+ */
+enum nl80211_bss_select_attr {
+ __NL80211_BSS_SELECT_ATTR_INVALID,
+ NL80211_BSS_SELECT_ATTR_RSSI,
+ NL80211_BSS_SELECT_ATTR_BAND_PREF,
+ NL80211_BSS_SELECT_ATTR_RSSI_ADJUST,
+
+ /* keep last */
+ __NL80211_BSS_SELECT_ATTR_AFTER_LAST,
+ NL80211_BSS_SELECT_ATTR_MAX = __NL80211_BSS_SELECT_ATTR_AFTER_LAST - 1
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 9f1c4aa..0256336 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -626,6 +626,10 @@ int wiphy_register(struct wiphy *wiphy)
!rdev->ops->set_mac_acl)))
return -EINVAL;
+ if (WARN_ON(wiphy->bss_select_support &&
+ (wiphy->bss_select_support & ~(BIT(__NL80211_BSS_SELECT_ATTR_AFTER_LAST) - 2))))
+ return -EINVAL;
+
if (wiphy->addresses)
memcpy(wiphy->perm_addr, wiphy->addresses[0].addr, ETH_ALEN);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 268cb49..319a337 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -486,6 +486,15 @@ nl80211_plan_policy[NL80211_SCHED_SCAN_PLAN_MAX + 1] = {
[NL80211_SCHED_SCAN_PLAN_ITERATIONS] = { .type = NLA_U32 },
};
+static const struct nla_policy
+nl80211_bss_select_policy[NL80211_BSS_SELECT_ATTR_MAX + 1] = {
+ [NL80211_BSS_SELECT_ATTR_RSSI] = { .type = NLA_FLAG },
+ [NL80211_BSS_SELECT_ATTR_BAND_PREF] = { .type = NLA_U32 },
+ [NL80211_BSS_SELECT_ATTR_RSSI_ADJUST] = {
+ .len = sizeof(struct nl80211_bss_select_rssi_adjust)
+ },
+};
+
static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
struct netlink_callback *cb,
struct cfg80211_registered_device **rdev,
@@ -1731,6 +1740,24 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
rdev->wiphy.ext_features))
goto nla_put_failure;
+ if (rdev->wiphy.bss_select_support) {
+ struct nlattr *nested;
+ u32 bss_select_support = rdev->wiphy.bss_select_support;
+
+ nested = nla_nest_start(msg, NL80211_ATTR_BSS_SELECT);
+ if (!nested)
+ goto nla_put_failure;
+
+ i = 0;
+ while (bss_select_support) {
+ if ((bss_select_support & 1) &&
+ nla_put_flag(msg, i))
+ goto nla_put_failure;
+ i++;
+ bss_select_support >>= 1;
+ }
+ nla_nest_end(msg, nested);
+ }
/* done */
state->split_start = 0;
break;
@@ -5758,6 +5785,68 @@ static int validate_scan_freqs(struct nlattr *freqs)
return n_channels;
}
+static bool is_band_valid(struct wiphy *wiphy, enum nl80211_band b)
+{
+ return b < NUM_NL80211_BAND && wiphy->bands[b];
+}
+
+static int parse_bss_select(struct nlattr *nla, struct wiphy *wiphy,
+ struct cfg80211_bss_selection *bss_select)
+{
+ struct nlattr *attr[NL80211_BSS_SELECT_ATTR_MAX + 1];
+ struct nlattr *nest;
+ int err;
+ bool found = false;
+ int i;
+
+ /* only process one nested attribute */
+ nest = nla_data(nla);
+ if (!nla_ok(nest, nla_len(nest)))
+ return -EBADF;
+
+ err = nla_parse(attr, NL80211_BSS_SELECT_ATTR_MAX, nla_data(nest),
+ nla_len(nest), nl80211_bss_select_policy);
+ if (err)
+ return err;
+
+ /* only one attribute may be given */
+ for (i = 0; i <= NL80211_BSS_SELECT_ATTR_MAX; i++)
+ if (attr[i]) {
+ if (found)
+ return -EINVAL;
+ found = true;
+ }
+
+ bss_select->behaviour = __NL80211_BSS_SELECT_ATTR_INVALID;
+
+ if (attr[NL80211_BSS_SELECT_ATTR_RSSI])
+ bss_select->behaviour = NL80211_BSS_SELECT_ATTR_RSSI;
+
+ if (attr[NL80211_BSS_SELECT_ATTR_BAND_PREF]) {
+ bss_select->behaviour = NL80211_BSS_SELECT_ATTR_BAND_PREF;
+ bss_select->param.band_pref =
+ nla_get_u32(attr[NL80211_BSS_SELECT_ATTR_BAND_PREF]);
+ if (!is_band_valid(wiphy, bss_select->param.band_pref))
+ return -EINVAL;
+ }
+ if (attr[NL80211_BSS_SELECT_ATTR_RSSI_ADJUST]) {
+ struct nl80211_bss_select_rssi_adjust *adj_param;
+
+ adj_param = nla_data(attr[NL80211_BSS_SELECT_ATTR_RSSI_ADJUST]);
+ bss_select->behaviour = NL80211_BSS_SELECT_ATTR_RSSI_ADJUST;
+ bss_select->param.adjust.band = adj_param->band;
+ bss_select->param.adjust.delta = adj_param->delta;
+ if (!is_band_valid(wiphy, bss_select->param.adjust.band))
+ return -EINVAL;
+ }
+
+ /* user-space did not provide behaviour attribute */
+ if (bss_select->behaviour == __NL80211_BSS_SELECT_ATTR_INVALID)
+ return -EINVAL;
+
+ return 0;
+}
+
static int nl80211_parse_random_mac(struct nlattr **attrs,
u8 *mac_addr, u8 *mac_addr_mask)
{
@@ -7991,6 +8080,21 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
return -EOPNOTSUPP;
}
+ /* only do bss selection when no BSSID is specified. */
+ if (!connect.bssid && wiphy->bss_select_support &&
+ info->attrs[NL80211_ATTR_BSS_SELECT]) {
+ err = parse_bss_select(info->attrs[NL80211_ATTR_BSS_SELECT],
+ wiphy, &connect.bss_select);
+ if (err) {
+ kzfree(connkeys);
+ return err;
+ }
+ if (!(wiphy->bss_select_support & BIT(connect.bss_select.behaviour))) {
+ kzfree(connkeys);
+ return -EINVAL;
+ }
+ }
+
wdev_lock(dev->ieee80211_ptr);
err = cfg80211_connect(rdev, dev, &connect, connkeys, NULL);
wdev_unlock(dev->ieee80211_ptr);
--
1.9.1
On 23-2-2016 15:20, Johannes Berg wrote:
> On Wed, 2016-02-17 at 11:47 +0100, Arend van Spriel wrote:
>>
>> + * @NL80211_ATTR_BSS_SELECT: nested attribute for driver supporting the
>> + * BSS selection feature. When used with %NL80211_CMD_GET_WIPHY it contains
>> + * NLA_FLAG type according &enum nl80211_bss_select_attr to indicate what
>> + * BSS selection behaviours are supported. When used with %NL80211_CMD_CONNECT
>> + * it contains the behaviour and parameters for BSS selection to be done by
>> + * driver and/or firmware.
>
> Maybe we should be less specific here regarding the NLA_FLAG and say
> that it will contain attributes for each, indicating which behaviours
> are supported, and say there's behaviour-dependent data inside each of
> those attributes?
For the GET_WIPHY case there will be not behaviour dependent data.
> It seems entirely plausible that future behaviours would require their
> own sub-capabilities; perhaps that's even the case today for having an
> adjustment range for example, not that I think it's really necessary
> now.
At least it does not harm to take that into account.
>> @@ -3617,6 +3624,7 @@ enum nl80211_band {
>> NL80211_BAND_2GHZ,
>> NL80211_BAND_5GHZ,
>> NL80211_BAND_60GHZ,
>> + NUM_NL80211_BAND,
>
> You should use IEEE80211_NUM_BANDS and remove this.
Ok. missed that one.
>> +static bool is_band_valid(struct wiphy *wiphy, enum nl80211_band b)
>> +{
>> + return b < NUM_NL80211_BAND && wiphy->bands[b];
>> +}
>
> Here.
will change it.
>> +static int parse_bss_select(struct nlattr *nla, struct wiphy *wiphy,
>> + struct cfg80211_bss_selection
>> *bss_select)
>> +{
>> + struct nlattr *attr[NL80211_BSS_SELECT_ATTR_MAX + 1];
>> + struct nlattr *nest;
>> + int err;
>> + bool found = false;
>> + int i;
>> +
>> + /* only process one nested attribute */
>> + nest = nla_data(nla);
>> + if (!nla_ok(nest, nla_len(nest)))
>> + return -EBADF;
>
> This isn't quite clear to me. Shouldn't you do this by declaring it
> NLA_TESTED in the nl80211_policy?
Guess you mean NLA_NESTED.
> Actually, not really? you use nla_ok() on the inner (without having
> checked at all that it's even long enough btw, since you didn't do the
> policy change), but that would already be done by nla_parse() below?
Because ATTR_BSS_SELECT is used in CMD_GET_WIPHY and CMD_CONNECT the
length of the attribute is not a constant. Or do I only need to put
length in nl80211_policy for userspace-2-kernel direction, ie. for
ATTR_BSS_SELECT in CMD_CONNECT.
> What are you trying to do here?
ATTR_BSS_SELECT will have only a single nested attribute. So I looked at
nla_for_each_nested():
1233 /**
1234 * nla_for_each_attr - iterate over a stream of attributes
1235 * @pos: loop counter, set to current attribute
1236 * @head: head of attribute stream
1237 * @len: length of attribute stream
1238 * @rem: initialized to len, holds bytes currently remaining in stream
1239 */
1240 #define nla_for_each_attr(pos, head, len, rem) \
1241 for (pos = head, rem = len; \
1242 nla_ok(pos, rem); \
1243 pos = nla_next(pos, &(rem)))
1244
1245 /**
1246 * nla_for_each_nested - iterate over nested attributes
1247 * @pos: loop counter, set to current attribute
1248 * @nla: attribute containing the nested attributes
1249 * @rem: initialized to len, holds bytes currently remaining in stream
1250 */
1251 #define nla_for_each_nested(pos, nla, rem) \
1252 nla_for_each_attr(pos, nla_data(nla), nla_len(nla), rem)
So:
nla_for_each_nested(nest, nla, rem)
becomes:
for(nest = nla_data(nla), rem = nla_len(nla); nla_ok(nest, rem); ...)
As there is no need to iterate I just do the for loop initializer and
the loop condition using the if(nla_ok()).
> Regardless of that, EBADF seems like a bad error number.
Ok. Just use EINVAL here as well?
>> + err = nla_parse(attr, NL80211_BSS_SELECT_ATTR_MAX,
>> nla_data(nest),
>> + nla_len(nest), nl80211_bss_select_policy);
>> + if (err)
>> + return err;
>> +
>> + /* only one attribute may be given */
>> + for (i = 0; i <= NL80211_BSS_SELECT_ATTR_MAX; i++)
>> + if (attr[i]) {
>> + if (found)
>> + return -EINVAL;
>> + found = true;
>> + }
>
> I'd kinda prefer braces, but maybe that's just me. Surely not a
> blocking issue :)
I don't have strong opinion about it so I can change it.
Regards,
Arend
Announce support for nl80211 feature BSS_SELECT and process
BSS selection behaviour provided in .connect() callback.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
V7:
- add WLC_SET_ASSOC_PREFER to fix firmware behaviour.
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 70 +++++++++++++++++++++-
.../wireless/broadcom/brcm80211/brcmfmac/common.c | 38 +++++++-----
.../wireless/broadcom/brcm80211/brcmfmac/core.h | 1 +
.../wireless/broadcom/brcm80211/brcmfmac/fwil.h | 1 +
4 files changed, 92 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index d00c5c1..593385b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -238,6 +238,20 @@ struct parsed_vndr_ies {
struct parsed_vndr_ie_info ie_info[VNDR_IE_PARSE_LIMIT];
};
+static u8 nl80211_band_to_fwil(enum nl80211_band band)
+{
+ switch (band) {
+ case NL80211_BAND_2GHZ:
+ return WLC_BAND_2G;
+ case NL80211_BAND_5GHZ:
+ return WLC_BAND_5G;
+ default:
+ WARN_ON(1);
+ break;
+ }
+ return 0;
+}
+
static u16 chandef_to_chanspec(struct brcmu_d11inf *d11inf,
struct cfg80211_chan_def *ch)
{
@@ -1734,6 +1748,50 @@ enum nl80211_auth_type brcmf_war_auth_type(struct brcmf_if *ifp,
return type;
}
+static void brcmf_set_join_pref(struct brcmf_if *ifp,
+ struct cfg80211_bss_selection *bss_select)
+{
+ struct brcmf_join_pref_params join_pref_params[2];
+ enum nl80211_band band;
+ int err, i = 0;
+
+ join_pref_params[i].len = 2;
+ join_pref_params[i].rssi_gain = 0;
+
+ if (bss_select->behaviour != NL80211_BSS_SELECT_ATTR_BAND_PREF)
+ brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_ASSOC_PREFER, WLC_BAND_AUTO);
+
+ switch (bss_select->behaviour) {
+ case __NL80211_BSS_SELECT_ATTR_INVALID:
+ brcmf_c_set_joinpref_default(ifp);
+ return;
+ case NL80211_BSS_SELECT_ATTR_BAND_PREF:
+ join_pref_params[i].type = BRCMF_JOIN_PREF_BAND;
+ band = bss_select->param.band_pref;
+ join_pref_params[i].band = nl80211_band_to_fwil(band);
+ i++;
+ break;
+ case NL80211_BSS_SELECT_ATTR_RSSI_ADJUST:
+ join_pref_params[i].type = BRCMF_JOIN_PREF_RSSI_DELTA;
+ band = bss_select->param.adjust.band;
+ join_pref_params[i].band = nl80211_band_to_fwil(band);
+ join_pref_params[i].rssi_gain = bss_select->param.adjust.delta;
+ i++;
+ break;
+ case NL80211_BSS_SELECT_ATTR_RSSI:
+ default:
+ break;
+ }
+ join_pref_params[i].type = BRCMF_JOIN_PREF_RSSI;
+ join_pref_params[i].len = 2;
+ join_pref_params[i].rssi_gain = 0;
+ join_pref_params[i].band = 0;
+ err = brcmf_fil_iovar_data_set(ifp, "join_pref", join_pref_params,
+ sizeof(join_pref_params));
+ if (err)
+ brcmf_err("Set join_pref error (%d)\n", err);
+}
+
static s32
brcmf_cfg80211_connect(struct wiphy *wiphy, struct net_device *ndev,
struct cfg80211_connect_params *sme)
@@ -1859,11 +1917,11 @@ brcmf_cfg80211_connect(struct wiphy *wiphy, struct net_device *ndev,
ext_join_params->scan_le.scan_type = -1;
ext_join_params->scan_le.home_time = cpu_to_le32(-1);
- if (sme->bssid)
+ if (sme->bssid) {
memcpy(&ext_join_params->assoc_le.bssid, sme->bssid, ETH_ALEN);
- else
+ } else {
eth_broadcast_addr(ext_join_params->assoc_le.bssid);
-
+ }
if (cfg->channel) {
ext_join_params->assoc_le.chanspec_num = cpu_to_le32(1);
@@ -1890,6 +1948,8 @@ brcmf_cfg80211_connect(struct wiphy *wiphy, struct net_device *ndev,
ext_join_params->scan_le.nprobes = cpu_to_le32(-1);
}
+ brcmf_set_join_pref(ifp, &sme->bss_select);
+
err = brcmf_fil_bsscfg_data_set(ifp, "join", ext_join_params,
join_params_size);
kfree(ext_join_params);
@@ -6176,6 +6236,10 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
wiphy->signal_type = CFG80211_SIGNAL_TYPE_MBM;
wiphy->cipher_suites = __wl_cipher_suites;
wiphy->n_cipher_suites = ARRAY_SIZE(__wl_cipher_suites);
+ wiphy->bss_select_support = BIT(NL80211_BSS_SELECT_ATTR_RSSI) |
+ BIT(NL80211_BSS_SELECT_ATTR_BAND_PREF) |
+ BIT(NL80211_BSS_SELECT_ATTR_RSSI_ADJUST);
+
wiphy->flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT |
WIPHY_FLAG_OFFCHAN_TX |
WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index cfee477..f48e8d4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -33,7 +33,7 @@ const u8 ALLFFMAC[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
#define BRCMF_DEFAULT_SCAN_CHANNEL_TIME 40
#define BRCMF_DEFAULT_SCAN_UNASSOC_TIME 40
-/* boost value for RSSI_DELTA in preferred join selection */
+/* default boost value for RSSI_DELTA in preferred join selection */
#define BRCMF_JOIN_PREF_RSSI_BOOST 8
#define BRCMF_DEFAULT_TXGLOM_SIZE 32 /* max tx frames in glom chain */
@@ -77,11 +77,31 @@ MODULE_PARM_DESC(ignore_probe_fail, "always succeed probe for debugging");
struct brcmf_mp_global_t brcmf_mp_global;
+void brcmf_c_set_joinpref_default(struct brcmf_if *ifp)
+{
+ struct brcmf_join_pref_params join_pref_params[2];
+ int err;
+
+ /* Setup join_pref to select target by RSSI (boost on 5GHz) */
+ join_pref_params[0].type = BRCMF_JOIN_PREF_RSSI_DELTA;
+ join_pref_params[0].len = 2;
+ join_pref_params[0].rssi_gain = BRCMF_JOIN_PREF_RSSI_BOOST;
+ join_pref_params[0].band = WLC_BAND_5G;
+
+ join_pref_params[1].type = BRCMF_JOIN_PREF_RSSI;
+ join_pref_params[1].len = 2;
+ join_pref_params[1].rssi_gain = 0;
+ join_pref_params[1].band = 0;
+ err = brcmf_fil_iovar_data_set(ifp, "join_pref", join_pref_params,
+ sizeof(join_pref_params));
+ if (err)
+ brcmf_err("Set join_pref error (%d)\n", err);
+}
+
int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
{
s8 eventmask[BRCMF_EVENTING_MASK_LEN];
u8 buf[BRCMF_DCMD_SMLEN];
- struct brcmf_join_pref_params join_pref_params[2];
struct brcmf_rev_info_le revinfo;
struct brcmf_rev_info *ri;
char *ptr;
@@ -148,19 +168,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
goto done;
}
- /* Setup join_pref to select target by RSSI(with boost on 5GHz) */
- join_pref_params[0].type = BRCMF_JOIN_PREF_RSSI_DELTA;
- join_pref_params[0].len = 2;
- join_pref_params[0].rssi_gain = BRCMF_JOIN_PREF_RSSI_BOOST;
- join_pref_params[0].band = WLC_BAND_5G;
- join_pref_params[1].type = BRCMF_JOIN_PREF_RSSI;
- join_pref_params[1].len = 2;
- join_pref_params[1].rssi_gain = 0;
- join_pref_params[1].band = 0;
- err = brcmf_fil_iovar_data_set(ifp, "join_pref", join_pref_params,
- sizeof(join_pref_params));
- if (err)
- brcmf_err("Set join_pref error (%d)\n", err);
+ brcmf_c_set_joinpref_default(ifp);
/* Setup event_msgs, enable E_IF */
err = brcmf_fil_iovar_data_get(ifp, "event_msgs", eventmask,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index 8f39435..c9a1a13 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -220,5 +220,6 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb);
void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on);
+void brcmf_c_set_joinpref_default(struct brcmf_if *ifp);
#endif /* BRCMFMAC_CORE_H */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
index 6b72df1..3a9a76d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
@@ -78,6 +78,7 @@
#define BRCMF_C_SET_SCAN_CHANNEL_TIME 185
#define BRCMF_C_SET_SCAN_UNASSOC_TIME 187
#define BRCMF_C_SCB_DEAUTHENTICATE_FOR_REASON 201
+#define BRCMF_C_SET_ASSOC_PREFER 205
#define BRCMF_C_GET_VALID_CHANNELS 217
#define BRCMF_C_GET_KEY_PRIMARY 235
#define BRCMF_C_SET_KEY_PRIMARY 236
--
1.9.1
On Wed, 2016-02-24 at 11:04 +0100, Arend Van Spriel wrote:
> I actually do not see any NLA_NESTED attributes with an explicit
> length. As you mentioned the nla_parse() of the nested attribute will
> validate the length of the stream so no need to put that in the
> policy.
Right, I think what you want can be achieved by just using NLA_NESTED
to make sure it's long enough for nesting anything, and then nla_parse.
I'm pretty sure we do the same in other places, no?
johannes
On 23-2-2016 21:52, Johannes Berg wrote:
> On Tue, 2016-02-23 at 21:46 +0100, Arend Van Spriel wrote:
>>
>>> Maybe we should be less specific here regarding the NLA_FLAG and
>>> say
>>> that it will contain attributes for each, indicating which
>>> behaviours
>>> are supported, and say there's behaviour-dependent data inside each
>>> of
>>> those attributes?
>>
>> For the GET_WIPHY case there will be not behaviour dependent data.
>>
>>> It seems entirely plausible that future behaviours would require
>>> their
>>> own sub-capabilities; perhaps that's even the case today for having
>>> an
>>> adjustment range for example, not that I think it's really
>>> necessary
>>> now.
>>
>> At least it does not harm to take that into account.
>
> Right; there isn't today, but there might be later, so for the
> GET_WIPHY case it might be worth just specifying that there will be an
> attribute, not that it's NLA_FLAG?
Will try and be vague :-)
> Not that it'll matter much - only when we actually add such could
> applications possibly want to parse them :)
That goes without saying :-p
>>>> + /* only process one nested attribute */
>>>> + nest = nla_data(nla);
>>>> + if (!nla_ok(nest, nla_len(nest)))
>>>> + return -EBADF;
>>>
>>> This isn't quite clear to me. Shouldn't you do this by declaring it
>>> NLA_TESTED in the nl80211_policy?
>>
>> Guess you mean NLA_NESTED.
>
> Heh, yeah.
>
>>> Actually, not really? you use nla_ok() on the inner (without having
>>> checked at all that it's even long enough btw, since you didn't do
>>> the
>>> policy change), but that would already be done by nla_parse()
>>> below?
>>
>> Because ATTR_BSS_SELECT is used in CMD_GET_WIPHY and CMD_CONNECT the
>> length of the attribute is not a constant. Or do I only need to put
>> length in nl80211_policy for userspace-2-kernel direction, ie. for
>> ATTR_BSS_SELECT in CMD_CONNECT.
>
> Ah. kernel->userspace messages don't use the policy in any way, it's
> just for the userspace->kernel message validation, so you can put there
> what's needed for CMD_CONNECT and ignore GET_WIPHY.
I actually do not see any NLA_NESTED attributes with an explicit length.
As you mentioned the nla_parse() of the nested attribute will validate
the length of the stream so no need to put that in the policy.
Regards,
Arend
On Tue, 2016-02-23 at 21:46 +0100, Arend Van Spriel wrote:
>
> > Maybe we should be less specific here regarding the NLA_FLAG and
> > say
> > that it will contain attributes for each, indicating which
> > behaviours
> > are supported, and say there's behaviour-dependent data inside each
> > of
> > those attributes?
>
> For the GET_WIPHY case there will be not behaviour dependent data.
>
> > It seems entirely plausible that future behaviours would require
> > their
> > own sub-capabilities; perhaps that's even the case today for having
> > an
> > adjustment range for example, not that I think it's really
> > necessary
> > now.
>
> At least it does not harm to take that into account.
Right; there isn't today, but there might be later, so for the
GET_WIPHY case it might be worth just specifying that there will be an
attribute, not that it's NLA_FLAG?
Not that it'll matter much - only when we actually add such could
applications possibly want to parse them :)
> > > + /* only process one nested attribute */
> > > + nest = nla_data(nla);
> > > + if (!nla_ok(nest, nla_len(nest)))
> > > + return -EBADF;
> >
> > This isn't quite clear to me. Shouldn't you do this by declaring it
> > NLA_TESTED in the nl80211_policy?
>
> Guess you mean NLA_NESTED.
Heh, yeah.
> > Actually, not really? you use nla_ok() on the inner (without having
> > checked at all that it's even long enough btw, since you didn't do
> > the
> > policy change), but that would already be done by nla_parse()
> > below?
>
> Because ATTR_BSS_SELECT is used in CMD_GET_WIPHY and CMD_CONNECT the
> length of the attribute is not a constant. Or do I only need to put
> length in nl80211_policy for userspace-2-kernel direction, ie. for
> ATTR_BSS_SELECT in CMD_CONNECT.
Ah. kernel->userspace messages don't use the policy in any way, it's
just for the userspace->kernel message validation, so you can put there
what's needed for CMD_CONNECT and ignore GET_WIPHY.
johannes
On Wed, 2016-02-17 at 11:47 +0100, Arend van Spriel wrote:
>
> + * @NL80211_ATTR_BSS_SELECT: nested attribute for driver supporting the
> + * BSS selection feature. When used with %NL80211_CMD_GET_WIPHY it contains
> + * NLA_FLAG type according &enum nl80211_bss_select_attr to indicate what
> + * BSS selection behaviours are supported. When used with %NL80211_CMD_CONNECT
> + * it contains the behaviour and parameters for BSS selection to be done by
> + * driver and/or firmware.
Maybe we should be less specific here regarding the NLA_FLAG and say
that it will contain attributes for each, indicating which behaviours
are supported, and say there's behaviour-dependent data inside each of
those attributes?
It seems entirely plausible that future behaviours would require their
own sub-capabilities; perhaps that's even the case today for having an
adjustment range for example, not that I think it's really necessary
now.
> @@ -3617,6 +3624,7 @@ enum nl80211_band {
> NL80211_BAND_2GHZ,
> NL80211_BAND_5GHZ,
> NL80211_BAND_60GHZ,
> + NUM_NL80211_BAND,
You should use IEEE80211_NUM_BANDS and remove this.
> +static bool is_band_valid(struct wiphy *wiphy, enum nl80211_band b)
> +{
> + return b < NUM_NL80211_BAND && wiphy->bands[b];
> +}
Here.
> +static int parse_bss_select(struct nlattr *nla, struct wiphy *wiphy,
> + struct cfg80211_bss_selection
> *bss_select)
> +{
> + struct nlattr *attr[NL80211_BSS_SELECT_ATTR_MAX + 1];
> + struct nlattr *nest;
> + int err;
> + bool found = false;
> + int i;
> +
> + /* only process one nested attribute */
> + nest = nla_data(nla);
> + if (!nla_ok(nest, nla_len(nest)))
> + return -EBADF;
This isn't quite clear to me. Shouldn't you do this by declaring it
NLA_TESTED in the nl80211_policy?
Actually, not really? you use nla_ok() on the inner (without having
checked at all that it's even long enough btw, since you didn't do the
policy change), but that would already be done by nla_parse() below?
What are you trying to do here?
Regardless of that, EBADF seems like a bad error number.
> + err = nla_parse(attr, NL80211_BSS_SELECT_ATTR_MAX,
> nla_data(nest),
> + nla_len(nest), nl80211_bss_select_policy);
> + if (err)
> + return err;
> +
> + /* only one attribute may be given */
> + for (i = 0; i <= NL80211_BSS_SELECT_ATTR_MAX; i++)
> + if (attr[i]) {
> + if (found)
> + return -EINVAL;
> + found = true;
> + }
I'd kinda prefer braces, but maybe that's just me. Surely not a
blocking issue :)
johannes