[RFC 1/3] mac80211: do not scan and monitor connection in parallel

[RFC 1/3] mac80211: do not scan and monitor connection in parallel

[Stanislaw Gruszka]

Before we send probes in connection monitoring we check if scan is not
pending. But we do that check without locking. Fix that and also do not
start scan if connection monitoring is in progress.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   35 +++++++++++++++++++++++------------
 net/mac80211/scan.c        |   29 ++++++++++++++++++++++++++++-
 net/mac80211/work.c        |    7 +------
 4 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 63fb0eb..a48d40f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1239,6 +1239,7 @@ int ieee80211_request_internal_scan(struct ieee80211_sub_if_data *sdata,
 int ieee80211_request_scan(struct ieee80211_sub_if_data *sdata,
 			   struct cfg80211_scan_request *req);
 void ieee80211_scan_cancel(struct ieee80211_local *local);
+void ieee80211_run_deferred_scan(struct ieee80211_local *local);
 ieee80211_rx_result
 ieee80211_scan_rx(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb);
 
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0df2237..0737278 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1496,19 +1496,24 @@ void ieee80211_sta_rx_notify(struct ieee80211_sub_if_data *sdata,
 static void ieee80211_reset_ap_probe(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+	struct ieee80211_local *local = sdata->local;
 
+	mutex_lock(&local->mtx);
 	if (!(ifmgd->flags & (IEEE80211_STA_BEACON_POLL |
-			      IEEE80211_STA_CONNECTION_POLL)))
-	    return;
+			      IEEE80211_STA_CONNECTION_POLL))) {
+		mutex_unlock(&local->mtx);
+		return;
+	}
 
 	ifmgd->flags &= ~(IEEE80211_STA_CONNECTION_POLL |
 			  IEEE80211_STA_BEACON_POLL);
-	mutex_lock(&sdata->local->iflist_mtx);
-	ieee80211_recalc_ps(sdata->local, -1);
-	mutex_unlock(&sdata->local->iflist_mtx);
+
+	mutex_lock(&local->iflist_mtx);
+	ieee80211_recalc_ps(local, -1);
+	mutex_unlock(&local->iflist_mtx);
 
 	if (sdata->local->hw.flags & IEEE80211_HW_CONNECTION_MONITOR)
-		return;
+		goto out;
 
 	/*
 	 * We've received a probe response, but are not sure whether
@@ -1520,6 +1525,9 @@ static void ieee80211_reset_ap_probe(struct ieee80211_sub_if_data *sdata)
 	mod_timer(&ifmgd->conn_mon_timer,
 		  round_jiffies_up(jiffies +
 				   IEEE80211_CONNECTION_IDLE_TIME));
+out:
+	ieee80211_run_deferred_scan(local);
+	mutex_unlock(&local->mtx);
 }
 
 void ieee80211_sta_tx_notify(struct ieee80211_sub_if_data *sdata,
@@ -1586,17 +1594,18 @@ static void ieee80211_mgd_probe_ap(struct ieee80211_sub_if_data *sdata,
 	if (!ieee80211_sdata_running(sdata))
 		return;
 
-	if (sdata->local->scanning)
-		return;
-
-	if (sdata->local->tmp_channel)
-		return;
-
 	mutex_lock(&ifmgd->mtx);
 
 	if (!ifmgd->associated)
 		goto out;
 
+	mutex_lock(&sdata->local->mtx);
+
+	if (sdata->local->tmp_channel || sdata->local->scanning) {
+		mutex_unlock(&sdata->local->mtx);
+		goto out;
+	}
+
 #ifdef CONFIG_MAC80211_VERBOSE_DEBUG
 	if (beacon && net_ratelimit())
 		printk(KERN_DEBUG "%s: detected beacon loss from AP "
@@ -1623,6 +1632,8 @@ static void ieee80211_mgd_probe_ap(struct ieee80211_sub_if_data *sdata,
 	else
 		ifmgd->flags |= IEEE80211_STA_CONNECTION_POLL;
 
+	mutex_unlock(&sdata->local->mtx);
+
 	if (already)
 		goto out;
 
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 33cd169..1b90f97 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -387,6 +387,33 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
 	return 0;
 }
 
+static bool ieee80211_can_scan(struct ieee80211_local *local,
+			       struct ieee80211_sub_if_data *sdata)
+{
+	if (!list_empty(&local->work_list))
+		return false;
+
+	if (sdata->vif.type == NL80211_IFTYPE_STATION &&
+	    sdata->u.mgd.flags & (IEEE80211_STA_BEACON_POLL |
+				  IEEE80211_STA_CONNECTION_POLL))
+		return false;
+
+	return true;
+}
+
+void ieee80211_run_deferred_scan(struct ieee80211_local *local)
+{
+	lockdep_assert_held(&local->mtx);
+
+	if (!local->scan_req || local->scanning)
+		return;
+
+	if (!ieee80211_can_scan(local, local->scan_sdata))
+		return;
+
+	ieee80211_queue_delayed_work(&local->hw, &local->scan_work,
+				     round_jiffies_relative(0));
+}
 
 static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
 				  struct cfg80211_scan_request *req)
@@ -399,7 +426,7 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
 	if (local->scan_req)
 		return -EBUSY;
 
-	if (!list_empty(&local->work_list)) {
+	if (!ieee80211_can_scan(local, sdata)) {
 		/* wait for the work to finish/time out */
 		local->scan_req = req;
 		local->scan_sdata = sdata;
