[PATCH v3 1/3] cfg80211: Add support to sched scan to report better BSSs

[PATCH v3 1/3] cfg80211: Add support to sched scan to report better BSSs

[Jouni Malinen]

From: vamsi krishna <vamsin@qti.qualcomm.com>

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 better RSSI than the current connected BSS. New
attributes to specify the relative RSSI (compared to the current BSS)
are added to the sched scan to implement this.

Signed-off-by: vamsi krishna <vamsin@qti.qualcomm.com>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 include/net/cfg80211.h       | 36 ++++++++++++++++++++++----------
 include/uapi/linux/nl80211.h | 29 ++++++++++++++++++++++++++
 net/wireless/nl80211.c       | 49 ++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 101 insertions(+), 13 deletions(-)

v3:
- use struct cfg80211_bss_select_adjust as the data structure for
  specifying band preference (instead of attr hardcoded for 5 GHz)
- add relative_rssi_set boolean to have a robust mechanism for
  determining whether the NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI
  attribute was included

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index cb13789..9dc11d3 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1620,6 +1620,17 @@ struct cfg80211_sched_scan_plan {
 };
 
 /**
+ * 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_sched_scan_request - scheduled scan request description
  *
  * @ssids: SSIDs to scan for (passed in the probe_reqs in active scans)
@@ -1654,6 +1665,16 @@ 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_set: Indicates whether @relative_rssi is set or not.
+ * @relative_rssi: Relative RSSI threshold in dB to restrict scan result
+ *	reporting in connected state to cases where a matching BSS is determined
+ *	to have better RSSI than the current connected BSS. The relative RSSI
+ *	threshold values are ignored in disconnected state.
+ * @rssi_adjust: delta dB of RSSI preference to be given to the BSSs that belong
+ *	to the specified band while deciding whether a better BSS is reported
+ *	using @relative_rssi. If delta is a negative number, the BSSs that
+ *	belong to the specified band will be penalized by delta dB in relative
+ *	comparisions.
  */
 struct cfg80211_sched_scan_request {
 	struct cfg80211_ssid *ssids;
@@ -1673,6 +1694,10 @@ struct cfg80211_sched_scan_request {
 	u8 mac_addr[ETH_ALEN] __aligned(2);
 	u8 mac_addr_mask[ETH_ALEN] __aligned(2);
 
+	bool relative_rssi_set;
+	s8 relative_rssi;
+	struct cfg80211_bss_select_adjust rssi_adjust;
+
 	/* internal */
 	struct wiphy *wiphy;
 	struct net_device *dev;
@@ -1981,17 +2006,6 @@ 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.
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 174f4b3..4e8bf28 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1982,6 +1982,19 @@ enum nl80211_commands {
  * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
  *	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 by using an offloaded operation to avoid
+ *	unnecessary wakeups.
+ *
+ * @NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST: When present the RSSI level for BSSs in
+ *	the specified band is to be adjusted before doing
+ *	%NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI based comparision to figure out
+ *	better BSSs. The attribute value is a packed structure
+ *	value as specified by &struct nl80211_bss_select_rssi_adjust.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2388,6 +2401,9 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_BSSID,
 
+	NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
+	NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -3080,6 +3096,13 @@ enum nl80211_reg_rule_attr {
  *	how this API was implemented in the past. Also, due to the same problem,
  *	the only way to create a matchset with only an RSSI filter (with this
  *	attribute) is if there's only a single matchset with the RSSI attribute.
+ * @NL80211_SCHED_SCAN_MATCH_ATTR_RELATIVE_RSSI: Flag indicating whether
+ *	%NL80211_SCHED_SCAN_MATCH_ATTR_RSSI to be used as absolute RSSI or
+ *	relative to current bss's RSSI.
+ * @NL80211_SCHED_SCAN_MATCH_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_SCHED_SCAN_MATCH_ATTR_MAX: highest scheduled scan filter
  *	attribute number currently defined
  * @__NL80211_SCHED_SCAN_MATCH_ATTR_AFTER_LAST: internal use
@@ -3089,6 +3112,8 @@ enum nl80211_sched_scan_match_attr {
 
 	NL80211_SCHED_SCAN_MATCH_ATTR_SSID,
 	NL80211_SCHED_SCAN_MATCH_ATTR_RSSI,
+	NL80211_SCHED_SCAN_MATCH_ATTR_RELATIVE_RSSI,
+	NL80211_SCHED_SCAN_MATCH_ATTR_RSSI_ADJUST,
 
 	/* keep last */
 	__NL80211_SCHED_SCAN_MATCH_ATTR_AFTER_LAST,
@@ -4699,6 +4724,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_RELATIVE_RSSI: 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.
@@ -4714,6 +4742,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_RELATIVE_RSSI,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b378d0a..bb3f04a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -405,6 +405,10 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[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_S8 },
+	[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST] = {
+		.len = sizeof(struct nl80211_bss_select_rssi_adjust)
+	},
 };
 
 /* policy for the key attributes */
@@ -6950,6 +6954,12 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
 	if (!n_plans || n_plans > wiphy->max_sched_scan_plans)
 		return ERR_PTR(-EINVAL);
 
+	if (!wiphy_ext_feature_isset(
+		    wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI) &&
+	    (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] ||
+	     attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]))
+		return ERR_PTR(-EINVAL);
+
 	request = kzalloc(sizeof(*request)
 			+ sizeof(*request->ssids) * n_ssids
 			+ sizeof(*request->match_sets) * n_match_sets
@@ -7156,6 +7166,25 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
 		request->delay =
 			nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_DELAY]);
 
+	if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]) {
+		request->relative_rssi = nla_get_s8(
+			attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
+		request->relative_rssi_set = true;
+	}
+
+	if (attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]) {
+		struct nl80211_bss_select_rssi_adjust *rssi_adjust;
+
+		rssi_adjust = nla_data(
+			attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]);
+		request->rssi_adjust.band = rssi_adjust->band;
+		request->rssi_adjust.delta = rssi_adjust->delta;
+		if (!is_band_valid(wiphy, request->rssi_adjust.band)) {
+			err = -EINVAL;
+			goto out_free;
+		}
+	}
+
 	err = nl80211_parse_sched_scan_plans(wiphy, n_plans, request, attrs);
 	if (err)
 		goto out_free;
@@ -9671,7 +9700,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;
@@ -9692,6 +9722,21 @@ 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 (req->relative_rssi_set &&
+	    nla_put_s8(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
+		       req->relative_rssi))
+		return -ENOBUFS;
+
+	if (req->rssi_adjust.band != __NL80211_BAND_ATTR_INVALID) {
+		struct nl80211_bss_select_rssi_adjust rssi_adjust;
+
+		rssi_adjust.band = req->rssi_adjust.band;
+		rssi_adjust.delta = req->rssi_adjust.delta;
+		if (nla_put(msg, NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST,
+			    sizeof(rssi_adjust), &rssi_adjust))
+			return -ENOBUFS;
+	}
+
 	freqs = nla_nest_start(msg, NL80211_ATTR_SCAN_FREQUENCIES);
 	if (!freqs)
 		return -ENOBUFS;
@@ -9805,7 +9850,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;
 
-- 
2.7.4

[PATCH v2 2/3] cfg80211: Add support to randomize TA of Public Action frames

[Jouni Malinen]

From: vamsi krishna <vamsin@qti.qualcomm.com>

Add support to use a random local address (Address 2 = TA in transmit
and the same address in receive functionality) for Public Action frames
in order to improve privacy of WLAN clients. Applications fill the
random transmit address in the frame buffer in the NL80211_CMD_FRAME
command. This can be used only with the drivers that indicate support
for random local address by setting the new
NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA and/or
NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA_CONNECTED in ext_features.

The driver needs to configure receive behavior to accept frames to the
specified random address during the time the frame exchange is pending
and such frames need to be acknowledged similarly to frames sent to the
local permanent address when this random address functionality is not
used.

Signed-off-by: vamsi krishna <vamsin@qti.qualcomm.com>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 include/uapi/linux/nl80211.h |  6 ++++++
 net/wireless/mlme.c          | 21 +++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

v2:
- remove unnecessary NL80211_ATTR_MGMT_TX_RANDOM_SA and allow address
  change based on the driver capability flag without requiring
  explicit per-frame indication from user space
