[PATCH 1/2] cfg80211: Add support for aborting an ongoing scan

[PATCH 1/2] cfg80211: Add support for aborting an ongoing scan

[Sunil Dutt]

From: Vidyullatha Kanchanapally <vkanchan@qti.qualcomm.com>

Implement new functionality for aborting an ongoing scan.

Add NL80211_CMD_ABORT_SCAN to the nl80211 interface. After
aborting the scan, driver shall provide the scan status by
calling cfg80211_scan_done().

Reviewed-by: Jouni Malinen <jouni@qca.qualcomm.com>
Signed-off-by: Vidyullatha Kanchanapally <vkanchan@qti.qualcomm.com>
Signed-off-by: Sunil Dutt <usdutt@qti.qualcomm.com>
---
 include/net/cfg80211.h       |  4 ++++
 include/uapi/linux/nl80211.h |  5 +++++
 net/wireless/nl80211.c       | 25 +++++++++++++++++++++++++
 net/wireless/rdev-ops.h      | 12 ++++++++++++
 net/wireless/trace.h         |  5 +++++
 5 files changed, 51 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 48155be..90afac5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2488,6 +2488,9 @@ struct cfg80211_qos_map {
  *	and returning to the base channel for communication with the AP.
  * @tdls_cancel_channel_switch: Stop channel-switching with a TDLS peer. Both
  *	peers must be on the base channel when the call completes.
+ *
+ * @abort_scan: Tell the driver to abort an ongoing scan. The driver shall
+ *	indicate the status of the scan through cfg80211_scan_done().
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2752,6 +2755,7 @@ struct cfg80211_ops {
 	void	(*tdls_cancel_channel_switch)(struct wiphy *wiphy,
 					      struct net_device *dev,
 					      const u8 *addr);
+	int     (*abort_scan)(struct wiphy *wiphy, struct net_device *dev);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1f0b4cf..1e045ba 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -820,6 +820,9 @@
  *	as an event to indicate changes for devices with wiphy-specific regdom
  *	management.
  *
+ * @NL80211_CMD_ABORT_SCAN: stop an ongoing scan. Returns -ENOENT if a scan is
+ *	not running.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1006,6 +1009,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_WIPHY_REG_CHANGE,
 
+	NL80211_CMD_ABORT_SCAN,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d693c9d..d7d5011 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10588,6 +10588,23 @@ static int nl80211_tdls_cancel_channel_switch(struct sk_buff *skb,
 	return 0;
 }
 
+static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+
+	if (!rdev->ops->abort_scan)
+		return -EOPNOTSUPP;
+
+	if (rdev->scan_msg)
+		return 0;
+
+	if (!rdev->scan_req)
+		return -ENOENT;
+
+	return rdev_abort_scan(rdev, dev);
+}
+
 #define NL80211_FLAG_NEED_WIPHY		0x01
 #define NL80211_FLAG_NEED_NETDEV	0x02
 #define NL80211_FLAG_NEED_RTNL		0x04
@@ -11406,6 +11423,14 @@ static const struct genl_ops nl80211_ops[] = {
 		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
 				  NL80211_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL80211_CMD_ABORT_SCAN,
+		.doit = nl80211_abort_scan,
+		.policy = nl80211_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+				  NL80211_FLAG_NEED_RTNL,
+	},
 };
 
 /* notification functions */
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index c23516d..4dd222b 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1020,4 +1020,16 @@ rdev_tdls_cancel_channel_switch(struct cfg80211_registered_device *rdev,
 	trace_rdev_return_void(&rdev->wiphy);
 }
 
+static inline int
+rdev_abort_scan(struct cfg80211_registered_device *rdev,
+		struct net_device *dev)
+{
+	int ret;
+
+	trace_rdev_abort_scan(&rdev->wiphy, dev);
+	ret = rdev->ops->abort_scan(&rdev->wiphy, dev);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
+
 #endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 0c392d3..ceb20eb 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2818,6 +2818,11 @@ TRACE_EVENT(cfg80211_stop_iface,
 		  WIPHY_PR_ARG, WDEV_PR_ARG)
 );
 
+DEFINE_EVENT(wiphy_netdev_evt, rdev_abort_scan,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev),
+	TP_ARGS(wiphy, netdev)
+);
+
 #endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
1.8.2.1

[PATCH 2/2] mac80211: Add support for aborting an ongoing scan

[Sunil Dutt]

From: Vidyullatha Kanchanapally <vkanchan@qti.qualcomm.com>

This commit adds implementation for abort scan in mac80211.

Reviewed-by: Jouni Malinen <jouni@qca.qualcomm.com>
Signed-off-by: Vidyullatha Kanchanapally <vkanchan@qti.qualcomm.com>
Signed-off-by: Sunil Dutt <usdutt@qti.qualcomm.com>
---
 net/mac80211/cfg.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 713cdbf..d4d78e1 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3797,6 +3797,16 @@ static int ieee80211_del_tx_ts(struct wiphy *wiphy, struct net_device *dev,
 	return -ENOENT;
 }
 