diff --git a/net/mac80211/work.c b/net/mac80211/work.c
index c6e230e..1f74af3 100644
--- a/net/mac80211/work.c
+++ b/net/mac80211/work.c
@@ -226,13 +226,8 @@ static void ieee80211_work_work(struct work_struct *work)
 		run_again(local, jiffies + HZ/2);
 	}
 
-	if (list_empty(&local->work_list) && local->scan_req &&
-	    !local->scanning)
-		ieee80211_queue_delayed_work(&local->hw,
-					     &local->scan_work,
-					     round_jiffies_relative(0));
-
 	ieee80211_recalc_idle(local);
+	ieee80211_run_deferred_scan(local);
 
 	mutex_unlock(&local->mtx);
 
-- 
1.7.1

[RFC 2/3] mac80211: protect ->scanning by mutex in ieee80211_work_work()

[Stanislaw Gruszka]

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/work.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/work.c b/net/mac80211/work.c
index 1f74af3..b2650a9 100644
--- a/net/mac80211/work.c
+++ b/net/mac80211/work.c
@@ -122,9 +122,6 @@ static void ieee80211_work_work(struct work_struct *work)
 	enum work_action rma;
 	bool remain_off_channel = false;
 
-	if (local->scanning)
-		return;
-
 	/*
 	 * ieee80211_queue_work() should have picked up most cases,
 	 * here we'll pick the rest.
@@ -134,6 +131,11 @@ static void ieee80211_work_work(struct work_struct *work)
 
 	mutex_lock(&local->mtx);
 
+	if (local->scanning) {
+		mutex_unlock(&local->mtx);
+		return;
+	}
+
 	ieee80211_recalc_idle(local);
 
 	list_for_each_entry_safe(wk, tmp, &local->work_list, list) {
-- 
1.7.1

[RFC 3/3] mac80211: synchronize channel switch and scan

[Stanislaw Gruszka]

Do not perform actual channel switch and scan is parallel. Devices
that implement channel switch in hardware are responsible to do
synchronization by itself if needed.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   19 ++++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a48d40f..98b0d6c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -378,6 +378,7 @@ enum ieee80211_sta_flags {
 	IEEE80211_STA_UAPSD_ENABLED	= BIT(7),
 	IEEE80211_STA_NULLFUNC_ACKED	= BIT(8),
 	IEEE80211_STA_RESET_SIGNAL_AVE	= BIT(9),
+	IEEE80211_STA_CSA_DEFERRED	= BIT(10),
 };
 
 struct ieee80211_mgd_auth_data {
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0737278..ebc60a2 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -726,6 +726,13 @@ static void ieee80211_chswitch_work(struct work_struct *work)
 	if (!ifmgd->associated)
 		goto out;
 
+	mutex_lock(&sdata->local->mtx);
+	if (sdata->local->scanning) {
+		ifmgd->flags |= IEEE80211_STA_CSA_DEFERRED;
+		mutex_unlock(&sdata->local->mtx);
+		goto out_unlock;
+	}
+
 	sdata->local->oper_channel = sdata->local->csa_channel;
 	if (!sdata->local->ops->channel_switch) {
 		/* call "hw_config" only if doing sw channel switch */
@@ -741,8 +748,11 @@ static void ieee80211_chswitch_work(struct work_struct *work)
 
 	ieee80211_wake_queues_by_reason(&sdata->local->hw,
 					IEEE80211_QUEUE_STOP_REASON_CSA);
+
+	mutex_unlock(&sdata->local->mtx);
  out:
 	ifmgd->flags &= ~IEEE80211_STA_CSA_RECEIVED;
+ out_unlock:
 	mutex_unlock(&ifmgd->mtx);
 }
 
@@ -800,9 +810,6 @@ void ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 	if (!ifmgd->associated)
 		return;
 
-	if (sdata->local->scanning)
-		return;
-
 	/* Disregard subsequent beacons if we are already running a timer
 	   processing a CSA */
 
@@ -3037,12 +3044,18 @@ void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata = local->scan_sdata;
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
 	/* Restart STA timers */
 	rcu_read_lock();
 	list_for_each_entry_rcu(sdata, &local->interfaces, list)
 		ieee80211_restart_sta_timer(sdata);
 	rcu_read_unlock();