- rename "SA" to "TA" to be more accurate for the RX/ACK purposes

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 4e8bf28..ebed28e 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4727,6 +4727,10 @@ enum nl80211_feature_flags {
  * @NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI: The driver supports sched_scan
  *	for reporting BSSs with better RSSI than the current connected BSS
  *	(%NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI).
+ * @NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA: This driver supports randomized TA
+ *	in @NL80211_CMD_FRAME while not associated.
+ * @NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA_CONNECTED: This driver supports
+ *	randomized TA in @NL80211_CMD_FRAME while associated.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4743,6 +4747,8 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_BEACON_RATE_VHT,
 	NL80211_EXT_FEATURE_FILS_STA,
 	NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI,
+	NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA,
+	NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA_CONNECTED,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 1c63a77..b876f40 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -662,8 +662,25 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
 			return err;
 	}
 
-	if (!ether_addr_equal(mgmt->sa, wdev_address(wdev)))
-		return -EINVAL;
+	if (!ether_addr_equal(mgmt->sa, wdev_address(wdev))) {
+		/* Allow random TA to be used with Public Action frames if the
+		 * driver has indicated support for this. Otherwise, only allow
+		 * the local address to be used.
+		 */
+		if (!ieee80211_is_action(mgmt->frame_control) ||
+		    mgmt->u.action.category != WLAN_CATEGORY_PUBLIC)
+			return -EINVAL;
+		if (!wdev->current_bss &&
+		    !wiphy_ext_feature_isset(
+			    &rdev->wiphy,
+			    NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA))
+			return -EINVAL;
+		if (wdev->current_bss &&
+		    !wiphy_ext_feature_isset(
+			    &rdev->wiphy,
+			    NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA_CONNECTED))
+			return -EINVAL;
+	}
 
 	/* Transmit the Action frame as requested by user space */
 	return rdev_mgmt_tx(rdev, wdev, params, cookie);
-- 
2.7.4

Re: [PATCH v2 2/3] cfg80211: Add support to randomize TA of Public Action frames

[Johannes Berg]

On Mon, 2017-01-09 at 19:53 +0200, Jouni Malinen wrote:
> 
> +		if (!wdev->current_bss &&
> +		    !wiphy_ext_feature_isset(
> +			    &rdev->wiphy,
> +			    NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA))
> +			return -EINVAL;
> +		if (wdev->current_bss &&
> +		    !wiphy_ext_feature_isset(
> +			    &rdev->wiphy,
> +			    NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA_CO
> NNECTED))
> +			return -EINVAL;
> +	}

This current_bss stuff is going to be somewhat racy, but I guess we can
live with that.

Looks good, but doesn't apply without the first patch.

johannes

[PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Jouni Malinen]

From: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>

This enhances the connect timeout API to also carry the reason for the
timeout. These reason codes for the connect time out are represented by
enum nl80211_timeout_reason and are passed to user space through a new
attribute NL80211_ATTR_TIMEOUT_REASON (u32).

Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 include/net/cfg80211.h       | 15 +++++++++++----
 include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++
 net/wireless/core.h          |  4 +++-
 net/wireless/mlme.c          |  3 ++-
 net/wireless/nl80211.c       |  8 ++++++--
 net/wireless/nl80211.h       |  3 ++-
 net/wireless/sme.c           | 33 ++++++++++++++++++++++++---------
 net/wireless/util.c          |  2 +-
 8 files changed, 70 insertions(+), 19 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9dc11d3..44277fe 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5090,6 +5090,9 @@ static inline void cfg80211_testmode_event(struct sk_buff *skb, gfp_t gfp)
  *      %WLAN_STATUS_UNSPECIFIED_FAILURE if your device cannot give you
  *      the real status code for failures.
  * @gfp: allocation flags
+ * @timeout_reason: reason for connection timeout. This is used when the
+ *	connection fails due to a timeout instead of an explicit rejection from
+ *	the AP. 0 (NL80211_CONNECT_TIMEOUT_UNSPECIFIED) is used for other cases.
  *
  * It should be called by the underlying driver whenever connect() has
  * succeeded. This is similar to cfg80211_connect_result(), but with the
@@ -5099,7 +5102,8 @@ static inline void cfg80211_testmode_event(struct sk_buff *skb, gfp_t gfp)
 void cfg80211_connect_bss(struct net_device *dev, const u8 *bssid,
 			  struct cfg80211_bss *bss, const u8 *req_ie,
 			  size_t req_ie_len, const u8 *resp_ie,
-			  size_t resp_ie_len, int status, gfp_t gfp);
+			  size_t resp_ie_len, int status, gfp_t gfp,
+			  enum nl80211_timeout_reason timeout_reason);
 
 /**
  * cfg80211_connect_result - notify cfg80211 of connection result
@@ -5125,7 +5129,8 @@ cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 			u16 status, gfp_t gfp)
 {
 	cfg80211_connect_bss(dev, bssid, NULL, req_ie, req_ie_len, resp_ie,
-			     resp_ie_len, status, gfp);
+			     resp_ie_len, status, gfp,
+			     NL80211_TIMEOUT_UNSPECIFIED);
 }
 
 /**
@@ -5136,6 +5141,7 @@ cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
  * @req_ie: association request IEs (maybe be %NULL)
  * @req_ie_len: association request IEs length
  * @gfp: allocation flags
+ * @timeout_reason: reason for connection timeout.
  *
  * It should be called by the underlying driver whenever connect() has failed
  * in a sequence where no explicit authentication/association rejection was
@@ -5145,10 +5151,11 @@ cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
  */
 static inline void
 cfg80211_connect_timeout(struct net_device *dev, const u8 *bssid,
-			 const u8 *req_ie, size_t req_ie_len, gfp_t gfp)
+			 const u8 *req_ie, size_t req_ie_len, gfp_t gfp,
+			 enum nl80211_timeout_reason timeout_reason)
 {
 	cfg80211_connect_bss(dev, bssid, NULL, req_ie, req_ie_len, NULL, 0, -1,
-			     gfp);
+			     gfp, timeout_reason);
 }
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index ebed28e..aa008f0 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1995,6 +1995,10 @@ enum nl80211_commands {
  *	better BSSs. The attribute value is a packed structure
  *	value as specified by &struct nl80211_bss_select_rssi_adjust.
  *
+ * @NL80211_ATTR_TIMEOUT_REASON: The reason for which an operation timed out.
+ *	u32 attribute with an &enum nl80211_timeout_reason value. This is used,
+ *	e.g., with %NL80211_CMD_CONNECT event.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2404,6 +2408,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
 	NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST,
 
+	NL80211_ATTR_TIMEOUT_REASON,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -4788,6 +4794,21 @@ enum nl80211_connect_failed_reason {
 };
 
 /**
+ * enum nl80211_timeout_reason - timeout reasons
+ *
+ * @NL80211_TIMEOUT_UNSPECIFIED: Timeout reason unspecified.
+ * @NL80211_TIMEOUT_SCAN: Scan (AP discovery) timed out.
+ * @NL80211_TIMEOUT_AUTH: Authentication timed out.
+ * @NL80211_TIMEOUT_ASSOC: Association timed out.
+ */
+enum nl80211_timeout_reason {
+	NL80211_TIMEOUT_UNSPECIFIED,
+	NL80211_TIMEOUT_SCAN,
+	NL80211_TIMEOUT_AUTH,
+	NL80211_TIMEOUT_ASSOC,
+};
+
+/**
  * enum nl80211_scan_flags -  scan request control flags
  *
  * Scan request control flags are used to control the handling
diff --git a/net/wireless/core.h b/net/wireless/core.h
index ba42055..58ca206 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -228,6 +228,7 @@ struct cfg80211_event {
 			size_t resp_ie_len;
 			struct cfg80211_bss *bss;
 			int status; /* -1 = failed; 0..65535 = status code */
+			enum nl80211_timeout_reason timeout_reason;
 		} cr;
 		struct {
 			const u8 *req_ie;
@@ -388,7 +389,8 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 			       const u8 *req_ie, size_t req_ie_len,
 			       const u8 *resp_ie, size_t resp_ie_len,
 			       int status, bool wextev,
-			       struct cfg80211_bss *bss);
+			       struct cfg80211_bss *bss,
+			       enum nl80211_timeout_reason timeout_reason);
 void __cfg80211_disconnected(struct net_device *dev, const u8 *ie,
 			     size_t ie_len, u16 reason, bool from_ap);
 int cfg80211_disconnect(struct cfg80211_registered_device *rdev,
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index b876f40..22b3d99 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -48,7 +48,8 @@ void cfg80211_rx_assoc_resp(struct net_device *dev, struct cfg80211_bss *bss,
 	/* update current_bss etc., consumes the bss reference */
 	__cfg80211_connect_result(dev, mgmt->bssid, NULL, 0, ie, len - ieoffs,
 				  status_code,
-				  status_code == WLAN_STATUS_SUCCESS, bss);
+				  status_code == WLAN_STATUS_SUCCESS, bss,
+				  NL80211_TIMEOUT_UNSPECIFIED);
 }
 EXPORT_SYMBOL(cfg80211_rx_assoc_resp);
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bb3f04a..64d77e3 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -409,6 +409,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST] = {
 		.len = sizeof(struct nl80211_bss_select_rssi_adjust)
 	},
