[RFC V2 0/5] cfg80211: support multiple scheduled scans

[RFC V2 0/5] cfg80211: support multiple scheduled scans

[Arend van Spriel]

After sending out the initial RFC for multiple scheduled scan support [1]
here a series that deal with it all (I hope) so including events and the
driver function call apis. Resending it due to build issue and it was based
on old mac80211-next master revision.

*Now* it does apply to the master branch of the mac80211-next repository.

Arend van Spriel (5):
  nl80211: allow multiple active scheduled scan requests
  nl80211: include request id in scheduled scan event messages
  cfg80211: add request id parameter to .sched_scan_stop() signature
  cfg80211: add request id to cfg80211_sched_scan_results() api
  cfg80211: add request id in cfg80211_sched_scan_stopped{,_rtnl}() api

 drivers/net/wireless/ath/ath6kl/cfg80211.c         |   4 +-
 drivers/net/wireless/ath/ath6kl/wmi.c              |   2 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         |  10 +-
 drivers/net/wireless/marvell/mwifiex/cfg80211.c    |   8 +-
 drivers/net/wireless/marvell/mwifiex/main.c        |   2 +-
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c |   2 +-
 drivers/net/wireless/marvell/mwifiex/sta_event.c   |   2 +-
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c   |   2 +-
 include/net/cfg80211.h                             |  32 +++--
 include/uapi/linux/nl80211.h                       |  12 +-
 net/mac80211/cfg.c                                 |   3 +-
 net/mac80211/pm.c                                  |   2 +-
 net/mac80211/scan.c                                |   4 +-
 net/mac80211/util.c                                |   2 +-
 net/wireless/core.c                                |  31 +++--
 net/wireless/core.h                                |  11 +-
 net/wireless/nl80211.c                             |  62 +++++++---
 net/wireless/nl80211.h                             |   3 +-
 net/wireless/rdev-ops.h                            |   8 +-
 net/wireless/scan.c                                | 135 +++++++++++++++------
 net/wireless/trace.h                               |  54 ++++++---
 21 files changed, 271 insertions(+), 120 deletions(-)

--
1.9.1

[RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Arend van Spriel]

This patch implements the idea to have multiple scheduled scan requests
running concurrently. It mainly illustrates how to deal with the incoming
request from user-space in terms of backward compatibility. In order to
use multiple scheduled scans user-space needs to provide a flag attribute
NL80211_ATTR_SCHED_SCAN_MULTI to indicate support. If not the request is
treated as a legacy scan.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 include/net/cfg80211.h       |  7 ++++
 include/uapi/linux/nl80211.h | 12 +++++-
 net/wireless/core.c          | 30 +++++++++-----
 net/wireless/core.h          | 10 ++++-
 net/wireless/nl80211.c       | 37 +++++++++++++++--
 net/wireless/rdev-ops.h      |  2 +-
 net/wireless/scan.c          | 96 ++++++++++++++++++++++++++++++++++++--------
 net/wireless/trace.h         | 18 ++++++---
 8 files changed, 173 insertions(+), 39 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b7aba6e..4b98ff8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1633,6 +1633,7 @@ struct cfg80211_bss_select_adjust {
 /**
  * struct cfg80211_sched_scan_request - scheduled scan request description
  *
+ * @reqid: identifies this request.
  * @ssids: SSIDs to scan for (passed in the probe_reqs in active scans)
  * @n_ssids: number of SSIDs
  * @n_channels: total number of channels to scan
@@ -1661,6 +1662,7 @@ struct cfg80211_bss_select_adjust {
  * @rcu_head: RCU callback used to free the struct
  * @owner_nlportid: netlink portid of owner (if this should is a request
  *	owned by a particular socket)
+ * @list: for keeping list of requests.
  * @delay: delay in seconds to use before starting the first scan
  *	cycle.  The driver may ignore this parameter and start
  *	immediately (or at any other time), if this feature is not
@@ -1677,6 +1679,7 @@ struct cfg80211_bss_select_adjust {
  *	comparisions.
  */
 struct cfg80211_sched_scan_request {
+	u64 reqid;
 	struct cfg80211_ssid *ssids;
 	int n_ssids;
 	u32 n_channels;
@@ -1704,6 +1707,7 @@ struct cfg80211_sched_scan_request {
 	unsigned long scan_start;
 	struct rcu_head rcu_head;
 	u32 owner_nlportid;
+	struct list_head list;
 
 	/* keep last */
 	struct ieee80211_channel *channels[0];
@@ -3457,6 +3461,8 @@ struct wiphy_iftype_ext_capab {
  *	this variable determines its size
  * @max_scan_ssids: maximum number of SSIDs the device can scan for in
  *	any given scan
+ * @max_sched_scan_reqs: maximum number of scheduled scan requests that
+ *	the device can run concurrently.
  * @max_sched_scan_ssids: maximum number of SSIDs the device can scan
  *	for in any given scheduled scan
  * @max_match_sets: maximum number of match sets the device can handle
@@ -3589,6 +3595,7 @@ struct wiphy {
 
 	int bss_priv_size;
 	u8 max_scan_ssids;
+	u8 max_sched_scan_reqs;
 	u8 max_sched_scan_ssids;
 	u8 max_match_sets;
 	u16 max_scan_ie_len;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c51b40c..961e381 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -351,7 +351,9 @@
  *	are used.  Extra IEs can also be passed from the userspace by
  *	using the %NL80211_ATTR_IE attribute.  The first cycle of the
  *	scheduled scan can be delayed by %NL80211_ATTR_SCHED_SCAN_DELAY
- *	is supplied.
+ *	is supplied. If the device supports multiple concurrent scheduled
+ *	scans, it will allow such when the caller provides the flag attribute
+ *	%NL80211_ATTR_SCHED_SCAN_MULTI to indicate user-space support for it.
  * @NL80211_CMD_STOP_SCHED_SCAN: stop a scheduled scan. Returns -ENOENT if
  *	scheduled scan is not running. The caller may assume that as soon
  *	as the call returns, it is safe to start a new scheduled scan again.
@@ -2000,6 +2002,11 @@ enum nl80211_commands {
  *	u32 attribute with an &enum nl80211_timeout_reason value. This is used,
  *	e.g., with %NL80211_CMD_CONNECT event.
  *
+ * @NL80211_ATTR_SCHED_SCAN_MULTI: flag attribute which user-space shall use to
+ *	indicate that it supports multiple active scheduled scan requests.
+ * @NL80211_ATTR_SCHED_SCAN_MAX_REQS: indicates maximum number of scheduled
+ *	scan request that may be active for the device (u8).
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2411,6 +2418,9 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_TIMEOUT_REASON,
 
+	NL80211_ATTR_SCHED_SCAN_MULTI,
+	NL80211_ATTR_SCHED_SCAN_MAX_REQS,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 903fc419..1f91e85 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -346,13 +346,17 @@ static void cfg80211_destroy_iface_wk(struct work_struct *work)
 static void cfg80211_sched_scan_stop_wk(struct work_struct *work)
 {
 	struct cfg80211_registered_device *rdev;
+	struct cfg80211_sched_scan_request *pos, *tmp;
 
 	rdev = container_of(work, struct cfg80211_registered_device,
 			   sched_scan_stop_wk);
 
 	rtnl_lock();
 
-	__cfg80211_stop_sched_scan(rdev, false);
+	/* request gets removed from list so need safe iterator */
+	list_for_each_entry_safe(pos, tmp, &rdev->sched_scan_req_list, list) {
+		cfg80211_stop_sched_scan_req(rdev, pos, false);
+	}
 
 	rtnl_unlock();
 }
@@ -436,6 +440,7 @@ struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv,
 	spin_lock_init(&rdev->beacon_registrations_lock);
 	spin_lock_init(&rdev->bss_lock);
 	INIT_LIST_HEAD(&rdev->bss_list);
+	INIT_LIST_HEAD(&rdev->sched_scan_req_list);
 	INIT_WORK(&rdev->scan_done_wk, __cfg80211_scan_done);
 	INIT_WORK(&rdev->sched_scan_results_wk, __cfg80211_sched_scan_results);
 	INIT_LIST_HEAD(&rdev->mlme_unreg);
@@ -690,6 +695,10 @@ int wiphy_register(struct wiphy *wiphy)
 		    (wiphy->bss_select_support & ~(BIT(__NL80211_BSS_SELECT_ATTR_AFTER_LAST) - 2))))
 		return -EINVAL;
 
+	if ((wiphy->flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN) &&
+	    !wiphy->max_sched_scan_reqs)
+		wiphy->max_sched_scan_reqs = 1;
+
 	if (wiphy->addresses)
 		memcpy(wiphy->perm_addr, wiphy->addresses[0].addr, ETH_ALEN);
 
@@ -1000,7 +1009,7 @@ void __cfg80211_leave(struct cfg80211_registered_device *rdev,
 		      struct wireless_dev *wdev)
 {
 	struct net_device *dev = wdev->netdev;
-	struct cfg80211_sched_scan_request *sched_scan_req;
+	struct cfg80211_sched_scan_request *pos, *tmp;
 
 	ASSERT_RTNL();
 	ASSERT_WDEV_LOCK(wdev);
@@ -1011,9 +1020,10 @@ void __cfg80211_leave(struct cfg80211_registered_device *rdev,
 		break;
 	case NL80211_IFTYPE_P2P_CLIENT:
 	case NL80211_IFTYPE_STATION:
-		sched_scan_req = rtnl_dereference(rdev->sched_scan_req);
-		if (sched_scan_req && dev == sched_scan_req->dev)
-			__cfg80211_stop_sched_scan(rdev, false);
+		list_for_each_entry_safe(pos, tmp, &rdev->sched_scan_req_list, list) {
+			if (dev == pos->dev)
+				cfg80211_stop_sched_scan_req(rdev, pos, false);
+		}
 
 #ifdef CONFIG_CFG80211_WEXT
 		kfree(wdev->wext.ie);
@@ -1088,7 +1098,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_registered_device *rdev;
-	struct cfg80211_sched_scan_request *sched_scan_req;
+	struct cfg80211_sched_scan_request *pos, *tmp;
 
 	if (!wdev)
 		return NOTIFY_DONE;
@@ -1157,10 +1167,10 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 			___cfg80211_scan_done(rdev, false);
 		}
 
-		sched_scan_req = rtnl_dereference(rdev->sched_scan_req);
-		if (WARN_ON(sched_scan_req &&
-			    sched_scan_req->dev == wdev->netdev)) {
-			__cfg80211_stop_sched_scan(rdev, false);
+		list_for_each_entry_safe(pos, tmp,
+					 &rdev->sched_scan_req_list, list) {
+			if (WARN_ON(pos && pos->dev == wdev->netdev))
+				cfg80211_stop_sched_scan_req(rdev, pos, false);
 		}
 
 		rdev->opencount--;
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 58ca206..cc99520 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -74,6 +74,7 @@ struct cfg80211_registered_device {
 	u32 bss_entries;
 	struct cfg80211_scan_request *scan_req; /* protected by RTNL */
 	struct sk_buff *scan_msg;
+	struct list_head sched_scan_req_list;
 	struct cfg80211_sched_scan_request __rcu *sched_scan_req;
 	unsigned long suspend_at;
 	struct work_struct scan_done_wk;
@@ -423,9 +424,16 @@ int cfg80211_validate_key_settings(struct cfg80211_registered_device *rdev,
 void __cfg80211_scan_done(struct work_struct *wk);
 void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev,
 			   bool send_message);
+void cfg80211_add_sched_scan_req(struct cfg80211_registered_device *rdev,
+				 struct cfg80211_sched_scan_request *req);
+int cfg80211_sched_scan_req_possible(struct cfg80211_registered_device *rdev,
+				     bool want_multi);
 void __cfg80211_sched_scan_results(struct work_struct *wk);
+int cfg80211_stop_sched_scan_req(struct cfg80211_registered_device *rdev,
+				 struct cfg80211_sched_scan_request *req,
+				 bool driver_initiated);
 int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev,
-			       bool driver_initiated);
+			       u64 reqid, bool driver_initiated);
 void cfg80211_upload_connect_keys(struct wireless_dev *wdev);
 int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 			  struct net_device *dev, enum nl80211_iftype ntype,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b4e7bdd..411774b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -410,6 +410,7 @@ enum nl80211_multicast_groups {
 		.len = sizeof(struct nl80211_bss_select_rssi_adjust)
 	},
 	[NL80211_ATTR_TIMEOUT_REASON] = { .type = NLA_U32 },
+	[NL80211_ATTR_SCHED_SCAN_MULTI] = { .type = NLA_FLAG },
 };
 
 /* policy for the key attributes */
@@ -1468,6 +1469,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 			       rdev->wiphy.coverage_class) ||
 		    nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCAN_SSIDS,
 			       rdev->wiphy.max_scan_ssids) ||
+		    nla_put_u8(msg, NL80211_ATTR_SCHED_SCAN_MAX_REQS,
+			       rdev->wiphy.max_sched_scan_reqs) ||
 		    nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCHED_SCAN_SSIDS,
 			       rdev->wiphy.max_sched_scan_ssids) ||
 		    nla_put_u16(msg, NL80211_ATTR_MAX_SCAN_IE_LEN,
@@ -7207,14 +7210,17 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_sched_scan_request *sched_scan_req;
+	bool want_multi;
 	int err;
 
 	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN) ||
 	    !rdev->ops->sched_scan_start)
 		return -EOPNOTSUPP;
 
