All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V7 1/2] nl80211: add feature for BSS selection support
@ 2016-02-17 10:47 Arend van Spriel
  2016-02-17 10:47 ` [PATCH V7 2/2] brcmfmac: add support for nl80211 BSS_SELECT feature Arend van Spriel
  2016-02-23 14:20 ` [PATCH V7 1/2] nl80211: add feature for BSS selection support Johannes Berg
  0 siblings, 2 replies; 7+ messages in thread
From: Arend van Spriel @ 2016-02-17 10:47 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Arend van Spriel

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 <meuleman@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Lei Zhang <leizh@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH V7 2/2] brcmfmac: add support for nl80211 BSS_SELECT feature
  2016-02-17 10:47 [PATCH V7 1/2] nl80211: add feature for BSS selection support Arend van Spriel
@ 2016-02-17 10:47 ` Arend van Spriel
  2016-02-23 14:20 ` [PATCH V7 1/2] nl80211: add feature for BSS selection support Johannes Berg
  1 sibling, 0 replies; 7+ messages in thread
From: Arend van Spriel @ 2016-02-17 10:47 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Arend van Spriel

Announce support for nl80211 feature BSS_SELECT and process
BSS selection behaviour provided in .connect() callback.

Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Lei Zhang <leizh@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V7 1/2] nl80211: add feature for BSS selection support
  2016-02-17 10:47 [PATCH V7 1/2] nl80211: add feature for BSS selection support Arend van Spriel
  2016-02-17 10:47 ` [PATCH V7 2/2] brcmfmac: add support for nl80211 BSS_SELECT feature Arend van Spriel
@ 2016-02-23 14:20 ` Johannes Berg
  2016-02-23 20:46   ` Arend Van Spriel
  1 sibling, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2016-02-23 14:20 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: linux-wireless

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V7 1/2] nl80211: add feature for BSS selection support
  2016-02-23 14:20 ` [PATCH V7 1/2] nl80211: add feature for BSS selection support Johannes Berg
@ 2016-02-23 20:46   ` Arend Van Spriel
  2016-02-23 20:52     ` Johannes Berg
  0 siblings, 1 reply; 7+ messages in thread
From: Arend Van Spriel @ 2016-02-23 20:46 UTC (permalink / raw)
  To: Johannes Berg, Arend van Spriel; +Cc: linux-wireless



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V7 1/2] nl80211: add feature for BSS selection support
  2016-02-23 20:46   ` Arend Van Spriel
@ 2016-02-23 20:52     ` Johannes Berg
  2016-02-24 10:04       ` Arend Van Spriel
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2016-02-23 20:52 UTC (permalink / raw)
  To: arend; +Cc: linux-wireless

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V7 1/2] nl80211: add feature for BSS selection support
  2016-02-23 20:52     ` Johannes Berg
@ 2016-02-24 10:04       ` Arend Van Spriel
  2016-02-24 10:37         ` Johannes Berg
  0 siblings, 1 reply; 7+ messages in thread
From: Arend Van Spriel @ 2016-02-24 10:04 UTC (permalink / raw)
  To: Johannes Berg, arend; +Cc: linux-wireless



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V7 1/2] nl80211: add feature for BSS selection support
  2016-02-24 10:04       ` Arend Van Spriel
@ 2016-02-24 10:37         ` Johannes Berg
  0 siblings, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2016-02-24 10:37 UTC (permalink / raw)
  To: arend; +Cc: linux-wireless

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-02-24 10:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-17 10:47 [PATCH V7 1/2] nl80211: add feature for BSS selection support Arend van Spriel
2016-02-17 10:47 ` [PATCH V7 2/2] brcmfmac: add support for nl80211 BSS_SELECT feature Arend van Spriel
2016-02-23 14:20 ` [PATCH V7 1/2] nl80211: add feature for BSS selection support Johannes Berg
2016-02-23 20:46   ` Arend Van Spriel
2016-02-23 20:52     ` Johannes Berg
2016-02-24 10:04       ` Arend Van Spriel
2016-02-24 10:37         ` Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.