+	[NL80211_ATTR_TIMEOUT_REASON] = { .type = NLA_U32 },
 };
 
 /* policy for the key attributes */
@@ -13232,7 +13233,8 @@ void nl80211_send_connect_result(struct cfg80211_registered_device *rdev,
 				 struct net_device *netdev, const u8 *bssid,
 				 const u8 *req_ie, size_t req_ie_len,
 				 const u8 *resp_ie, size_t resp_ie_len,
-				 int status, gfp_t gfp)
+				 int status, gfp_t gfp,
+				 enum nl80211_timeout_reason timeout_reason)
 {
 	struct sk_buff *msg;
 	void *hdr;
@@ -13253,7 +13255,9 @@ void nl80211_send_connect_result(struct cfg80211_registered_device *rdev,
 	    nla_put_u16(msg, NL80211_ATTR_STATUS_CODE,
 			status < 0 ? WLAN_STATUS_UNSPECIFIED_FAILURE :
 			status) ||
-	    (status < 0 && nla_put_flag(msg, NL80211_ATTR_TIMED_OUT)) ||
+	    (status < 0 &&
+	     (nla_put_flag(msg, NL80211_ATTR_TIMED_OUT) ||
+	      nla_put_u32(msg, NL80211_ATTR_TIMEOUT_REASON, timeout_reason))) ||
 	    (req_ie &&
 	     nla_put(msg, NL80211_ATTR_REQ_IE, req_ie_len, req_ie)) ||
 	    (resp_ie &&
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 75f8252..633b3eb 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -56,7 +56,8 @@ void nl80211_send_connect_result(struct cfg80211_registered_device *rdev,
 				 struct net_device *netdev, const u8 *bssid,
 				 const u8 *req_ie, size_t req_ie_len,
 				 const u8 *resp_ie, size_t resp_ie_len,
-				 int status, gfp_t gfp);
+				 int status, gfp_t gfp,
+				 enum nl80211_timeout_reason timeout_reason);
 void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
 			 struct net_device *netdev, const u8 *bssid,
 			 const u8 *req_ie, size_t req_ie_len,
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 4669391..472225d 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -38,6 +38,7 @@ struct cfg80211_conn {
 		CFG80211_CONN_ASSOCIATE_NEXT,
 		CFG80211_CONN_ASSOCIATING,
 		CFG80211_CONN_ASSOC_FAILED,
+		CFG80211_CONN_ASSOC_FAILED_TIMEOUT,
 		CFG80211_CONN_DEAUTH,
 		CFG80211_CONN_ABANDON,
 		CFG80211_CONN_CONNECTED,
@@ -140,7 +141,8 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
 	return err;
 }
 
-static int cfg80211_conn_do_work(struct wireless_dev *wdev)
+static int cfg80211_conn_do_work(struct wireless_dev *wdev,
+				 enum nl80211_timeout_reason *treason)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
 	struct cfg80211_connect_params *params;
@@ -172,6 +174,7 @@ static int cfg80211_conn_do_work(struct wireless_dev *wdev)
 					  params->key, params->key_len,
 					  params->key_idx, NULL, 0);
 	case CFG80211_CONN_AUTH_FAILED:
+		*treason = NL80211_TIMEOUT_AUTH;
 		return -ENOTCONN;
 	case CFG80211_CONN_ASSOCIATE_NEXT:
 		if (WARN_ON(!rdev->ops->assoc))
@@ -198,6 +201,9 @@ static int cfg80211_conn_do_work(struct wireless_dev *wdev)
 					     WLAN_REASON_DEAUTH_LEAVING,
 					     false);
 		return err;
+	case CFG80211_CONN_ASSOC_FAILED_TIMEOUT:
+		*treason = NL80211_TIMEOUT_ASSOC;
+		/* fall through */
 	case CFG80211_CONN_ASSOC_FAILED:
 		cfg80211_mlme_deauth(rdev, wdev->netdev, params->bssid,
 				     NULL, 0,
@@ -223,6 +229,7 @@ void cfg80211_conn_work(struct work_struct *work)
 		container_of(work, struct cfg80211_registered_device, conn_work);
 	struct wireless_dev *wdev;
 	u8 bssid_buf[ETH_ALEN], *bssid = NULL;
+	enum nl80211_timeout_reason treason;
 
 	rtnl_lock();
 
@@ -244,10 +251,12 @@ void cfg80211_conn_work(struct work_struct *work)
 			memcpy(bssid_buf, wdev->conn->params.bssid, ETH_ALEN);
 			bssid = bssid_buf;
 		}
-		if (cfg80211_conn_do_work(wdev)) {
+		treason = NL80211_TIMEOUT_UNSPECIFIED;
+		if (cfg80211_conn_do_work(wdev, &treason)) {
 			__cfg80211_connect_result(
 					wdev->netdev, bssid,
-					NULL, 0, NULL, 0, -1, false, NULL);
+					NULL, 0, NULL, 0, -1, false, NULL,
+					treason);
 		}
 		wdev_unlock(wdev);
 	}
@@ -352,7 +361,8 @@ void cfg80211_sme_rx_auth(struct wireless_dev *wdev, const u8 *buf, size_t len)
 	} else if (status_code != WLAN_STATUS_SUCCESS) {
 		__cfg80211_connect_result(wdev->netdev, mgmt->bssid,
 					  NULL, 0, NULL, 0,
-					  status_code, false, NULL);
+					  status_code, false, NULL,
+					  NL80211_TIMEOUT_UNSPECIFIED);
 	} else if (wdev->conn->state == CFG80211_CONN_AUTHENTICATING) {
 		wdev->conn->state = CFG80211_CONN_ASSOCIATE_NEXT;
 		schedule_work(&rdev->conn_work);
@@ -422,7 +432,7 @@ void cfg80211_sme_assoc_timeout(struct wireless_dev *wdev)
 	if (!wdev->conn)
 		return;
 
-	wdev->conn->state = CFG80211_CONN_ASSOC_FAILED;
+	wdev->conn->state = CFG80211_CONN_ASSOC_FAILED_TIMEOUT;
 	schedule_work(&rdev->conn_work);
 }
 