-	if (rdev->sched_scan_req)
-		return -EINPROGRESS;
+	want_multi = !!info->attrs[NL80211_ATTR_SCHED_SCAN_MULTI];
+	err = cfg80211_sched_scan_req_possible(rdev, want_multi);
+	if (err)
+		return err;
 
 	sched_scan_req = nl80211_parse_sched_scan(&rdev->wiphy, wdev,
 						  info->attrs);
@@ -7223,6 +7229,14 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 	if (err)
 		goto out_err;
 
+	/* leave request id zero for legacy request
+	 * or if driver does not support multi-scheduled scan
+	 */
+	if (want_multi && rdev->wiphy.max_sched_scan_reqs > 1) {
+		while (!sched_scan_req->reqid)
+			sched_scan_req->reqid = rdev->wiphy.cookie_counter++;
+	}
+
 	err = rdev_sched_scan_start(rdev, dev, sched_scan_req);
 	if (err)
 		goto out_free;
@@ -7233,7 +7247,7 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 	if (info->attrs[NL80211_ATTR_SOCKET_OWNER])
 		sched_scan_req->owner_nlportid = info->snd_portid;
 
-	rcu_assign_pointer(rdev->sched_scan_req, sched_scan_req);
+	cfg80211_add_sched_scan_req(rdev, sched_scan_req);
 
 	nl80211_send_sched_scan(rdev, dev,
 				NL80211_CMD_START_SCHED_SCAN);
@@ -7248,13 +7262,28 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 static int nl80211_stop_sched_scan(struct sk_buff *skb,
 				   struct genl_info *info)
 {
+	struct cfg80211_sched_scan_request *req;
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	u64 cookie;
 
 	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN) ||
 	    !rdev->ops->sched_scan_stop)
 		return -EOPNOTSUPP;
 
-	return __cfg80211_stop_sched_scan(rdev, false);
+	if (info->attrs[NL80211_ATTR_COOKIE]) {
+		cookie = nla_get_u64(info->attrs[NL80211_ATTR_COOKIE]);
+		return __cfg80211_stop_sched_scan(rdev, cookie, false);
+	} else {
+		req = list_first_or_null_rcu(&rdev->sched_scan_req_list,
+					     struct cfg80211_sched_scan_request,
+					     list);
+		if (!req || req->reqid ||
+		    (req->owner_nlportid &&
+		     req->owner_nlportid != info->snd_portid))
+			return -ENOENT;
+
+		return cfg80211_stop_sched_scan_req(rdev, req, false);
+	}
 }
 
 static int nl80211_start_radar_detection(struct sk_buff *skb,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 2f42507..9ada5e2 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -802,7 +802,7 @@ static inline int rdev_get_antenna(struct cfg80211_registered_device *rdev,
 		      struct cfg80211_sched_scan_request *request)
 {
 	int ret;
-	trace_rdev_sched_scan_start(&rdev->wiphy, dev, request);
+	trace_rdev_sched_scan_start(&rdev->wiphy, dev, request->reqid);
 	ret = rdev->ops->sched_scan_start(&rdev->wiphy, dev, request);
 	trace_rdev_return_int(&rdev->wiphy, ret);
 	return ret;
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 21be56b..6704198 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -300,6 +300,64 @@ void cfg80211_scan_done(struct cfg80211_scan_request *request,
 }
 EXPORT_SYMBOL(cfg80211_scan_done);
 
+void cfg80211_add_sched_scan_req(struct cfg80211_registered_device *rdev,
+				 struct cfg80211_sched_scan_request *req)
+{
+	list_add_rcu(&req->list, &rdev->sched_scan_req_list);
+}
+
+static void cfg80211_del_sched_scan_req(struct cfg80211_registered_device *rdev,
+					struct cfg80211_sched_scan_request *req)
+{
+	list_del_rcu(&req->list);
+	kfree_rcu(req, rcu_head);
+}
+
+static struct cfg80211_sched_scan_request *
+cfg80211_find_sched_scan_req(struct cfg80211_registered_device *rdev, u64 reqid)
+{
+	struct cfg80211_sched_scan_request *pos;
+
+	list_for_each_entry(pos, &rdev->sched_scan_req_list, list) {
+		if (pos->reqid == reqid)
+			return pos;
+	}
+	return ERR_PTR(-ENOENT);
+}
+
+/*
+ * Determines if a scheduled scan request can be handled. When a legacy
+ * scheduled scan is running no other scheduled scan is allowed regardless
+ * whether the request is for legacy or multi-support scan. When a multi-support
+ * scheduled scan is running a request for legacy scan is not allowed. In this
+ * case a request for multi-support scan can be handled if resources are
+ * available, ie. struct wiphy::max_sched_scan_reqs limit is not yet reached.
+ */
+int cfg80211_sched_scan_req_possible(struct cfg80211_registered_device *rdev,
+				     bool want_multi)
+{
+	struct cfg80211_sched_scan_request *pos;
+	int i = 0;
+
+	list_for_each_entry(pos, &rdev->sched_scan_req_list, list) {
+		/* request id zero means legacy in progress */
+		if (!i && !pos->reqid)
+			return -EINPROGRESS;
+		i++;
+	}
+
+	if (i) {
+		/* no legacy allowed when multi request(s) are active */
+		if (!want_multi)
+			return -EINPROGRESS;
+
+		/* resource limit reached */
+		if (i == rdev->wiphy.max_sched_scan_reqs)
+			return -ENOSPC;
+	}
+	return 0;
+}
+
 void __cfg80211_sched_scan_results(struct work_struct *wk)
 {
 	struct cfg80211_registered_device *rdev;
@@ -347,7 +405,7 @@ void cfg80211_sched_scan_stopped_rtnl(struct wiphy *wiphy)
 
 	trace_cfg80211_sched_scan_stopped(wiphy);
 
-	__cfg80211_stop_sched_scan(rdev, true);
+	__cfg80211_stop_sched_scan(rdev, 0, true);
 }
 EXPORT_SYMBOL(cfg80211_sched_scan_stopped_rtnl);
 
@@ -359,34 +417,40 @@ void cfg80211_sched_scan_stopped(struct wiphy *wiphy)
 }
 EXPORT_SYMBOL(cfg80211_sched_scan_stopped);
 
-int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev,
-			       bool driver_initiated)
+int cfg80211_stop_sched_scan_req(struct cfg80211_registered_device *rdev,
+				 struct cfg80211_sched_scan_request *req,
+				 bool driver_initiated)
 {
-	struct cfg80211_sched_scan_request *sched_scan_req;
-	struct net_device *dev;
-
 	ASSERT_RTNL();
 
-	if (!rdev->sched_scan_req)
-		return -ENOENT;
-
-	sched_scan_req = rtnl_dereference(rdev->sched_scan_req);
-	dev = sched_scan_req->dev;
-
 	if (!driver_initiated) {
-		int err = rdev_sched_scan_stop(rdev, dev);
+		int err = rdev_sched_scan_stop(rdev, req->dev);
 		if (err)
 			return err;
 	}
 
-	nl80211_send_sched_scan(rdev, dev, NL80211_CMD_SCHED_SCAN_STOPPED);
+	nl80211_send_sched_scan(rdev, req->dev, NL80211_CMD_SCHED_SCAN_STOPPED);
 
-	RCU_INIT_POINTER(rdev->sched_scan_req, NULL);
-	kfree_rcu(sched_scan_req, rcu_head);
+	cfg80211_del_sched_scan_req(rdev, req);
 
 	return 0;
 }
 