+static int ieee80211_abort_scan(struct wiphy *wiphy, struct net_device *dev)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = sdata->local;
+
+	/* Cancel the ongoing scan */
+	ieee80211_scan_cancel(local);
+	return 0;
+}
+
 const struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -3881,4 +3891,5 @@ const struct cfg80211_ops mac80211_config_ops = {
 	.set_ap_chanwidth = ieee80211_set_ap_chanwidth,
 	.add_tx_ts = ieee80211_add_tx_ts,
 	.del_tx_ts = ieee80211_del_tx_ts,
+	.abort_scan = ieee80211_abort_scan,
 };
-- 
1.8.2.1

Re: [PATCH 1/2] cfg80211: Add support for aborting an ongoing scan

[Johannes Berg]

>   *
> + * @NL80211_CMD_ABORT_SCAN: stop an ongoing scan. Returns -ENOENT if a scan is
> + *	not running.


I think we might need to indicate in nl80211 exported capabilities
whether or not this command is available?

The return code requirement should probably also be documented in
cfg80211.h, and the fact that you get a scan-done message should be
here as well.

Or is that not necessary - i.e. no relevant races are possible - and
the function return can be void?

johannes

Re: [PATCH 2/2] mac80211: Add support for aborting an ongoing scan

[Johannes Berg]

On Thu, 2015-10-29 at 13:16 +0530, Sunil Dutt wrote:
> From: Vidyullatha Kanchanapally <vkanchan@qti.qualcomm.com>
> 
> This commit adds implementation for abort scan in mac80211.
> 
> Reviewed-by: Jouni Malinen <jouni@qca.qualcomm.com>
> Signed-off-by: Vidyullatha Kanchanapally <vkanchan@qti.qualcomm.com>
> Signed-off-by: Sunil Dutt <usdutt@qti.qualcomm.com>
> ---
>  net/mac80211/cfg.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 713cdbf..d4d78e1 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -3797,6 +3797,16 @@ static int ieee80211_del_tx_ts(struct wiphy 
> *wiphy, struct net_device *dev,
>  	return -ENOENT;
>  }
>  
> +static int ieee80211_abort_scan(struct wiphy *wiphy, struct 
> net_device *dev)
> +{
> +	struct ieee80211_sub_if_data *sdata = 
> IEEE80211_DEV_TO_SUB_IF(dev);
> +	struct ieee80211_local *local = sdata->local;
> +
> +	/* Cancel the ongoing scan */
> +	ieee80211_scan_cancel(local);
> +	return 0;
> +}

You're not returning -ENOENT when there's no scan, which could be racy?
I'm not sure those races would be relevant, but you should think about
that and document it.

johannes

Re: [PATCH 1/2] cfg80211: Add support for aborting an ongoing scan

[Jouni Malinen]

On Fri, Oct 30, 2015 at 10:34:00AM +0100, Johannes Berg wrote:
> > + * @NL80211_CMD_ABORT_SCAN: stop an ongoing scan. Returns -ENOENT if a scan is
> > + *	not running.

> I think we might need to indicate in nl80211 exported capabilities
> whether or not this command is available?
> 
> The return code requirement should probably also be documented in
> cfg80211.h, and the fact that you get a scan-done message should be
> here as well.
> 
> Or is that not necessary - i.e. no relevant races are possible - and
> the function return can be void?

The currently identified use cases do not seem to care much about the
actual result of this command since there is going to be a wait for the
scan completed event anyway and the call is not even used unless there
is a known ongoing scan. As such, even the difference of a driver
supporting this command or not is not that significant taken into
account this is used only as an optimization to speed up the following
operation when a long scan operation was in progress. Things work fine
(though without that speed benefit) even if the driver does not
support this. Anyway, making cfg80211 advertise whether the new command
is available could obviously be done should someone come up with a use
case that depends on knowing that the optimization is available.

The wpa_supplicant patches I'm planning on pushing in as soon as the
nl80211 command ID gets assigned are here:
http://w1.fi/p/scan/

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 1/2] cfg80211: Add support for aborting an ongoing scan

[Johannes Berg]

On Fri, 2015-10-30 at 12:37 +0200, Jouni Malinen wrote:
> 
> The currently identified use cases do not seem to care much about the
> actual result of this command since there is going to be a wait for 
> the
> scan completed event anyway and the call is not even used unless 
> there
> is a known ongoing scan. As such, even the difference of a driver
> supporting this command or not is not that significant taken into
> account this is used only as an optimization to speed up the 
> following
> operation when a long scan operation was in progress. Things work 
> fine
> (though without that speed benefit) even if the driver does not
> support this. Anyway, making cfg80211 advertise whether the new 
> command
> is available could obviously be done should someone come up with a 
> use
> case that depends on knowing that the optimization is available.
> 

Fair point. In that case though, making the driver operation void would
make sense.

johannes