@@ -564,7 +574,9 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
 
 	/* we're good if we have a matching bss struct */
 	if (bss) {
-		err = cfg80211_conn_do_work(wdev);
+		enum nl80211_timeout_reason treason;
+
+		err = cfg80211_conn_do_work(wdev, &treason);
 		cfg80211_put_bss(wdev->wiphy, bss);
 	} else {
 		/* otherwise we'll need to scan for the AP first */
@@ -661,7 +673,8 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 			       const u8 *req_ie, size_t req_ie_len,
 			       const u8 *resp_ie, size_t resp_ie_len,
 			       int status, bool wextev,
-			       struct cfg80211_bss *bss)
+			       struct cfg80211_bss *bss,
+			       enum nl80211_timeout_reason timeout_reason)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	const u8 *country_ie;
@@ -680,7 +693,7 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 	nl80211_send_connect_result(wiphy_to_rdev(wdev->wiphy), dev,
 				    bssid, req_ie, req_ie_len,
 				    resp_ie, resp_ie_len,
-				    status, GFP_KERNEL);
+				    status, GFP_KERNEL, timeout_reason);
 
 #ifdef CONFIG_CFG80211_WEXT
 	if (wextev) {
@@ -771,7 +784,8 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 void cfg80211_connect_bss(struct net_device *dev, const u8 *bssid,
 			  struct cfg80211_bss *bss, const u8 *req_ie,
 			  size_t req_ie_len, const u8 *resp_ie,
-			  size_t resp_ie_len, int status, gfp_t gfp)
+			  size_t resp_ie_len, int status, gfp_t gfp,
+			  enum nl80211_timeout_reason timeout_reason)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
@@ -811,6 +825,7 @@ void cfg80211_connect_bss(struct net_device *dev, const u8 *bssid,
 		cfg80211_hold_bss(bss_from_pub(bss));
 	ev->cr.bss = bss;
 	ev->cr.status = status;
+	ev->cr.timeout_reason = timeout_reason;
 
 	spin_lock_irqsave(&wdev->event_lock, flags);
 	list_add_tail(&ev->list, &wdev->event_list);
diff --git a/net/wireless/util.c b/net/wireless/util.c
index cd8a7ae..1b92968 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -951,7 +951,7 @@ void cfg80211_process_wdev_events(struct wireless_dev *wdev)
 				ev->cr.resp_ie, ev->cr.resp_ie_len,
 				ev->cr.status,
 				ev->cr.status == WLAN_STATUS_SUCCESS,
-				ev->cr.bss);
+				ev->cr.bss, ev->cr.timeout_reason);
 			break;
 		case EVENT_ROAMED:
 			__cfg80211_roamed(wdev, ev->rm.bss, ev->rm.req_ie,
-- 
2.7.4

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Arend Van Spriel]

On 9-1-2017 18:53, Jouni Malinen wrote:
> From: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
> 
> This enhances the connect timeout API to also carry the reason for the
> timeout. These reason codes for the connect time out are represented by
> enum nl80211_timeout_reason and are passed to user space through a new
> attribute NL80211_ATTR_TIMEOUT_REASON (u32).
> 
> Signed-off-by: Purushottam Kushwaha <pkushwah@qti.qualcomm.com>
> Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
> ---

[...]

> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index 4669391..472225d 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -38,6 +38,7 @@ struct cfg80211_conn {
>  		CFG80211_CONN_ASSOCIATE_NEXT,
>  		CFG80211_CONN_ASSOCIATING,
>  		CFG80211_CONN_ASSOC_FAILED,
> +		CFG80211_CONN_ASSOC_FAILED_TIMEOUT,

Was kinda expecting AUTH_FAILED_TIMEOUT....

>  		CFG80211_CONN_DEAUTH,
>  		CFG80211_CONN_ABANDON,
>  		CFG80211_CONN_CONNECTED,
> @@ -140,7 +141,8 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
>  	return err;
>  }
>  
> -static int cfg80211_conn_do_work(struct wireless_dev *wdev)
> +static int cfg80211_conn_do_work(struct wireless_dev *wdev,
> +				 enum nl80211_timeout_reason *treason)
>  {
>  	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
>  	struct cfg80211_connect_params *params;
> @@ -172,6 +174,7 @@ static int cfg80211_conn_do_work(struct wireless_dev *wdev)
>  					  params->key, params->key_len,
>  					  params->key_idx, NULL, 0);
>  	case CFG80211_CONN_AUTH_FAILED:
> +		*treason = NL80211_TIMEOUT_AUTH;

... but it seems AUTH failure always is a timeout?

Regards,
Arend

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Malinen, Jouni]

On Mon, Jan 09, 2017 at 09:24:56PM +0100, Arend Van Spriel wrote:
> > diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> > @@ -38,6 +38,7 @@ struct cfg80211_conn {
> >  		CFG80211_CONN_ASSOCIATE_NEXT,
> >  		CFG80211_CONN_ASSOCIATING,
> >  		CFG80211_CONN_ASSOC_FAILED,
> > +		CFG80211_CONN_ASSOC_FAILED_TIMEOUT,
>=20
> Was kinda expecting AUTH_FAILED_TIMEOUT....

Me too when going through the changes.. But only the association failure
cases had different triggers that needed a change here.

> > @@ -172,6 +174,7 @@ static int cfg80211_conn_do_work(struct wireless_de=
v *wdev)
> >  	case CFG80211_CONN_AUTH_FAILED:
> > +		*treason =3D NL80211_TIMEOUT_AUTH;
>=20
> ... but it seems AUTH failure always is a timeout?

The CFG80211_CONN_AUTH_FAILED case is currently used only in
cfg80211_sme_auth_timeout() which is indeed always a timeout.

--=20
Jouni Malinen                                            PGP id EFC895FA=

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Johannes Berg]

On Wed, 2017-01-11 at 13:13 +0000, Malinen, Jouni wrote:
> 
> > > @@ -172,6 +174,7 @@ static int cfg80211_conn_do_work(struct
> > > wireless_dev *wdev)
> > >  	case CFG80211_CONN_AUTH_FAILED:
> > > +		*treason = NL80211_TIMEOUT_AUTH;
> > 
> > ... but it seems AUTH failure always is a timeout?
> 
> The CFG80211_CONN_AUTH_FAILED case is currently used only in
> cfg80211_sme_auth_timeout() which is indeed always a timeout.

Might be worth simply renaming it, since you have the reason there
unconditionally?

johannes

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Malinen, Jouni]

T24gV2VkLCBKYW4gMTEsIDIwMTcgYXQgMDI6MjY6MDZQTSArMDEwMCwgSm9oYW5uZXMgQmVyZyB3
cm90ZToNCj4gT24gV2VkLCAyMDE3LTAxLTExIGF0IDEzOjEzICswMDAwLCBNYWxpbmVuLCBKb3Vu
aSB3cm90ZToNCj4gPiANCj4gPiA+ID4gQEAgLTE3Miw2ICsxNzQsNyBAQCBzdGF0aWMgaW50IGNm
ZzgwMjExX2Nvbm5fZG9fd29yayhzdHJ1Y3QNCj4gPiA+ID4gd2lyZWxlc3NfZGV2ICp3ZGV2KQ0K
PiA+ID4gPiDCoAljYXNlIENGRzgwMjExX0NPTk5fQVVUSF9GQUlMRUQ6DQo+ID4gPiA+ICsJCSp0
cmVhc29uID0gTkw4MDIxMV9USU1FT1VUX0FVVEg7DQo+ID4gPiANCj4gPiA+IC4uLiBidXQgaXQg
c2VlbXMgQVVUSCBmYWlsdXJlIGFsd2F5cyBpcyBhIHRpbWVvdXQ/DQo+ID4gDQo+ID4gVGhlIENG
RzgwMjExX0NPTk5fQVVUSF9GQUlMRUQgY2FzZSBpcyBjdXJyZW50bHkgdXNlZCBvbmx5IGluDQo+
ID4gY2ZnODAyMTFfc21lX2F1dGhfdGltZW91dCgpIHdoaWNoIGlzIGluZGVlZCBhbHdheXMgYSB0
aW1lb3V0Lg0KPiANCj4gTWlnaHQgYmUgd29ydGggc2ltcGx5IHJlbmFtaW5nIGl0LCBzaW5jZSB5
b3UgaGF2ZSB0aGUgcmVhc29uIHRoZXJlDQo+IHVuY29uZGl0aW9uYWxseT8NCg0KU3VyZSwgdGhh
dCBzb3VuZHMgZmluZSBhbmQgZG9jdW1lbnRzIHRoZSBleGlzdGluZyBjYXNlIG1vcmUgYWNjdXJh
dGVseS4NCg0KLS0gDQpKb3VuaSBNYWxpbmVuICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBQR1AgaWQgRUZDODk1RkE=

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Johannes Berg]

> + * @timeout_reason: reason for connection timeout. This is used when
> the
> + *	connection fails due to a timeout instead of an explicit
> rejection from
> + *	the AP. 0 (NL80211_CONNECT_TIMEOUT_UNSPECIFIED) is used
> for other cases.

I think this description is misleading - one could easily understand
"for other cases" to indicate for the cases that the AP did explicitly
reject it, but that's obviously not true.

Perhaps that could be reworded, to say it's used when it's not known,
or such? I'd not indicate the value (0) either, just specify the name,
and put a % in front to get better formatting for it please.