+int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev,
+			       u64 reqid, bool driver_initiated)
+{
+	struct cfg80211_sched_scan_request *sched_scan_req;
+
+	ASSERT_RTNL();
+
+	sched_scan_req = cfg80211_find_sched_scan_req(rdev, reqid);
+	if (IS_ERR(sched_scan_req))
+		return PTR_ERR(sched_scan_req);
+
+	return cfg80211_stop_sched_scan_req(rdev, sched_scan_req,
+					    driver_initiated);
+}
+
 void cfg80211_bss_age(struct cfg80211_registered_device *rdev,
                       unsigned long age_secs)
 {
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index ea1b47e..7990c07 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -1588,20 +1588,26 @@
 	TP_ARGS(wiphy, rx, tx)
 );
 
-TRACE_EVENT(rdev_sched_scan_start,
-	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
-		 struct cfg80211_sched_scan_request *request),
-	TP_ARGS(wiphy, netdev, request),
+DECLARE_EVENT_CLASS(wiphy_netdev_id_evt,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u64 id),
+	TP_ARGS(wiphy, netdev, id),
 	TP_STRUCT__entry(
 		WIPHY_ENTRY
 		NETDEV_ENTRY
+		__field(u64, id)
 	),
 	TP_fast_assign(
 		WIPHY_ASSIGN;
 		NETDEV_ASSIGN;
+		__entry->id = id;
 	),
-	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT,
-		  WIPHY_PR_ARG, NETDEV_PR_ARG)
+	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", id: %llu",
+		  WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->id)
+);
+
+DEFINE_EVENT(wiphy_netdev_id_evt, rdev_sched_scan_start,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u64 id),
+	TP_ARGS(wiphy, netdev, id)
 );
 
 TRACE_EVENT(rdev_tdls_mgmt,
-- 
1.9.1

[RFC V2 2/5] nl80211: include request id in scheduled scan event messages

[Arend van Spriel]

Include the request id in the event messages so user-space knows
which scan the event relates to.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 net/wireless/nl80211.c | 23 +++++++++++------------
 net/wireless/nl80211.h |  3 +--
 net/wireless/scan.c    |  5 ++---
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 411774b..f303f6f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7249,8 +7249,7 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 
 	cfg80211_add_sched_scan_req(rdev, sched_scan_req);
 
-	nl80211_send_sched_scan(rdev, dev,
-				NL80211_CMD_START_SCHED_SCAN);
+	nl80211_send_sched_scan(sched_scan_req, NL80211_CMD_START_SCHED_SCAN);
 	return 0;
 
 out_free:
@@ -12932,18 +12931,19 @@ static int nl80211_prep_scan_msg(struct sk_buff *msg,
 
 static int
 nl80211_prep_sched_scan_msg(struct sk_buff *msg,
-			    struct cfg80211_registered_device *rdev,
-			    struct net_device *netdev,
-			    u32 portid, u32 seq, int flags, u32 cmd)
+			    struct cfg80211_sched_scan_request *req, u32 cmd)
 {
 	void *hdr;
 
-	hdr = nl80211hdr_put(msg, portid, seq, flags, cmd);
+	hdr = nl80211hdr_put(msg, 0, 0, 0, cmd);
 	if (!hdr)
 		return -1;
 
-	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
-	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex))
+	if (nla_put_u32(msg, NL80211_ATTR_WIPHY,
+			wiphy_to_rdev(req->wiphy)->wiphy_idx) ||
+	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, req->dev->ifindex) ||
+	    nla_put_u64_64bit(msg, NL80211_ATTR_COOKIE, req->reqid,
+			      NL80211_ATTR_PAD))
 		goto nla_put_failure;
 
 	genlmsg_end(msg, hdr);
@@ -13003,8 +13003,7 @@ void nl80211_send_scan_msg(struct cfg80211_registered_device *rdev,
 				NL80211_MCGRP_SCAN, GFP_KERNEL);
 }
 
-void nl80211_send_sched_scan(struct cfg80211_registered_device *rdev,
-			     struct net_device *netdev, u32 cmd)
+void nl80211_send_sched_scan(struct cfg80211_sched_scan_request *req, u32 cmd)
 {
 	struct sk_buff *msg;
 
@@ -13012,12 +13011,12 @@ void nl80211_send_sched_scan(struct cfg80211_registered_device *rdev,
 	if (!msg)
 		return;
 
-	if (nl80211_prep_sched_scan_msg(msg, rdev, netdev, 0, 0, 0, cmd) < 0) {
+	if (nl80211_prep_sched_scan_msg(msg, req, cmd) < 0) {
 		nlmsg_free(msg);
 		return;
 	}
 
-	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
+	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(req->wiphy), msg, 0,
 				NL80211_MCGRP_SCAN, GFP_KERNEL);
 }
 
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index e488dca..eb66de6 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -16,8 +16,7 @@ struct sk_buff *nl80211_build_scan_msg(struct cfg80211_registered_device *rdev,
 				       struct wireless_dev *wdev, bool aborted);
 void nl80211_send_scan_msg(struct cfg80211_registered_device *rdev,
 			   struct sk_buff *msg);
-void nl80211_send_sched_scan(struct cfg80211_registered_device *rdev,
-			     struct net_device *netdev, u32 cmd);
+void nl80211_send_sched_scan(struct cfg80211_sched_scan_request *req, u32 cmd);
 void nl80211_common_reg_change_event(enum nl80211_commands cmd_id,
 				     struct regulatory_request *request);
 
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 6704198..f2b2064 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -379,8 +379,7 @@ void __cfg80211_sched_scan_results(struct work_struct *wk)
 			spin_unlock_bh(&rdev->bss_lock);
 			request->scan_start = jiffies;
 		}
-		nl80211_send_sched_scan(rdev, request->dev,
-					NL80211_CMD_SCHED_SCAN_RESULTS);
+		nl80211_send_sched_scan(request, NL80211_CMD_SCHED_SCAN_RESULTS);
 	}
 
 	rtnl_unlock();
@@ -429,7 +428,7 @@ int cfg80211_stop_sched_scan_req(struct cfg80211_registered_device *rdev,
 			return err;
 	}
 
-	nl80211_send_sched_scan(rdev, req->dev, NL80211_CMD_SCHED_SCAN_STOPPED);
+	nl80211_send_sched_scan(req, NL80211_CMD_SCHED_SCAN_STOPPED);
 
 	cfg80211_del_sched_scan_req(rdev, req);
 
-- 
1.9.1

[RFC V2 3/5] cfg80211: add request id parameter to .sched_scan_stop() signature

[Arend van Spriel]