+
+	if (ifmgd->flags & IEEE80211_STA_CSA_DEFERRED) {
+		ifmgd->flags &= ~IEEE80211_STA_CSA_DEFERRED;
+		ieee80211_queue_work(&sdata->local->hw, &ifmgd->chswitch_work);
+	}
 }
 
 int ieee80211_max_network_latency(struct notifier_block *nb,
-- 
1.7.1

Re: [RFC 1/3] mac80211: do not scan and monitor connection in parallel

[Johannes Berg]

On Fri, 2012-03-16 at 13:02 +0100, Stanislaw Gruszka wrote:
> Before we send probes in connection monitoring we check if scan is not
> pending. But we do that check without locking. Fix that and also do not
> start scan if connection monitoring is in progress.

Wouldn't it be easier to just abort the check instead of deferring the
scan?

johannes

Re: [RFC 2/3] mac80211: protect ->scanning by mutex in ieee80211_work_work()

[Johannes Berg]

I don't really see the point in this. It seems that this doesn't really
help, you'd actually have to protect all the work stuff here with the
mutex so the scanning can also be excluded or so?

johannes

On Fri, 2012-03-16 at 13:02 +0100, Stanislaw Gruszka wrote:
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  net/mac80211/work.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mac80211/work.c b/net/mac80211/work.c
> index 1f74af3..b2650a9 100644
> --- a/net/mac80211/work.c
> +++ b/net/mac80211/work.c
> @@ -122,9 +122,6 @@ static void ieee80211_work_work(struct work_struct *work)
>  	enum work_action rma;
>  	bool remain_off_channel = false;
>  
> -	if (local->scanning)
> -		return;
> -
>  	/*
>  	 * ieee80211_queue_work() should have picked up most cases,
>  	 * here we'll pick the rest.
> @@ -134,6 +131,11 @@ static void ieee80211_work_work(struct work_struct *work)
>  
>  	mutex_lock(&local->mtx);
>  
> +	if (local->scanning) {
> +		mutex_unlock(&local->mtx);
> +		return;
> +	}
> +
>  	ieee80211_recalc_idle(local);
>  
>  	list_for_each_entry_safe(wk, tmp, &local->work_list, list) {

Re: [RFC 1/3] mac80211: do not scan and monitor connection in parallel

[Stanislaw Gruszka]

On Fri, Mar 16, 2012 at 01:52:58PM +0100, Johannes Berg wrote:
> On Fri, 2012-03-16 at 13:02 +0100, Stanislaw Gruszka wrote:
> > Before we send probes in connection monitoring we check if scan is not
> > pending. But we do that check without locking. Fix that and also do not
> > start scan if connection monitoring is in progress.
> 
> Wouldn't it be easier to just abort the check instead of deferring the
> scan?

It would be easier. However we already defer scan on
!list_empty(&local->work_list) case. Also random failures of
scan request can not be well seen from user-space perspective.

Stanislaw

Re: [RFC 2/3] mac80211: protect ->scanning by mutex in ieee80211_work_work()

[Stanislaw Gruszka]

On Fri, Mar 16, 2012 at 01:54:13PM +0100, Johannes Berg wrote:
> I don't really see the point in this. It seems that this doesn't really
> help, you'd actually have to protect all the work stuff here with the
> mutex so the scanning can also be excluded or so?

You mean some other work stuff than ieee80211_work_work() ?

We have almost all ieee80211_work_work() code protected by local->mtx
already, I just moved local->scanning check after the mutex_lock.

Stanislaw

Re: [RFC 1/3] mac80211: do not scan and monitor connection in parallel

[Johannes Berg]

On Fri, 2012-03-16 at 14:13 +0100, Stanislaw Gruszka wrote:
> On Fri, Mar 16, 2012 at 01:52:58PM +0100, Johannes Berg wrote:
> > On Fri, 2012-03-16 at 13:02 +0100, Stanislaw Gruszka wrote:
> > > Before we send probes in connection monitoring we check if scan is not
> > > pending. But we do that check without locking. Fix that and also do not
> > > start scan if connection monitoring is in progress.
> > 
> > Wouldn't it be easier to just abort the check instead of deferring the
> > scan?
> 
> It would be easier. However we already defer scan on
> !list_empty(&local->work_list) case. Also random failures of
> scan request can not be well seen from user-space perspective.

Ah, I didn't mean aborting the scan request, but rather the connection
monitor check. But yeah, I guess you're right about already deferring.

johannes

Re: [RFC 2/3] mac80211: protect ->scanning by mutex in ieee80211_work_work()

[Johannes Berg]

On Fri, 2012-03-16 at 14:16 +0100, Stanislaw Gruszka wrote:
> On Fri, Mar 16, 2012 at 01:54:13PM +0100, Johannes Berg wrote:
> > I don't really see the point in this. It seems that this doesn't really
> > help, you'd actually have to protect all the work stuff here with the
> > mutex so the scanning can also be excluded or so?
> 
> You mean some other work stuff than ieee80211_work_work() ?
> 
> We have almost all ieee80211_work_work() code protected by local->mtx
> already, I just moved local->scanning check after the mutex_lock.

Oh, ok. I thought it wasn't really protected, I guess then this makes
some sense.

johannes