> +			     resp_ie_len, status, gfp,
> +			     NL80211_TIMEOUT_UNSPECIFIED);
>  }

NL80211_CONNECT_TIMEOUT_UNSPECIFIED in the comment is wrong then.

johannes

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Malinen, Jouni]

T24gV2VkLCBKYW4gMTEsIDIwMTcgYXQgMDI6MzE6MzFQTSArMDEwMCwgSm9oYW5uZXMgQmVyZyB3
cm90ZToNCj4gPiArICogQHRpbWVvdXRfcmVhc29uOiByZWFzb24gZm9yIGNvbm5lY3Rpb24gdGlt
ZW91dC4gVGhpcyBpcyB1c2VkIHdoZW4NCj4gPiB0aGUNCj4gPiArICoJY29ubmVjdGlvbiBmYWls
cyBkdWUgdG8gYSB0aW1lb3V0IGluc3RlYWQgb2YgYW4gZXhwbGljaXQNCj4gPiByZWplY3Rpb24g
ZnJvbQ0KPiA+ICsgKgl0aGUgQVAuIDAgKE5MODAyMTFfQ09OTkVDVF9USU1FT1VUX1VOU1BFQ0lG
SUVEKSBpcyB1c2VkDQo+ID4gZm9yIG90aGVyIGNhc2VzLg0KPiANCj4gSSB0aGluayB0aGlzIGRl
c2NyaXB0aW9uIGlzIG1pc2xlYWRpbmcgLSBvbmUgY291bGQgZWFzaWx5IHVuZGVyc3RhbmQNCj4g
ImZvciBvdGhlciBjYXNlcyIgdG8gaW5kaWNhdGUgZm9yIHRoZSBjYXNlcyB0aGF0IHRoZSBBUCBk
aWQgZXhwbGljaXRseQ0KPiByZWplY3QgaXQsIGJ1dCB0aGF0J3Mgb2J2aW91c2x5IG5vdCB0cnVl
Lg0KDQpXZWxsLCB0aGUgZXhwZWN0YXRpb24gaGVyZSByZWFsbHkgd2FzIHRoYXQgdGhlIHJlYXNv
biBmb3IgdGhlIHRpbWVvdXQNCndvdWxkIGJlIGtub3duIGlmIHRoZXJlIHdhcyBhIHRpbWVvdXQg
YW5kIHRoZSB1bnNwZWNpZmllZCB2YWx1ZSB3b3VsZCBiZQ0KdXNlZCBpbiBhbGwgb3RoZXIgY2Fz
ZXMsIGkuZS4sIGluIGNhc2VzIHdoZXJlIHRoZSBBUCBkaWQgaW5kZWVkDQpleHBsaWNpdGx5IHJl
amVjdCB0aGUgY29ubmVjdGlvbi4NCg0KSSBndWVzcyB0aGVyZSBtaWdodCBiZSBhIGRyaXZlciB3
aGVyZSB0aGUgY29ubmVjdCByZXF1ZXN0IGdvZXMgaW50bw0KZmlybXdhcmUgaW1wbGVtZW50YXRp
b24gYW5kIHRoZSBkcml2ZXIgd291bGQgbm90IGtub3cgd2hldGhlciB0aGUNCm9wZXJhdGlvbiBm
YWlsZWQgZHVlIHRvIGF1dGhlbnRpY2F0aW9uIGZyYW1lIG9yIGFzc29jaWF0aW9uIGZyYW1lDQp0
aW1lb3V0IG91dC4uIEltcGxlbWVudGF0aW9uIGl0c2VsZiB3b3VsZCBzdGlsbCBiZSBmaW5lLCBi
dXQgSSBhZ3JlZQ0KdGhhdCBpdCBtaWdodCBiZSBhIGJpdCBjb25mdXNpbmcgdGhlIHRyeSB0byBp
bnRlcnByZXQgdGhlIGRlc2NyaXB0aW9uDQpoZXJlIG9uIHdoYXQgdGhlIGRyaXZlciBzaG91bGQg
ZG8uDQoNCj4gUGVyaGFwcyB0aGF0IGNvdWxkIGJlIHJld29yZGVkLCB0byBzYXkgaXQncyB1c2Vk
IHdoZW4gaXQncyBub3Qga25vd24sDQo+IG9yIHN1Y2g/IEknZCBub3QgaW5kaWNhdGUgdGhlIHZh
bHVlICgwKSBlaXRoZXIsIGp1c3Qgc3BlY2lmeSB0aGUgbmFtZSwNCj4gYW5kIHB1dCBhICUgaW4g
ZnJvbnQgdG8gZ2V0IGJldHRlciBmb3JtYXR0aW5nIGZvciBpdCBwbGVhc2UuDQoNClN1cmUsIEkg
Y2FuIHNheSB0aGF0IE5MODAyMTFfVElNRU9VVF9VTlNQRUNJRklFRCBpcyB1c2VkIHdoZW4gdGhl
IHJlYXNvbg0KZm9yIHRoZSB0aW1lb3V0IGlzIG5vdCBrbm93biBvciB0aGVyZSB3YXMgYW4gZXhw
bGljaXQgcmVqZWN0aW9uIGluc3RlYWQNCm9mIGEgdGltZW91dC4NCg0KVGhhdCBzYWlkLCBjZmc4
MDIxMV9jb25uZWN0X2JzcygpIGlzIHJlYWxseSBjdXJyZW50bHkgZG9jdW1lbnRlZCB0byBiZQ0K
dXNlZCBvbmx5IGZvciB0aGUgc3VjY2VzcyBjYXNlIGp1c3QgbGlrZSBjZmc4MDIxMV9jb25uZWN0
X3Jlc3VsdCgpLiBJbg0Kb3RoZXIgd29yZHMsIGlmIGEgZHJpdmVyIHdlcmUgdG8gY2FsbCBjZmc4
MDIxMV9jb25uZWN0X2JzcygpLCBpdCBzaG91bGQNCnJlYWxseSBhbHdheXMgc3BlY2lmeSBOTDgw
MjExX1RJTUVPVVRfVU5TUEVDSUZJRUQgaGVyZSBiYXNlZCBvbiB0aGUNCmN1cnJlbnQgZG9jdW1l
bnRlZCB1c2UuIEFsbCBmYWlsdXJlIHdvdWxkIHRoZW4gbmVlZCB0byBiZSByZXBvcnRlZCB3aXRo
DQpjZmc4MDIxMV9jb25uZWN0X3RpbWVvdXQoKSBmb3IgdGhlIHRpbWVvdXQgY2FzZSBvciBieSBu
b3QgZm9sbG93aW5nIHRoZQ0KZG9jdW1lbnRhdGlvbiBhbmQgY2FsbGluZyBjZmc4MDIxMV9jb25u
ZWN0X3Jlc3VsdCgpIG9yDQpjZmc4MDIxMV9jb25uZWN0X2JzcygpIGZvciByZWplY3Rpb24gY2Fz
ZXMuIFRoYXQgc2FpZCwgdGhlIGRvY3VtZW50YXRpb24NCmZvciB0aGUgY29ubmVjdCgpIGNhbGxi
YWNrIGZ1bmN0aW9uIGRvZXMgZGVzY3JpYmUgdGhlIGZhaWx1cmUgY2FzZQ0KYmVoYXZpb3IgY29y
cmVjdGx5LiBJIHRoaW5rIEkgY2xlYW5lZCB1cCB0aGF0IGF0IHNvbWUgcG9pbnQsIGJ1dCBkaWQg
bm90DQp1cGRhdGUgdGhlIGZ1bmN0aW9uIGRvY3VtZW50YXRpb24gYXQgdGhlIHNhbWUgdGltZS4N
Cg0KU28gaXQgbG9va3MgbGlrZSBzb21lIGFkZGl0aW9uYWwgY2xlYW51cCB3b3VsZCBiZSBuZWVk
ZWQgdG8gbWFrZSB0aGUNCmRvY3VtZW50YXRpb24gYWN0dWFsbHkgbWF0Y2ggd2hhdCB3ZSBleHBl
Y3QgdGhlIGRyaXZlciB0byBkbyBmb3INCnJlamVjdGlvbiBjYXNlcy4uIEknZCBsaWtlIHRvIGxl
YXZlIGl0IG91dCBmcm9tIHRoaXMgc3BlY2lmaWMgcGF0Y2ggYW5kDQphZGRyZXNzIHRoZSBjbGVh
bnVwIG9mIGV4aXN0aW5nIGZhaWx1cmUgY2FzZSBkZXNjcmlwdGlvbiBpbiBhIHNlcGFyYXRlDQpw
YXRjaC4NCg0KPiA+ICsJCQnCoMKgwqDCoMKgTkw4MDIxMV9USU1FT1VUX1VOU1BFQ0lGSUVEKTsN
Cj4gDQo+IE5MODAyMTFfQ09OTkVDVF9USU1FT1VUX1VOU1BFQ0lGSUVEIGluIHRoZSBjb21tZW50
IGlzIHdyb25nIHRoZW4uDQoNClllYWgsIHRoZXNlIGdvdCByZW5hbWVkIGF0IHNvbWUgcG9pbnQg
YW5kIGxvb2tzIGxpa2UgdGhhdCBvbmUgd2FzDQptaXNzZWQuDQoNCi0tIA0KSm91bmkgTWFsaW5l
biAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUEdQIGlkIEVGQzg5
NUZB

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Johannes Berg]