For multiple scheduled scan support the driver needs to know which
scheduled scan request is being stopped. Pass the request id in the
.sched_scan_stop() callback.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c                |  2 +-
 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c   |  6 +++---
 drivers/net/wireless/marvell/mwifiex/cfg80211.c           |  2 +-
 include/net/cfg80211.h                                    | 15 ++++++++-------
 net/mac80211/cfg.c                                        |  3 ++-
 net/wireless/rdev-ops.h                                   |  6 +++---
 net/wireless/scan.c                                       |  2 +-
 net/wireless/trace.h                                      | 10 +++++-----
 8 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index b7fe0af..1509286 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -3354,7 +3354,7 @@ static int ath6kl_cfg80211_sscan_start(struct wiphy *wiphy,
 }
 
 static int ath6kl_cfg80211_sscan_stop(struct wiphy *wiphy,
-				      struct net_device *dev)
+				      struct net_device *dev, u64 reqid)
 {
 	struct ath6kl_vif *vif = netdev_priv(dev);
 	bool stopped;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index ccae3bb..8280f19 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3392,7 +3392,7 @@ static int brcmf_start_internal_escan(struct brcmf_if *ifp,
 }
 
 static int brcmf_cfg80211_sched_scan_stop(struct wiphy *wiphy,
-					  struct net_device *ndev)
+					  struct net_device *ndev, u64 reqid)
 {
 	struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
 	struct brcmf_if *ifp = netdev_priv(ndev);
@@ -3595,7 +3595,7 @@ static s32 brcmf_cfg80211_resume(struct wiphy *wiphy)
 				      cfg->wowl.pre_pmmode);
 		cfg->wowl.active = false;
 		if (cfg->wowl.nd_enabled) {
-			brcmf_cfg80211_sched_scan_stop(cfg->wiphy, ifp->ndev);
+			brcmf_cfg80211_sched_scan_stop(cfg->wiphy, ifp->ndev, 0);
 			brcmf_fweh_unregister(cfg->pub, BRCMF_E_PFN_NET_FOUND);
 			brcmf_fweh_register(cfg->pub, BRCMF_E_PFN_NET_FOUND,
 					    brcmf_notify_sched_scan_results);
@@ -3679,7 +3679,7 @@ static s32 brcmf_cfg80211_suspend(struct wiphy *wiphy,
 
 	/* Stop scheduled scan */
 	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_PNO))
-		brcmf_cfg80211_sched_scan_stop(wiphy, ndev);
+		brcmf_cfg80211_sched_scan_stop(wiphy, ndev, 0);
 
 	/* end any scanning */
 	if (test_bit(BRCMF_SCAN_STATUS_BUSY, &cfg->scan_status))
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 1e3bd43..243349476 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -2701,7 +2701,7 @@ static int mwifiex_set_ibss_params(struct mwifiex_private *priv,
  * previous bgscan configuration in the firmware
  */
 static int mwifiex_cfg80211_sched_scan_stop(struct wiphy *wiphy,
-					    struct net_device *dev)
+					    struct net_device *dev, u64 reqid)
 {
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
 
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 4b98ff8..0a65cfe 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2709,12 +2709,12 @@ struct cfg80211_nan_func {
  * @set_cqm_txe_config: Configure connection quality monitor TX error
  *	thresholds.
  * @sched_scan_start: Tell the driver to start a scheduled scan.
- * @sched_scan_stop: Tell the driver to stop an ongoing scheduled scan. This
- *	call must stop the scheduled scan and be ready for starting a new one
- *	before it returns, i.e. @sched_scan_start may be called immediately
- *	after that again and should not fail in that case. The driver should
- *	not call cfg80211_sched_scan_stopped() for a requested stop (when this
- *	method returns 0.)
+ * @sched_scan_stop: Tell the driver to stop an ongoing scheduled scan with
+ *	given request id. This call must stop the scheduled scan and be ready
+ *	for starting a new one before it returns, i.e. @sched_scan_start may be
+ *	called immediately after that again and should not fail in that case.
+ *	The driver should not call cfg80211_sched_scan_stopped() for a requested
+ *	stop (when this method returns 0).
  *
  * @mgmt_frame_register: Notify driver that a management frame type was
  *	registered. The callback is allowed to sleep.
@@ -3009,7 +3009,8 @@ struct cfg80211_ops {
 	int	(*sched_scan_start)(struct wiphy *wiphy,
 				struct net_device *dev,
 				struct cfg80211_sched_scan_request *request);
-	int	(*sched_scan_stop)(struct wiphy *wiphy, struct net_device *dev);
+	int	(*sched_scan_stop)(struct wiphy *wiphy, struct net_device *dev,
+				   u64 reqid);
 
 	int	(*set_rekey_data)(struct wiphy *wiphy, struct net_device *dev,
 				  struct cfg80211_gtk_rekey_data *data);
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a0be2f6..85748d0 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2198,7 +2198,8 @@ static void ieee80211_abort_scan(struct wiphy *wiphy, struct wireless_dev *wdev)
 }
 
 static int
-ieee80211_sched_scan_stop(struct wiphy *wiphy, struct net_device *dev)
+ieee80211_sched_scan_stop(struct wiphy *wiphy, struct net_device *dev,
+			  u64 reqid)
 {
 	struct ieee80211_local *local = wiphy_priv(wiphy);
 
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 9ada5e2..9da0701 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -809,11 +809,11 @@ static inline int rdev_get_antenna(struct cfg80211_registered_device *rdev,
 }
 
 static inline int rdev_sched_scan_stop(struct cfg80211_registered_device *rdev,
-				       struct net_device *dev)
+				       struct net_device *dev, u64 reqid)
 {
 	int ret;
-	trace_rdev_sched_scan_stop(&rdev->wiphy, dev);
-	ret = rdev->ops->sched_scan_stop(&rdev->wiphy, dev);
+	trace_rdev_sched_scan_stop(&rdev->wiphy, dev, reqid);
+	ret = rdev->ops->sched_scan_stop(&rdev->wiphy, dev, reqid);
 	trace_rdev_return_int(&rdev->wiphy, ret);
 	return ret;
 }
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index f2b2064..04615338 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -423,7 +423,7 @@ int cfg80211_stop_sched_scan_req(struct cfg80211_registered_device *rdev,
 	ASSERT_RTNL();
 
 	if (!driver_initiated) {
-		int err = rdev_sched_scan_stop(rdev, req->dev);
+		int err = rdev_sched_scan_stop(rdev, req->dev, req->reqid);
 		if (err)
 			return err;
 	}
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 7990c07..8c04685 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -576,11 +576,6 @@
 	TP_ARGS(wiphy, netdev)
 );
 
-DEFINE_EVENT(wiphy_netdev_evt, rdev_sched_scan_stop,
-	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev),
-	TP_ARGS(wiphy, netdev)
-);
-
 DEFINE_EVENT(wiphy_netdev_evt, rdev_set_rekey_data,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev),
 	TP_ARGS(wiphy, netdev)
@@ -1610,6 +1605,11 @@
 	TP_ARGS(wiphy, netdev, id)
 );
 