On Thu, 2017-01-12 at 13:58 +0000, Malinen, Jouni wrote:
> 
> > I think this description is misleading - one could easily
> > understand
> > "for other cases" to indicate for the cases that the AP did
> > explicitly
> > reject it, but that's obviously not true.
> 
> Well, the expectation here really was that the reason for the timeout
> would be known if there was a timeout and the unspecified value would
> be used in all other cases, i.e., in cases where the AP did indeed
> explicitly reject the connection.

Hmm. It doesn't really make sense to include the attribute in that case
at all though, does it?

> I guess there might be a driver where the connect request goes into
> firmware implementation and the driver would not know whether the
> operation failed due to authentication frame or association frame
> timeout out.. Implementation itself would still be fine, but I agree
> that it might be a bit confusing the try to interpret the description
> here on what the driver should do.
> 
> > Perhaps that could be reworded, to say it's used when it's not
> > known,
> > or such? I'd not indicate the value (0) either, just specify the
> > name,
> > and put a % in front to get better formatting for it please.
> 
> Sure, I can say that NL80211_TIMEOUT_UNSPECIFIED is used when the
> reason for the timeout is not known or there was an explicit
> rejection instead of a timeout.

See above - why even think about this attribute in the successful case?

> That said, cfg80211_connect_bss() is really currently documented to
> be used only for the success case just like
> cfg80211_connect_result(). In other words, if a driver were to call
> cfg80211_connect_bss(), it should really always specify
> NL80211_TIMEOUT_UNSPECIFIED here based on the current documented us.

> All failure would then need to be reported with
> cfg80211_connect_timeout() for the timeout case or by not following
> the documentation and calling cfg80211_connect_result() or
> cfg80211_connect_bss() for rejection cases. That said, the
> documentation for the connect() callback function does describe the
> failure case behavior correctly. I think I cleaned up that at some
> point, but did not update the function documentation at the same
> time.

Ok.

> So it looks like some additional cleanup would be needed to make the
> documentation actually match what we expect the driver to do for
> rejection cases.. I'd like to leave it out from this specific patch
> and address the cleanup of existing failure case description in a
> separate patch.

Fair enough. I still think we should not include the
ATTR_TIMEOUT_REASON for the successful or explicit rejection case at
all though. We can really even distinguish that in the low-level
function, I think?

johannes

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Malinen, Jouni]

On Thu, Jan 12, 2017 at 03:06:19PM +0100, Johannes Berg wrote:
> On Thu, 2017-01-12 at 13:58 +0000, Malinen, Jouni wrote:
> >=20
> > > I think this description is misleading - one could easily
> > > understand
> > > "for other cases" to indicate for the cases that the AP did
> > > explicitly
> > > reject it, but that's obviously not true.
> >=20
> > Well, the expectation here really was that the reason for the timeout
> > would be known if there was a timeout and the unspecified value would
> > be used in all other cases, i.e., in cases where the AP did indeed
> > explicitly reject the connection.
>=20
> Hmm. It doesn't really make sense to include the attribute in that case
> at all though, does it?

We don't.. This discussion here is about the C API where we cannot
remove the argument from the call without adding yet another inline
wrapper, but the actual function that generates the netlink message does
not add the timeout reason attribute for success or explicit rejection
cases.

> > Sure, I can say that NL80211_TIMEOUT_UNSPECIFIED is used when the
> > reason for the timeout is not known or there was an explicit
> > rejection instead of a timeout.
>=20
> See above - why even think about this attribute in the successful case?

See above.. C API. Or do you want yet another wrapper for
cfg80211_connect_bss() to be added while trying to hide
cfg80211_connect_bss() from drivers somehow?

> Fair enough. I still think we should not include the
> ATTR_TIMEOUT_REASON for the successful or explicit rejection case at
> all though. We can really even distinguish that in the low-level
> function, I think?

nl80211_send_connect_result() already does this:

        (status < 0 &&
         (nla_put_flag(msg, NL80211_ATTR_TIMED_OUT) ||
          nla_put_u32(msg, NL80211_ATTR_TIMEOUT_REASON, timeout_reason))) |=
|

That status =3D=3D -1 special case used to be internal special value within
cfg80211, but it gets exposed to drivers since we use
cfg80211_connect_bss() both internally and from drivers instead of
having separate wrappers for drivers for cases where the bss entry is
explicitly specified.

--=20
Jouni Malinen                                            PGP id EFC895FA=

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Johannes Berg]

> We don't.. This discussion here is about the C API where we cannot
> remove the argument from the call without adding yet another inline
> wrapper, but the actual function that generates the netlink message
> does not add the timeout reason attribute for success or explicit
> rejection cases.

Ah. But we can just say here then that it's ignored in those cases, and
not really worry about it?

johannes

Re: [PATCH 3/3] cfg80211: Specify the reason for connect timeout

[Malinen, Jouni]

On Thu, Jan 12, 2017 at 03:32:20PM +0100, Johannes Berg wrote:
>=20
> > We don't.. This discussion here is about the C API where we cannot
> > remove the argument from the call without adding yet another inline
> > wrapper, but the actual function that generates the netlink message
> > does not add the timeout reason attribute for success or explicit
> > rejection cases.
>=20
> Ah. But we can just say here then that it's ignored in those cases, and
> not really worry about it?

Sure, I'll update the comment to say that.

--=20
Jouni Malinen                                            PGP id EFC895FA=

Re: [PATCH v3 1/3] cfg80211: Add support to sched scan to report better BSSs

[Arend Van Spriel]