+DEFINE_EVENT(wiphy_netdev_id_evt, rdev_sched_scan_stop,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u64 id),
+	TP_ARGS(wiphy, netdev, id)
+);
+
 TRACE_EVENT(rdev_tdls_mgmt,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
 		 u8 *peer, u8 action_code, u8 dialog_token,
-- 
1.9.1

[RFC V2 4/5] cfg80211: add request id to cfg80211_sched_scan_results() api

[Arend van Spriel]

Have proper request id filled in the SCHED_SCAN_RESULTS notification
toward user-space by having the driver provide it through the api.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/ath/ath6kl/wmi.c              |  2 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         |  2 +-
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c |  2 +-
 include/net/cfg80211.h                             |  4 ++-
 net/mac80211/scan.c                                |  2 +-
 net/wireless/core.c                                |  1 -
 net/wireless/core.h                                |  1 -
 net/wireless/nl80211.c                             |  2 ++
 net/wireless/scan.c                                | 30 +++++++++++-----------
 net/wireless/trace.h                               | 17 +++++++++---
 10 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 84a6d12..04df853 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -1082,7 +1082,7 @@ void ath6kl_wmi_sscan_timer(unsigned long ptr)
 {
 	struct ath6kl_vif *vif = (struct ath6kl_vif *) ptr;
 
-	cfg80211_sched_scan_results(vif->ar->wiphy);
+	cfg80211_sched_scan_results(vif->ar->wiphy, 0);
 }
 
 static int ath6kl_wmi_bssinfo_event_rx(struct wmi *wmi, u8 *datap, int len,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 8280f19..34d318e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -766,7 +766,7 @@ s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg,
 		brcmf_dbg(SCAN, "scheduled scan completed\n");
 		cfg->internal_escan = false;
 		if (!aborted)
-			cfg80211_sched_scan_results(cfg_to_wiphy(cfg));
+			cfg80211_sched_scan_results(cfg_to_wiphy(cfg), 0);
 	} else if (scan_request) {
 		struct cfg80211_scan_info info = {
 			.aborted = aborted,
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 8548027..12b471f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -1201,7 +1201,7 @@ int mwifiex_process_sta_cmdresp(struct mwifiex_private *priv, u16 cmdresp_no,
 		break;
 	case HostCmd_CMD_802_11_BG_SCAN_QUERY:
 		ret = mwifiex_ret_802_11_scan(priv, resp);
-		cfg80211_sched_scan_results(priv->wdev.wiphy);
+		cfg80211_sched_scan_results(priv->wdev.wiphy, 0);
 		mwifiex_dbg(adapter, CMD,
 			    "info: CMD_RESP: BG_SCAN result is ready!\n");
 		break;
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0a65cfe..8a635e0 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1702,6 +1702,7 @@ struct cfg80211_sched_scan_request {
 	struct cfg80211_bss_select_adjust rssi_adjust;
 
 	/* internal */
+	struct work_struct results_wk;
 	struct wiphy *wiphy;
 	struct net_device *dev;
 	unsigned long scan_start;
@@ -4455,8 +4456,9 @@ void cfg80211_scan_done(struct cfg80211_scan_request *request,
  * cfg80211_sched_scan_results - notify that new scan results are available
  *
  * @wiphy: the wiphy which got scheduled scan results
+ * @reqid: identifier for the related scheduled scan request
  */
-void cfg80211_sched_scan_results(struct wiphy *wiphy);
+void cfg80211_sched_scan_results(struct wiphy *wiphy, u64 reqid);
 
 /**
  * cfg80211_sched_scan_stopped - notify that the scheduled scan has stopped
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index faab3c4..3fd8757 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -1219,7 +1219,7 @@ void ieee80211_sched_scan_results(struct ieee80211_hw *hw)
 
 	trace_api_sched_scan_results(local);
 
-	cfg80211_sched_scan_results(hw->wiphy);
+	cfg80211_sched_scan_results(hw->wiphy, 0);
 }
 EXPORT_SYMBOL(ieee80211_sched_scan_results);
 
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 1f91e85..3ac0f91 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -442,7 +442,6 @@ struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv,
 	INIT_LIST_HEAD(&rdev->bss_list);
 	INIT_LIST_HEAD(&rdev->sched_scan_req_list);
 	INIT_WORK(&rdev->scan_done_wk, __cfg80211_scan_done);
-	INIT_WORK(&rdev->sched_scan_results_wk, __cfg80211_sched_scan_results);
 	INIT_LIST_HEAD(&rdev->mlme_unreg);
 	spin_lock_init(&rdev->mlme_unreg_lock);
 	INIT_WORK(&rdev->mlme_unreg_wk, cfg80211_mlme_unreg_wk);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index cc99520..f69a4e8 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -78,7 +78,6 @@ struct cfg80211_registered_device {
 	struct cfg80211_sched_scan_request __rcu *sched_scan_req;
 	unsigned long suspend_at;
 	struct work_struct scan_done_wk;
-	struct work_struct sched_scan_results_wk;
 
 	struct genl_info *cur_cmd_info;
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f303f6f..4f34873 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7229,6 +7229,8 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 	if (err)
 		goto out_err;
 
+	INIT_WORK(&sched_scan_req->results_wk, __cfg80211_sched_scan_results);
+
 	/* leave request id zero for legacy request
 	 * or if driver does not support multi-scheduled scan
 	 */
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 04615338..5f24c13 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -361,38 +361,38 @@ int cfg80211_sched_scan_req_possible(struct cfg80211_registered_device *rdev,
 void __cfg80211_sched_scan_results(struct work_struct *wk)
 {
 	struct cfg80211_registered_device *rdev;
-	struct cfg80211_sched_scan_request *request;
+	struct cfg80211_sched_scan_request *req;
 
-	rdev = container_of(wk, struct cfg80211_registered_device,
-			    sched_scan_results_wk);
+	req = container_of(wk, struct cfg80211_sched_scan_request, results_wk);
+	rdev = wiphy_to_rdev(req->wiphy);
 
 	rtnl_lock();
 
-	request = rtnl_dereference(rdev->sched_scan_req);
-
 	/* we don't have sched_scan_req anymore if the scan is stopping */
-	if (request) {
-		if (request->flags & NL80211_SCAN_FLAG_FLUSH) {
+	if (req) {
+		if (req->flags & NL80211_SCAN_FLAG_FLUSH) {
 			/* flush entries from previous scans */
 			spin_lock_bh(&rdev->bss_lock);
-			__cfg80211_bss_expire(rdev, request->scan_start);
+			__cfg80211_bss_expire(rdev, req->scan_start);
 			spin_unlock_bh(&rdev->bss_lock);
-			request->scan_start = jiffies;
+			req->scan_start = jiffies;
 		}
-		nl80211_send_sched_scan(request, NL80211_CMD_SCHED_SCAN_RESULTS);
+		nl80211_send_sched_scan(req, NL80211_CMD_SCHED_SCAN_RESULTS);
 	}
 
 	rtnl_unlock();
 }
 
-void cfg80211_sched_scan_results(struct wiphy *wiphy)
+void cfg80211_sched_scan_results(struct wiphy *wiphy, u64 reqid)
 {
-	trace_cfg80211_sched_scan_results(wiphy);
+	struct cfg80211_sched_scan_request *request;
+
+	trace_cfg80211_sched_scan_results(wiphy, reqid);
 	/* ignore if we're not scanning */
 
-	if (rcu_access_pointer(wiphy_to_rdev(wiphy)->sched_scan_req))
-		queue_work(cfg80211_wq,
-			   &wiphy_to_rdev(wiphy)->sched_scan_results_wk);
+	request = cfg80211_find_sched_scan_req(wiphy_to_rdev(wiphy), reqid);
+	if (!IS_ERR(request))
+		queue_work(cfg80211_wq, &request->results_wk);
 }
 EXPORT_SYMBOL(cfg80211_sched_scan_results);
 
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 8c04685..cb10ce7 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2795,14 +2795,23 @@
 		  MAC_PR_ARG(tsf_bssid))
 );
 
-DEFINE_EVENT(wiphy_only_evt, cfg80211_sched_scan_results,
+DEFINE_EVENT(wiphy_only_evt, cfg80211_sched_scan_stopped,
 	TP_PROTO(struct wiphy *wiphy),
 	TP_ARGS(wiphy)
 );
 
-DEFINE_EVENT(wiphy_only_evt, cfg80211_sched_scan_stopped,
-	TP_PROTO(struct wiphy *wiphy),
-	TP_ARGS(wiphy)
+TRACE_EVENT(cfg80211_sched_scan_results,
+	TP_PROTO(struct wiphy *wiphy, u64 id),
+	TP_ARGS(wiphy, id),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		__field(u64, id)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		__entry->id = id;
+	),
+	TP_printk(WIPHY_PR_FMT ", id: %llu", WIPHY_PR_ARG, __entry->id)
 );
 
 TRACE_EVENT(cfg80211_get_bss,
-- 
1.9.1

[RFC V2 5/5] cfg80211: add request id in cfg80211_sched_scan_stopped{,_rtnl}() api

[Arend van Spriel]

Allow driver to indicate which scheduled scan request is being aborted
by providing the request id.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c              |  2 +-
 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c |  2 +-
 drivers/net/wireless/marvell/mwifiex/cfg80211.c         |  6 +++---
 drivers/net/wireless/marvell/mwifiex/main.c             |  2 +-
 drivers/net/wireless/marvell/mwifiex/sta_event.c        |  2 +-
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c        |  2 +-
 include/net/cfg80211.h                                  |  6 ++++--
 net/mac80211/pm.c                                       |  2 +-
 net/mac80211/scan.c                                     |  2 +-
 net/mac80211/util.c                                     |  2 +-
 net/wireless/scan.c                                     | 10 +++++-----
 net/wireless/trace.h                                    | 17 +++++++++++------
 12 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 1509286..e52aef8 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -170,7 +170,7 @@ static void ath6kl_cfg80211_sscan_disable(struct ath6kl_vif *vif)
 	if (!stopped)
 		return;
 
-	cfg80211_sched_scan_stopped(ar->wiphy);
+	cfg80211_sched_scan_stopped(ar->wiphy, 0);
 }
 
 static int ath6kl_set_wpa_version(struct ath6kl_vif *vif,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 34d318e..8c5e54a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3359,7 +3359,7 @@ static int brcmf_start_internal_escan(struct brcmf_if *ifp,
 		goto free_req;
 
 out_err:
-	cfg80211_sched_scan_stopped(wiphy);
+	cfg80211_sched_scan_stopped(wiphy, 0);
 free_req:
 	kfree(request);
 	return err;
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 243349476..b5b7741 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -2036,7 +2036,7 @@ static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
 
 	if (!mwifiex_stop_bg_scan(priv))
-		cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy);
+		cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy, 0);
 
 	if (mwifiex_deauthenticate(priv, NULL))
 		return -EFAULT;
@@ -2304,7 +2304,7 @@ static int mwifiex_cfg80211_inform_ibss_bss(struct mwifiex_private *priv)
 		    (int)sme->ssid_len, (char *)sme->ssid, sme->bssid);
 
 	if (!mwifiex_stop_bg_scan(priv))
-		cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy);
+		cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy, 0);
 
 	ret = mwifiex_cfg80211_assoc(priv, sme->ssid_len, sme->ssid, sme->bssid,
 				     priv->bss_mode, sme->channel, sme, 0);
@@ -2513,7 +2513,7 @@ static int mwifiex_set_ibss_params(struct mwifiex_private *priv,
 		priv->scan_block = false;
 
 	if (!mwifiex_stop_bg_scan(priv))
-		cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy);
+		cfg80211_sched_scan_stopped_rtnl(priv->wdev.wiphy, 0);
 
 	user_scan_cfg = kzalloc(sizeof(*user_scan_cfg), GFP_KERNEL);
 	if (!user_scan_cfg)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index e5c3a8a..ff9b767 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -750,7 +750,7 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter,
 		mwifiex_dbg(priv->adapter, INFO,
 			    "aborting bgscan on ndo_stop\n");
 		mwifiex_stop_bg_scan(priv);
-		cfg80211_sched_scan_stopped(priv->wdev.wiphy);
+		cfg80211_sched_scan_stopped(priv->wdev.wiphy, 0);
 	}
 
 	return 0;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c b/drivers/net/wireless/marvell/mwifiex/sta_event.c
index 9df0c4d..cce61d8 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_event.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c
@@ -793,7 +793,7 @@ int mwifiex_process_sta_event(struct mwifiex_private *priv)
 
 	case EVENT_BG_SCAN_STOPPED:
 		dev_dbg(adapter->dev, "event: BGS_STOPPED\n");
-		cfg80211_sched_scan_stopped(priv->wdev.wiphy);
+		cfg80211_sched_scan_stopped(priv->wdev.wiphy, 0);
 		if (priv->sched_scanning)
 			priv->sched_scanning = false;
 		break;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index 644f3a2..d9a9e25 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -560,7 +560,7 @@ int mwifiex_enable_hs(struct mwifiex_adapter *adapter)
 #endif
 			mwifiex_dbg(adapter, CMD, "aborting bgscan!\n");
 			mwifiex_stop_bg_scan(priv);
-			cfg80211_sched_scan_stopped(priv->wdev.wiphy);
+			cfg80211_sched_scan_stopped(priv->wdev.wiphy, 0);
 #ifdef CONFIG_PM
 		}
 #endif
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8a635e0..fc3f0b2 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4464,24 +4464,26 @@ void cfg80211_scan_done(struct cfg80211_scan_request *request,
  * cfg80211_sched_scan_stopped - notify that the scheduled scan has stopped
  *
  * @wiphy: the wiphy on which the scheduled scan stopped
+ * @reqid: identifier for the related scheduled scan request
  *
  * The driver can call this function to inform cfg80211 that the
  * scheduled scan had to be stopped, for whatever reason.  The driver
  * is then called back via the sched_scan_stop operation when done.
  */
-void cfg80211_sched_scan_stopped(struct wiphy *wiphy);
+void cfg80211_sched_scan_stopped(struct wiphy *wiphy, u64 reqid);
 
 /**
  * cfg80211_sched_scan_stopped_rtnl - notify that the scheduled scan has stopped
  *
  * @wiphy: the wiphy on which the scheduled scan stopped
+ * @reqid: identifier for the related scheduled scan request
  *
  * The driver can call this function to inform cfg80211 that the
  * scheduled scan had to be stopped, for whatever reason.  The driver
  * is then called back via the sched_scan_stop operation when done.
  * This function should be called with rtnl locked.
  */
-void cfg80211_sched_scan_stopped_rtnl(struct wiphy *wiphy);
+void cfg80211_sched_scan_stopped_rtnl(struct wiphy *wiphy, u64 reqid);
 
 /**
  * cfg80211_inform_bss_frame_data - inform cfg80211 of a received BSS frame
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index 28a3a09..d37de3f 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -10,7 +10,7 @@ static void ieee80211_sched_scan_cancel(struct ieee80211_local *local)
 {
 	if (ieee80211_request_sched_scan_stop(local))
 		return;
-	cfg80211_sched_scan_stopped_rtnl(local->hw.wiphy);
+	cfg80211_sched_scan_stopped_rtnl(local->hw.wiphy, 0);
 }
 
 int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 3fd8757..94be10e 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -1239,7 +1239,7 @@ void ieee80211_sched_scan_end(struct ieee80211_local *local)
 
 	mutex_unlock(&local->mtx);
 
-	cfg80211_sched_scan_stopped(local->hw.wiphy);
+	cfg80211_sched_scan_stopped(local->hw.wiphy, 0);
 }
 
 void ieee80211_sched_scan_stopped_work(struct work_struct *work)
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index ac59fbd..616d55d 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2103,7 +2103,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 	mutex_unlock(&local->mtx);
 
 	if (sched_scan_stopped)
-		cfg80211_sched_scan_stopped_rtnl(local->hw.wiphy);
+		cfg80211_sched_scan_stopped_rtnl(local->hw.wiphy, 0);
 
  wake_up:
 	if (local->in_reconfig) {
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 5f24c13..22bb9fd 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -396,22 +396,22 @@ void cfg80211_sched_scan_results(struct wiphy *wiphy, u64 reqid)
 }
 EXPORT_SYMBOL(cfg80211_sched_scan_results);
 
-void cfg80211_sched_scan_stopped_rtnl(struct wiphy *wiphy)
+void cfg80211_sched_scan_stopped_rtnl(struct wiphy *wiphy, u64 reqid)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
 
 	ASSERT_RTNL();
 
-	trace_cfg80211_sched_scan_stopped(wiphy);
+	trace_cfg80211_sched_scan_stopped(wiphy, reqid);
 
-	__cfg80211_stop_sched_scan(rdev, 0, true);
+	__cfg80211_stop_sched_scan(rdev, reqid, true);
 }
 EXPORT_SYMBOL(cfg80211_sched_scan_stopped_rtnl);
 
-void cfg80211_sched_scan_stopped(struct wiphy *wiphy)
+void cfg80211_sched_scan_stopped(struct wiphy *wiphy, u64 reqid)
 {
 	rtnl_lock();
-	cfg80211_sched_scan_stopped_rtnl(wiphy);
+	cfg80211_sched_scan_stopped_rtnl(wiphy, reqid);
 	rtnl_unlock();
 }
 EXPORT_SYMBOL(cfg80211_sched_scan_stopped);
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index cb10ce7..7d03a0f 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2795,12 +2795,7 @@
 		  MAC_PR_ARG(tsf_bssid))
 );
 
-DEFINE_EVENT(wiphy_only_evt, cfg80211_sched_scan_stopped,
-	TP_PROTO(struct wiphy *wiphy),
-	TP_ARGS(wiphy)
-);
-
-TRACE_EVENT(cfg80211_sched_scan_results,
+DECLARE_EVENT_CLASS(wiphy_id_evt,
 	TP_PROTO(struct wiphy *wiphy, u64 id),
 	TP_ARGS(wiphy, id),
 	TP_STRUCT__entry(
@@ -2814,6 +2809,16 @@
 	TP_printk(WIPHY_PR_FMT ", id: %llu", WIPHY_PR_ARG, __entry->id)
 );
 
+DEFINE_EVENT(wiphy_id_evt, cfg80211_sched_scan_stopped,
+	TP_PROTO(struct wiphy *wiphy, u64 id),
+	TP_ARGS(wiphy, id)
+);
+
+DEFINE_EVENT(wiphy_id_evt, cfg80211_sched_scan_results,
+	TP_PROTO(struct wiphy *wiphy, u64 id),
+	TP_ARGS(wiphy, id)
+);
+
 TRACE_EVENT(cfg80211_get_bss,
 	TP_PROTO(struct wiphy *wiphy, struct ieee80211_channel *channel,
 		 const u8 *bssid, const u8 *ssid, size_t ssid_len,
-- 
1.9.1

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

> + * @max_sched_scan_reqs: maximum number of scheduled scan requests
> that
> + *	the device can run concurrently.

Perhaps we should get rid of WIPHY_FLAG_SUPPORTS_SCHED_SCAN and just
set this to 1 for such devices? Otherwise we have two different
requirements, and we need to track that 0 is an invalid value here if
WIPHY_FLAG_SUPPORTS_SCHED_SCAN is set, or something like that?

> + * @NL80211_ATTR_SCHED_SCAN_MAX_REQS: indicates maximum number of
> scheduled
> + *	scan request that may be active for the device (u8).

I'd make that a u32 - not that I believe we'll ever want to change this
in the future, but there's simply no value in making it a u8 since it
uses the same amount of space in a netlink message.

> +	list_for_each_entry_safe(pos, tmp, &rdev-
> >sched_scan_req_list, list) {
> +		cfg80211_stop_sched_scan_req(rdev, pos, false);
> +	}

nit: don't really need braces here.

> +	if ((wiphy->flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN) &&
> +	    !wiphy->max_sched_scan_reqs)
> +		wiphy->max_sched_scan_reqs = 1;

Yeah, this. Why bother?

(should even be simple to come up with an spatch to change all the
drivers, but there are only five anyway)

> +		    nla_put_u8(msg,
> NL80211_ATTR_SCHED_SCAN_MAX_REQS,
> +			       rdev->wiphy.max_sched_scan_reqs) ||
>  		    nla_put_u8(msg,
> NL80211_ATTR_MAX_NUM_SCHED_SCAN_SSIDS,
>  			       rdev->wiphy.max_sched_scan_ssids) ||

This might break older userspace - you'll have to put it in a later
portion of the code.

I'm also a bit surprised the attributes aren't actually optional for
when sched scan isn't supported, I'd make the new one optional and I
guess we can fix the others later too, if desired.

> +	bool want_multi;

That's bool

> +	want_multi = !!info->attrs[NL80211_ATTR_SCHED_SCAN_MULTI];

so you don't really need the !! as it's implied by the rules for bool
:)

> +	/* leave request id zero for legacy request
> +	 * or if driver does not support multi-scheduled scan
> +	 */
> +	if (want_multi && rdev->wiphy.max_sched_scan_reqs > 1) {

Why do the >1 check here? It probably doesn't really make a difference
since only one can be running at a time, but it might be nicer - at
least for debug in userspace - to have a real value for all multi
scans?

> +		while (!sched_scan_req->reqid)

Pretty sure we won't run over the u64 ... but I guess it doesn't matter
much :)


I don't see you sending the reqid/cookie back to userspace here though,
that's missing?

>  static int nl80211_stop_sched_scan(struct sk_buff *skb,
>  				   struct genl_info *info)
>  {
> +	struct cfg80211_sched_scan_request *req;
>  	struct cfg80211_registered_device *rdev = info->user_ptr[0];
> +	u64 cookie;
>  
>  	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN) ||
>  	    !rdev->ops->sched_scan_stop)
>  		return -EOPNOTSUPP;
>  
> -	return __cfg80211_stop_sched_scan(rdev, false);
> +	if (info->attrs[NL80211_ATTR_COOKIE]) {
> +		cookie = nla_get_u64(info-
> >attrs[NL80211_ATTR_COOKIE]);
> +		return __cfg80211_stop_sched_scan(rdev, cookie,
> false);
> +	} else {
> +		req = list_first_or_null_rcu(&rdev-
> >sched_scan_req_list,
> +					     struct
> cfg80211_sched_scan_request,
> +					     list);
> +		if (!req || req->reqid ||
> +		    (req->owner_nlportid &&
> +		     req->owner_nlportid != info->snd_portid))
> +			return -ENOENT;

Shouldn't this also check that it's non-multi?

> +void cfg80211_add_sched_scan_req(struct cfg80211_registered_device
> *rdev,
> +				 struct cfg80211_sched_scan_request
> *req)
> +{
> +	list_add_rcu(&req->list, &rdev->sched_scan_req_list);
> +}
> +
> +static void cfg80211_del_sched_scan_req(struct
> cfg80211_registered_device *rdev,
> +					struct
> cfg80211_sched_scan_request *req)
> +{
> +	list_del_rcu(&req->list);
> +	kfree_rcu(req, rcu_head);
> +}

Some locking assertions in these would be good, I think.

> +static struct cfg80211_sched_scan_request *
> +cfg80211_find_sched_scan_req(struct cfg80211_registered_device
> *rdev, u64 reqid)
> +{
> +	struct cfg80211_sched_scan_request *pos;
> +
> +	list_for_each_entry(pos, &rdev->sched_scan_req_list, list) {
> +		if (pos->reqid == reqid)
> +			return pos;
> +	}
> +	return ERR_PTR(-ENOENT);
> +}

Here too, I guess, since you don't actually use RCU.

johannes

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Arend Van Spriel]

On 24-1-2017 10:40, Johannes Berg wrote:
> 
>> + * @max_sched_scan_reqs: maximum number of scheduled scan requests
>> that
>> + *	the device can run concurrently.
> 
> Perhaps we should get rid of WIPHY_FLAG_SUPPORTS_SCHED_SCAN and just
> set this to 1 for such devices? Otherwise we have two different
> requirements, and we need to track that 0 is an invalid value here if
> WIPHY_FLAG_SUPPORTS_SCHED_SCAN is set, or something like that?

Ok. Doesn't that cause issues in user-space. Or do you only want to get
rid of it in cfg80211 api and report the flag to user-space when
max_sched_scan_reqs equals 1?

>> + * @NL80211_ATTR_SCHED_SCAN_MAX_REQS: indicates maximum number of
>> scheduled
>> + *	scan request that may be active for the device (u8).
> 
> I'd make that a u32 - not that I believe we'll ever want to change this
> in the future, but there's simply no value in making it a u8 since it
> uses the same amount of space in a netlink message.

Ok.

>> +	list_for_each_entry_safe(pos, tmp, &rdev-
>>> sched_scan_req_list, list) {
>> +		cfg80211_stop_sched_scan_req(rdev, pos, false);
>> +	}
> 
> nit: don't really need braces here.

True.

>> +	if ((wiphy->flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN) &&
>> +	    !wiphy->max_sched_scan_reqs)
>> +		wiphy->max_sched_scan_reqs = 1;
> 
> Yeah, this. Why bother?
> 
> (should even be simple to come up with an spatch to change all the
> drivers, but there are only five anyway)

Done.

>> +		    nla_put_u8(msg,
>> NL80211_ATTR_SCHED_SCAN_MAX_REQS,
>> +			       rdev->wiphy.max_sched_scan_reqs) ||
>>  		    nla_put_u8(msg,
>> NL80211_ATTR_MAX_NUM_SCHED_SCAN_SSIDS,
>>  			       rdev->wiphy.max_sched_scan_ssids) ||
> 
> This might break older userspace - you'll have to put it in a later
> portion of the code.
> 
> I'm also a bit surprised the attributes aren't actually optional for
> when sched scan isn't supported, I'd make the new one optional and I
> guess we can fix the others later too, if desired.

Why would it break user-space. Is the order in which attributes are
added into the stream something user-space relies on.

>> +	bool want_multi;
> 
> That's bool
> 
>> +	want_multi = !!info->attrs[NL80211_ATTR_SCHED_SCAN_MULTI];
> 
> so you don't really need the !! as it's implied by the rules for bool
> :)

I see.

>> +	/* leave request id zero for legacy request
>> +	 * or if driver does not support multi-scheduled scan
>> +	 */
>> +	if (want_multi && rdev->wiphy.max_sched_scan_reqs > 1) {
> 
> Why do the >1 check here? It probably doesn't really make a difference
> since only one can be running at a time, but it might be nicer - at
> least for debug in userspace - to have a real value for all multi
> scans?
> 
>> +		while (!sched_scan_req->reqid)
> 
> Pretty sure we won't run over the u64 ... but I guess it doesn't matter
> much :)
> 
> 
> I don't see you sending the reqid/cookie back to userspace here though,
> that's missing?

Indeed that is in the second patch. Maybe I should put that commit first
in the series.

>>  static int nl80211_stop_sched_scan(struct sk_buff *skb,
>>  				   struct genl_info *info)
>>  {
>> +	struct cfg80211_sched_scan_request *req;
>>  	struct cfg80211_registered_device *rdev = info->user_ptr[0];
>> +	u64 cookie;
>>  
>>  	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN) ||
>>  	    !rdev->ops->sched_scan_stop)
>>  		return -EOPNOTSUPP;
>>  
>> -	return __cfg80211_stop_sched_scan(rdev, false);
>> +	if (info->attrs[NL80211_ATTR_COOKIE]) {
>> +		cookie = nla_get_u64(info-
>>> attrs[NL80211_ATTR_COOKIE]);
>> +		return __cfg80211_stop_sched_scan(rdev, cookie,
>> false);
>> +	} else {
>> +		req = list_first_or_null_rcu(&rdev-
>>> sched_scan_req_list,
>> +					     struct
>> cfg80211_sched_scan_request,
>> +					     list);
>> +		if (!req || req->reqid ||
>> +		    (req->owner_nlportid &&
>> +		     req->owner_nlportid != info->snd_portid))
>> +			return -ENOENT;
> 
> Shouldn't this also check that it's non-multi?

non-multi == (req->reqid == 0). non-multi/legacy and multi can not be
active at the same time so we can use list_first_or_null_rcu here. If
req->reqid of first entry is non-zero there is no non-multi to stop here.

>> +void cfg80211_add_sched_scan_req(struct cfg80211_registered_device
>> *rdev,
>> +				 struct cfg80211_sched_scan_request
>> *req)
>> +{
>> +	list_add_rcu(&req->list, &rdev->sched_scan_req_list);
>> +}
>> +
>> +static void cfg80211_del_sched_scan_req(struct
>> cfg80211_registered_device *rdev,
>> +					struct
>> cfg80211_sched_scan_request *req)
>> +{
>> +	list_del_rcu(&req->list);
>> +	kfree_rcu(req, rcu_head);
>> +}
> 
> Some locking assertions in these would be good, I think.

Will do.

>> +static struct cfg80211_sched_scan_request *
>> +cfg80211_find_sched_scan_req(struct cfg80211_registered_device
>> *rdev, u64 reqid)
>> +{
>> +	struct cfg80211_sched_scan_request *pos;
>> +
>> +	list_for_each_entry(pos, &rdev->sched_scan_req_list, list) {
>> +		if (pos->reqid == reqid)
>> +			return pos;
>> +	}
>> +	return ERR_PTR(-ENOENT);
>> +}
> 
> Here too, I guess, since you don't actually use RCU.

So should I use RCU here? Not sure what is the better choice here.

Regards,
Arend

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

On Tue, 2017-02-14 at 13:35 +0100, Arend Van Spriel wrote:
> On 24-1-2017 10:40, Johannes Berg wrote:
> > 
> > > + * @max_sched_scan_reqs: maximum number of scheduled scan
> > > requests
> > > that
> > > + *	the device can run concurrently.
> > 
> > Perhaps we should get rid of WIPHY_FLAG_SUPPORTS_SCHED_SCAN and
> > just
> > set this to 1 for such devices? Otherwise we have two different
> > requirements, and we need to track that 0 is an invalid value here
> > if
> > WIPHY_FLAG_SUPPORTS_SCHED_SCAN is set, or something like that?
> 
> Ok. Doesn't that cause issues in user-space. Or do you only want to
> get rid of it in cfg80211 api and report the flag to user-space when
> max_sched_scan_reqs equals 1?

WIPHY_FLAG_* aren't directly reported to userspace at all.

> > This might break older userspace - you'll have to put it in a later
> > portion of the code.
> > 
> > I'm also a bit surprised the attributes aren't actually optional
> > for when sched scan isn't supported, I'd make the new one optional
> > and I guess we can fix the others later too, if desired.
> 
> Why would it break user-space. Is the order in which attributes are
> added into the stream something user-space relies on.

No. But there was a size limit on how much older userspace could
process before we did the splitting.

> > > +static struct cfg80211_sched_scan_request *
> > > +cfg80211_find_sched_scan_req(struct cfg80211_registered_device
> > > *rdev, u64 reqid)
> > > +{
> > > +	struct cfg80211_sched_scan_request *pos;
> > > +
> > > +	list_for_each_entry(pos, &rdev->sched_scan_req_list,
> > > list) {
> > > +		if (pos->reqid == reqid)
> > > +			return pos;
> > > +	}
> > > +	return ERR_PTR(-ENOENT);
> > > +}
> > 
> > Here too, I guess, since you don't actually use RCU.
> 
> So should I use RCU here? Not sure what is the better choice here.

No no, I just meant to add locking assertions :)

johannes

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Arend Van Spriel]