On 9-1-2017 18:53, Jouni Malinen wrote:
> From: vamsi krishna <vamsin@qti.qualcomm.com>
> 
> 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 better RSSI than the current connected BSS. New
> attributes to specify the relative RSSI (compared to the current BSS)
> are added to the sched scan to implement this.
> 
> Signed-off-by: vamsi krishna <vamsin@qti.qualcomm.com>
> Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
> ---
>  include/net/cfg80211.h       | 36 ++++++++++++++++++++++----------
>  include/uapi/linux/nl80211.h | 29 ++++++++++++++++++++++++++
>  net/wireless/nl80211.c       | 49 ++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 101 insertions(+), 13 deletions(-)
> 
> v3:
> - use struct cfg80211_bss_select_adjust as the data structure for
>   specifying band preference (instead of attr hardcoded for 5 GHz)
> - add relative_rssi_set boolean to have a robust mechanism for
>   determining whether the NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI
>   attribute was included
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index cb13789..9dc11d3 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1620,6 +1620,17 @@ struct cfg80211_sched_scan_plan {
>  };
>  
>  /**
> + * 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_sched_scan_request - scheduled scan request description
>   *
>   * @ssids: SSIDs to scan for (passed in the probe_reqs in active scans)
> @@ -1654,6 +1665,16 @@ 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_set: Indicates whether @relative_rssi is set or not.

So you see a use-case for doing a scan with @relative_rssi being zero,
right?

> + * @relative_rssi: Relative RSSI threshold in dB to restrict scan result
> + *	reporting in connected state to cases where a matching BSS is determined
> + *	to have better RSSI than the current connected BSS. The relative RSSI
> + *	threshold values are ignored in disconnected state.

The description says "better RSSI" so I suppose it could be typed as u8.
The last sentence is intended driver behavior

> + * @rssi_adjust: delta dB of RSSI preference to be given to the BSSs that belong
> + *	to the specified band while deciding whether a better BSS is reported
> + *	using @relative_rssi. If delta is a negative number, the BSSs that
> + *	belong to the specified band will be penalized by delta dB in relative
> + *	comparisions.
>   */
>  struct cfg80211_sched_scan_request {
>  	struct cfg80211_ssid *ssids;
> @@ -1673,6 +1694,10 @@ struct cfg80211_sched_scan_request {
>  	u8 mac_addr[ETH_ALEN] __aligned(2);
>  	u8 mac_addr_mask[ETH_ALEN] __aligned(2);
>  
> +	bool relative_rssi_set;
> +	s8 relative_rssi;
> +	struct cfg80211_bss_select_adjust rssi_adjust;
> +
>  	/* internal */
>  	struct wiphy *wiphy;
>  	struct net_device *dev;
> @@ -1981,17 +2006,6 @@ 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.
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 174f4b3..4e8bf28 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1982,6 +1982,19 @@ enum nl80211_commands {
>   * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
>   *	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 by using an offloaded operation to avoid
> + *	unnecessary wakeups.
> + *
> + * @NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST: When present the RSSI level for BSSs in
> + *	the specified band is to be adjusted before doing
> + *	%NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI based comparision to figure out
> + *	better BSSs. The attribute value is a packed structure
> + *	value as specified by &struct nl80211_bss_select_rssi_adjust.
> + *
>   * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2388,6 +2401,9 @@ enum nl80211_attrs {
>  
>  	NL80211_ATTR_BSSID,
>  
> +	NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> +	NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST,
> +
>  	/* add attributes here, update the policy in nl80211.c */
>  
>  	__NL80211_ATTR_AFTER_LAST,
> @@ -3080,6 +3096,13 @@ enum nl80211_reg_rule_attr {
>   *	how this API was implemented in the past. Also, due to the same problem,
>   *	the only way to create a matchset with only an RSSI filter (with this
>   *	attribute) is if there's only a single matchset with the RSSI attribute.
> + * @NL80211_SCHED_SCAN_MATCH_ATTR_RELATIVE_RSSI: Flag indicating whether
> + *	%NL80211_SCHED_SCAN_MATCH_ATTR_RSSI to be used as absolute RSSI or
> + *	relative to current bss's RSSI.
> + * @NL80211_SCHED_SCAN_MATCH_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_SCHED_SCAN_MATCH_ATTR_MAX: highest scheduled scan filter
>   *	attribute number currently defined
>   * @__NL80211_SCHED_SCAN_MATCH_ATTR_AFTER_LAST: internal use
> @@ -3089,6 +3112,8 @@ enum nl80211_sched_scan_match_attr {
>  
>  	NL80211_SCHED_SCAN_MATCH_ATTR_SSID,
>  	NL80211_SCHED_SCAN_MATCH_ATTR_RSSI,
> +	NL80211_SCHED_SCAN_MATCH_ATTR_RELATIVE_RSSI,
> +	NL80211_SCHED_SCAN_MATCH_ATTR_RSSI_ADJUST,
>  
>  	/* keep last */
>  	__NL80211_SCHED_SCAN_MATCH_ATTR_AFTER_LAST,
> @@ -4699,6 +4724,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_RELATIVE_RSSI: 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.
> @@ -4714,6 +4742,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_RELATIVE_RSSI,
>  
>  	/* add new features before the definition below */
>  	NUM_NL80211_EXT_FEATURES,
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index b378d0a..bb3f04a 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -405,6 +405,10 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
>  	[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_S8 },
> +	[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST] = {
> +		.len = sizeof(struct nl80211_bss_select_rssi_adjust)
> +	},
>  };
>  
>  /* policy for the key attributes */
> @@ -6950,6 +6954,12 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
>  	if (!n_plans || n_plans > wiphy->max_sched_scan_plans)
>  		return ERR_PTR(-EINVAL);
>  
> +	if (!wiphy_ext_feature_isset(
> +		    wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI) &&
> +	    (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] ||
> +	     attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]))
> +		return ERR_PTR(-EINVAL);
> +
>  	request = kzalloc(sizeof(*request)
>  			+ sizeof(*request->ssids) * n_ssids
>  			+ sizeof(*request->match_sets) * n_match_sets
> @@ -7156,6 +7166,25 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
>  		request->delay =
>  			nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_DELAY]);
>  
> +	if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]) {
> +		request->relative_rssi = nla_get_s8(
> +			attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
> +		request->relative_rssi_set = true;
> +	}
> +
> +	if (attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]) {

Maybe I misread but I thought this attribute to be applicable only if
request->relative_rssi_set is true.

> +		struct nl80211_bss_select_rssi_adjust *rssi_adjust;
> +
> +		rssi_adjust = nla_data(
> +			attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]);
> +		request->rssi_adjust.band = rssi_adjust->band;
> +		request->rssi_adjust.delta = rssi_adjust->delta;
> +		if (!is_band_valid(wiphy, request->rssi_adjust.band)) {
> +			err = -EINVAL;
> +			goto out_free;
> +		}
> +	}
> +
>  	err = nl80211_parse_sched_scan_plans(wiphy, n_plans, request, attrs);
>  	if (err)
>  		goto out_free;
> @@ -9671,7 +9700,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,

This seems to be unrelated change. At least I do not see any reference
to wiphy variable in the added code below.