On 14-2-2017 13:51, Johannes Berg wrote:
> On Tue, 2017-02-14 at 13:35 +0100, Arend Van Spriel wrote:
>> On 24-1-2017 10:40, Johannes Berg wrote:
>>>
>>>> + * @max_sched_scan_reqs: maximum number of scheduled scan
>>>> requests
>>>> that
>>>> + *	the device can run concurrently.
>>>
>>> Perhaps we should get rid of WIPHY_FLAG_SUPPORTS_SCHED_SCAN and
>>> just
>>> set this to 1 for such devices? Otherwise we have two different
>>> requirements, and we need to track that 0 is an invalid value here
>>> if
>>> WIPHY_FLAG_SUPPORTS_SCHED_SCAN is set, or something like that?
>>
>> Ok. Doesn't that cause issues in user-space. Or do you only want to
>> get rid of it in cfg80211 api and report the flag to user-space when
>> max_sched_scan_reqs equals 1?
> 
> WIPHY_FLAG_* aren't directly reported to userspace at all.

I noticed diving in the code. Indirectly it is used for reporting
support of sched_scan_start command. Anyway, I got rid of it.

>>> This might break older userspace - you'll have to put it in a later
>>> portion of the code.
>>>
>>> I'm also a bit surprised the attributes aren't actually optional
>>> for when sched scan isn't supported, I'd make the new one optional
>>> and I guess we can fix the others later too, if desired.
>>
>> Why would it break user-space. Is the order in which attributes are
>> added into the stream something user-space relies on.
> 
> No. But there was a size limit on how much older userspace could
> process before we did the splitting.

I see. So basically adding stuff to (split_start == 0) is not wanted.
Just trying to get a clear requirement/rule here. Do we (still) know the
exact size limit?

>>>> +static struct cfg80211_sched_scan_request *
>>>> +cfg80211_find_sched_scan_req(struct cfg80211_registered_device
>>>> *rdev, u64 reqid)
>>>> +{
>>>> +	struct cfg80211_sched_scan_request *pos;
>>>> +
>>>> +	list_for_each_entry(pos, &rdev->sched_scan_req_list,
>>>> list) {
>>>> +		if (pos->reqid == reqid)
>>>> +			return pos;
>>>> +	}
>>>> +	return ERR_PTR(-ENOENT);
>>>> +}
>>>
>>> Here too, I guess, since you don't actually use RCU.
>>
>> So should I use RCU here? Not sure what is the better choice here.
> 
> No no, I just meant to add locking assertions :)

Sure. Did that already pending your reply ;-)

Regards,
Arend

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

On Tue, 2017-02-14 at 14:07 +0100, Arend Van Spriel wrote:

> > No. But there was a size limit on how much older userspace could
> > process before we did the splitting.
> 
> I see. So basically adding stuff to (split_start == 0) is not wanted.

Correct.

> Just trying to get a clear requirement/rule here. Do we (still) know
> the exact size limit?