> +				  struct sk_buff *msg,
>  				  struct cfg80211_sched_scan_request *req)
>  {
>  	struct nlattr *nd, *freqs, *matches, *match, *scan_plans, *scan_plan;
> @@ -9692,6 +9722,21 @@ 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 (req->relative_rssi_set &&
> +	    nla_put_s8(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> +		       req->relative_rssi))
> +		return -ENOBUFS;
> +
> +	if (req->rssi_adjust.band != __NL80211_BAND_ATTR_INVALID) {
> +		struct nl80211_bss_select_rssi_adjust rssi_adjust;
> +
> +		rssi_adjust.band = req->rssi_adjust.band;
> +		rssi_adjust.delta = req->rssi_adjust.delta;
> +		if (nla_put(msg, NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST,
> +			    sizeof(rssi_adjust), &rssi_adjust))
> +			return -ENOBUFS;
> +	}
> +
>  	freqs = nla_nest_start(msg, NL80211_ATTR_SCAN_FREQUENCIES);
>  	if (!freqs)
>  		return -ENOBUFS;
> @@ -9805,7 +9850,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;
>  
> 

Regards,
Arend

RE: [PATCH v3 1/3] cfg80211: Add support to sched scan to report better BSSs

[Vamsi, Krishna]

> -----Original Message-----
=20
> > + * @relative_rssi_set: Indicates whether @relative_rssi is set or not.
>=20
> So you see a use-case for doing a scan with @relative_rssi being zero, ri=
ght?

Yes. Zero value for relative_rssi is also valid.

> > + * @relative_rssi: Relative RSSI threshold in dB to restrict scan resu=
lt
> > + *	reporting in connected state to cases where a matching BSS is
> determined
> > + *	to have better RSSI than the current connected BSS. The relative RS=
SI
> > + *	threshold values are ignored in disconnected state.
>=20
> The description says "better RSSI" so I suppose it could be typed as u8.
> The last sentence is intended driver behavior

I like to leave this as s8 only. This will leave more flexibility to usersp=
ace especially in case of more than two bands in future.
=20
> > +	if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]) {
> > +		request->relative_rssi =3D nla_get_s8(
> > +			attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
> > +		request->relative_rssi_set =3D true;
> > +	}
> > +
> > +	if (attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]) {
>=20
> Maybe I misread but I thought this attribute to be applicable only if
> request->relative_rssi_set is true.

@relative_rssi is valid only when @relative_rssi_set is set to true and @rs=
si_adjust is valid only when @relative_rssi is valid. I think that is under=
standable to drivers and there is no need of explicit check here.

> >
> > -static int nl80211_send_wowlan_nd(struct sk_buff *msg,
> > +static int nl80211_send_wowlan_nd(struct wiphy *wiphy,
>=20
> This seems to be unrelated change. At least I do not see any reference to=
 wiphy
> variable in the added code below.

My bad, Will remove this unnecessary change and upload a patch.

Thanks,
Vamsi

Re: [PATCH v3 1/3] cfg80211: Add support to sched scan to report better BSSs

[Johannes Berg]

On Wed, 2017-01-11 at 07:48 +0000, Vamsi, Krishna wrote:
> > -----Original Message-----
> 
>  
> > > + * @relative_rssi_set: Indicates whether @relative_rssi is set
> > > or not.
> > 
> > So you see a use-case for doing a scan with @relative_rssi being
> > zero, right?
> 
> Yes. Zero value for relative_rssi is also valid.

Or negative even, I guess?

> > > + * @relative_rssi: Relative RSSI threshold in dB to restrict
> > > scan result
> > > + *	reporting in connected state to cases where a matching
> > > BSS is
> > 
> > determined
> > > + *	to have better RSSI than the current connected BSS.
> > > The relative RSSI
> > > + *	threshold values are ignored in disconnected state.
> > 
> > The description says "better RSSI" so I suppose it could be typed
> > as u8. The last sentence is intended driver behavior
> 
> I like to leave this as s8 only. This will leave more flexibility to
> userspace especially in case of more than two bands in future.

I guess you should reword that - instead of "better" it should say how
this value is applied, as a delta to the current RSSI, and then
reporting the result.

However, I don't understand your comment about this being related to
multiple bands, can you clarify? The relative_rssi just determines the
filter after the adjustment(s) done with rssi_adjust, but how could it
be relevant?

The only use case for relative_rssi being negative would be when you
actually *want* to see slightly worse networks than the one you're
connected to, e.g. to determine if you should use them because they
have better parameters (e.g. HT/VHT or soon HE).

> > > +	if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]) {
> > > +		request->relative_rssi = nla_get_s8(
> > > +			attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_R
> > > SSI]);
> > > +		request->relative_rssi_set = true;
> > > +	}
> > > +
> > > +	if (attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]) {
> > 
> > Maybe I misread but I thought this attribute to be applicable only
> > if
> > request->relative_rssi_set is true.
> 
> @relative_rssi is valid only when @relative_rssi_set is set to true
> and @rssi_adjust is valid only when @relative_rssi is valid. I think
> that is understandable to drivers and there is no need of explicit
> check here.

It wouldn't be problematic to parse the RSSI_ADJUST only when the
others are present though, so that a driver could apply the rssi_adjust
unconditionally (since, if it's not parsed, the delta will be 0.)

johannes

RE: [PATCH v3 1/3] cfg80211: Add support to sched scan to report better BSSs

[Vamsi, Krishna]

DQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEpvaGFubmVzIEJlcmcgW21h
aWx0bzpqb2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0XQ0KIA0KPiA+ID4gU28geW91IHNlZSBhIHVz
ZS1jYXNlIGZvciBkb2luZyBhIHNjYW4gd2l0aCBAcmVsYXRpdmVfcnNzaSBiZWluZw0KPiA+ID4g
emVybywgcmlnaHQ/DQo+ID4NCj4gPiBZZXMuIFplcm8gdmFsdWUgZm9yIHJlbGF0aXZlX3Jzc2kg
aXMgYWxzbyB2YWxpZC4NCj4gDQo+IE9yIG5lZ2F0aXZlIGV2ZW4sIEkgZ3Vlc3M/DQoNClllcywg
dGhpcyBjYW4gYmUgbmVnYXRpdmUgYWxzby4NCiANCj4gPiBJIGxpa2UgdG8gbGVhdmUgdGhpcyBh
cyBzOCBvbmx5LiBUaGlzIHdpbGwgbGVhdmUgbW9yZSBmbGV4aWJpbGl0eSB0bw0KPiA+IHVzZXJz
cGFjZSBlc3BlY2lhbGx5IGluIGNhc2Ugb2YgbW9yZSB0aGFuIHR3byBiYW5kcyBpbiBmdXR1cmUu
DQo+IA0KPiBJIGd1ZXNzIHlvdSBzaG91bGQgcmV3b3JkIHRoYXQgLSBpbnN0ZWFkIG9mICJiZXR0
ZXIiIGl0IHNob3VsZCBzYXkgaG93IHRoaXMgdmFsdWUNCj4gaXMgYXBwbGllZCwgYXMgYSBkZWx0
YSB0byB0aGUgY3VycmVudCBSU1NJLCBhbmQgdGhlbiByZXBvcnRpbmcgdGhlIHJlc3VsdC4NCj4g
DQo+IEhvd2V2ZXIsIEkgZG9uJ3QgdW5kZXJzdGFuZCB5b3VyIGNvbW1lbnQgYWJvdXQgdGhpcyBi
ZWluZyByZWxhdGVkIHRvIG11bHRpcGxlDQo+IGJhbmRzLCBjYW4geW91IGNsYXJpZnk/IFRoZSBy
ZWxhdGl2ZV9yc3NpIGp1c3QgZGV0ZXJtaW5lcyB0aGUgZmlsdGVyIGFmdGVyIHRoZQ0KPiBhZGp1
c3RtZW50KHMpIGRvbmUgd2l0aCByc3NpX2FkanVzdCwgYnV0IGhvdyBjb3VsZCBpdCBiZSByZWxl
dmFudD8NCj4gDQo+IFRoZSBvbmx5IHVzZSBjYXNlIGZvciByZWxhdGl2ZV9yc3NpIGJlaW5nIG5l
Z2F0aXZlIHdvdWxkIGJlIHdoZW4geW91IGFjdHVhbGx5DQo+ICp3YW50KiB0byBzZWUgc2xpZ2h0
bHkgd29yc2UgbmV0d29ya3MgdGhhbiB0aGUgb25lIHlvdSdyZSBjb25uZWN0ZWQgdG8sIGUuZy4g
dG8NCj4gZGV0ZXJtaW5lIGlmIHlvdSBzaG91bGQgdXNlIHRoZW0gYmVjYXVzZSB0aGV5IGhhdmUg
YmV0dGVyIHBhcmFtZXRlcnMgKGUuZy4NCj4gSFQvVkhUIG9yIHNvb24gSEUpLg0KDQpJIHdvdWxk
IGxpa2UgdG8gc3dhbGxvdyBteSB3b3Jkcy4gVGhlcmUgaXMgc29tZXRoaW5nIHdyb25nIHdpdGgg
bXkgZWFybGllciB0aGlua2luZy4NCiANCiANCj4gPg0KPiA+IEByZWxhdGl2ZV9yc3NpIGlzIHZh
bGlkIG9ubHkgd2hlbiBAcmVsYXRpdmVfcnNzaV9zZXQgaXMgc2V0IHRvIHRydWUNCj4gPiBhbmQg
QHJzc2lfYWRqdXN0IGlzIHZhbGlkIG9ubHkgd2hlbiBAcmVsYXRpdmVfcnNzaSBpcyB2YWxpZC4g
SSB0aGluaw0KPiA+IHRoYXQgaXMgdW5kZXJzdGFuZGFibGUgdG8gZHJpdmVycyBhbmQgdGhlcmUg
aXMgbm8gbmVlZCBvZiBleHBsaWNpdA0KPiA+IGNoZWNrIGhlcmUuDQo+IA0KPiBJdCB3b3VsZG4n
dCBiZSBwcm9ibGVtYXRpYyB0byBwYXJzZSB0aGUgUlNTSV9BREpVU1Qgb25seSB3aGVuIHRoZSBv
dGhlcnMgYXJlDQo+IHByZXNlbnQgdGhvdWdoLCBzbyB0aGF0IGEgZHJpdmVyIGNvdWxkIGFwcGx5
IHRoZSByc3NpX2FkanVzdCB1bmNvbmRpdGlvbmFsbHkNCj4gKHNpbmNlLCBpZiBpdCdzIG5vdCBw
YXJzZWQsIHRoZSBkZWx0YSB3aWxsIGJlIDAuKQ0KDQpTdXJlLCB3aWxsIHRha2UgY2FyZSBvZiB0
aGlzIGluIHRoZSBuZXh0IHBhdGNoLg0KDQpUaGFua3MsDQpWYW1zaQ0K