No, I don't remember. It's pretty ancient versions of iw/wpa_s that
weren't able to deal with it. There was also interaction with the
allocations in the dump function in nl80211 code itself, but we made
those pretty big already and userspace limits came into play iirc.

johannes

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Arend Van Spriel]

On 24-1-2017 10:40, Johannes Berg wrote:
>> +	/* leave request id zero for legacy request
>> +	 * or if driver does not support multi-scheduled scan
>> +	 */
>> +	if (want_multi && rdev->wiphy.max_sched_scan_reqs > 1) {
> 
> Why do the >1 check here? It probably doesn't really make a difference
> since only one can be running at a time, but it might be nicer - at
> least for debug in userspace - to have a real value for all multi
> scans?

I added the check, because drivers not supporting multi-scheduled scan
might not bother using the request id in event reporting.

Regards,
Arend

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

On Tue, 2017-02-14 at 14:33 +0100, Arend Van Spriel wrote:
> On 24-1-2017 10:40, Johannes Berg wrote:
> > > +	/* leave request id zero for legacy request
> > > +	 * or if driver does not support multi-scheduled scan
> > > +	 */
> > > +	if (want_multi && rdev->wiphy.max_sched_scan_reqs > 1) {
> > 
> > Why do the >1 check here? It probably doesn't really make a
> > difference
> > since only one can be running at a time, but it might be nicer - at
> > least for debug in userspace - to have a real value for all multi
> > scans?
> 
> I added the check, because drivers not supporting multi-scheduled
> scan might not bother using the request id in event reporting.

Ok, that's a good point.

johannes

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Arend Van Spriel]

On 14-2-2017 14:12, Johannes Berg wrote:
> On Tue, 2017-02-14 at 14:07 +0100, Arend Van Spriel wrote:
> 
>>> No. But there was a size limit on how much older userspace could
>>> process before we did the splitting.
>>
>> I see. So basically adding stuff to (split_start == 0) is not wanted.
> 
> Correct.

Uhm. Now I am staring at the code there and wonder about following. Up
until (split_start == 7) I see:

		state->split_start++;
		if (state->split)
			break;

For the remaining cases the break is unconditional. Any idea how to
interpret that?
	
>> Just trying to get a clear requirement/rule here. Do we (still) know
>> the exact size limit?
> 
> No, I don't remember. It's pretty ancient versions of iw/wpa_s that
> weren't able to deal with it. There was also interaction with the
> allocations in the dump function in nl80211 code itself, but we made
> those pretty big already and userspace limits came into play iirc.

Thanks,
Arend

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

On Tue, 2017-02-14 at 21:09 +0100, Arend Van Spriel wrote:
> On 14-2-2017 14:12, Johannes Berg wrote:
> > On Tue, 2017-02-14 at 14:07 +0100, Arend Van Spriel wrote:
> > 
> > > > No. But there was a size limit on how much older userspace
> > > > could
> > > > process before we did the splitting.
> > > 
> > > I see. So basically adding stuff to (split_start == 0) is not
> > > wanted.
> > 
> > Correct.
> 
> Uhm. Now I am staring at the code there and wonder about following.
> Up until (split_start == 7) I see:
> 
> 		state->split_start++;
> 		if (state->split)
> 			break;
> 
> For the remaining cases the break is unconditional. Any idea how to
> interpret that?

Yeah, actually, adding stuff to anything where split_start < 7 is
therefore not wanted :-)

The thing is that we if no split is accepted by the userspace tool
(state->split is false) then we have to send everything in one big
message. This is everything until split_start == 7, I guess.

After that, there's only new stuff that such old userspace will be
unable to interpret anyway, so the break is then unconditional - old
userspace without split will never see those new capabilities, and new
userspace can deal with the split.

johannes

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Arend Van Spriel]

On 14-2-2017 21:11, Johannes Berg wrote:
> On Tue, 2017-02-14 at 21:09 +0100, Arend Van Spriel wrote:
>> On 14-2-2017 14:12, Johannes Berg wrote:
>>> On Tue, 2017-02-14 at 14:07 +0100, Arend Van Spriel wrote:
>>>
>>>>> No. But there was a size limit on how much older userspace
>>>>> could
>>>>> process before we did the splitting.
>>>>
>>>> I see. So basically adding stuff to (split_start == 0) is not
>>>> wanted.
>>>
>>> Correct.
>>
>> Uhm. Now I am staring at the code there and wonder about following.
>> Up until (split_start == 7) I see:
>>
>> 		state->split_start++;
>> 		if (state->split)
>> 			break;
>>
>> For the remaining cases the break is unconditional. Any idea how to
>> interpret that?
> 
> Yeah, actually, adding stuff to anything where split_start < 7 is
> therefore not wanted :-)
> 
> The thing is that we if no split is accepted by the userspace tool
> (state->split is false) then we have to send everything in one big
> message. This is everything until split_start == 7, I guess.
> 
> After that, there's only new stuff that such old userspace will be
> unable to interpret anyway, so the break is then unconditional - old
> userspace without split will never see those new capabilities, and new
> userspace can deal with the split.

Ok. Now I bumped into a nice comment which I should have seen earlier ;-)

		/*
		 * Any information below this point is only available to
		 * applications that can deal with it being split. This
		 * helps ensure that newly added capabilities don't break
		 * older tools by overrunning their buffers.
		 *
		 * We still increment split_start so that in the split
		 * case we'll continue with more data in the next round,
		 * but break unconditionally so unsplit data stops here.
		 */
		state->split_start++;
		break;
	case 9:

Oh, well. So actually the sched_scan_plan attributes in case 0 slipped
through the cracks.

Regards,
Arend

Re: [RFC V2 1/5] nl80211: allow multiple active scheduled scan requests

[Johannes Berg]

> Ok. Now I bumped into a nice comment which I should have seen earlier
> ;-)

:-)

> Oh, well. So actually the sched_scan_plan attributes in case 0
> slipped through the cracks.

Oh. I guess we can still fix some things, but then again the iw version
that added support for split dumps was 4 years ago ...

johannes