[PATCH] mac80211: stop scan before connection

[PATCH] mac80211: stop scan before connection

[Emmanuel Grumbach]

From: David Spinadel <david.spinadel@intel.com>

Stop scan before authentication or association to make sure
that nothing interferes with connection flow.

Currently mac80211 defers RX auth and assoc packets (among other ones)
until after the scan is complete, so auth during scan is likely to fail
if scan took too much time.

Signed-off-by: David Spinadel <david.spinadel@intel.com>
Reviewed-by: Luciano Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index d77150d..3f3ac3e 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -4458,6 +4458,10 @@ static int ieee80211_prep_connection(struct ieee80211_sub_if_data *sdata,
 	} else
 		WARN_ON_ONCE(!ether_addr_equal(ifmgd->bssid, cbss->bssid));
 
+	/* Cancel scan to ensure that nothing interferes with connection */
+	if (local->scanning)
+		ieee80211_scan_cancel(local);
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH] mac80211: stop scan before connection

[Eliad Peller]

On Tue, Mar 17, 2015 at 7:58 PM, Emmanuel Grumbach
<emmanuel.grumbach@intel.com> wrote:
> From: David Spinadel <david.spinadel@intel.com>
>
> Stop scan before authentication or association to make sure
> that nothing interferes with connection flow.
>
this makes sense

> Currently mac80211 defers RX auth and assoc packets (among other ones)
> until after the scan is complete, so auth during scan is likely to fail
> if scan took too much time.
>
but i don't think deferring the frame handling makes sense in case of
hw_scan (there was a similar case with BA request handling).
i should have such (one-liner) patch pending somewhere...

Eliad.

Re: [PATCH] mac80211: stop scan before connection

[Johannes Berg]

On Wed, 2015-03-18 at 11:36 +0200, Eliad Peller wrote:
> On Tue, Mar 17, 2015 at 7:58 PM, Emmanuel Grumbach
> <emmanuel.grumbach@intel.com> wrote:
> > From: David Spinadel <david.spinadel@intel.com>
> >
> > Stop scan before authentication or association to make sure
> > that nothing interferes with connection flow.
> >
> this makes sense
> 
> > Currently mac80211 defers RX auth and assoc packets (among other ones)
> > until after the scan is complete, so auth during scan is likely to fail
> > if scan took too much time.
> >
> but i don't think deferring the frame handling makes sense in case of
> hw_scan (there was a similar case with BA request handling).
> i should have such (one-liner) patch pending somewhere...

We did consider that, but ultimately I think it's not desirable.

The aggregation case is different, but during auth/assoc the relevant
frames cannot be buffered by the AP. So unless you have a hardware
mechanism that makes the device not continue the scan for say 30-50ms
after the auth/assoc req frame went out, the connection process will be
very unreliable.

So in a sense you could say deferring the handling doesn't make sense,
but I'd still argue you shouldn't start this process while you're in the
middle of scanning, hence this patch.

We may have to do more exclusions (e.g. don't start a scan while
authenticating or associating) here though, and ultimately not
*processing* the frames will be irrelevant as they shouldn't be coming
in anyway.

johannes

Re: [PATCH] mac80211: stop scan before connection

[Eliad Peller]

On Wed, Mar 18, 2015 at 11:46 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2015-03-18 at 11:36 +0200, Eliad Peller wrote:
>> On Tue, Mar 17, 2015 at 7:58 PM, Emmanuel Grumbach
>> <emmanuel.grumbach@intel.com> wrote:
>> > From: David Spinadel <david.spinadel@intel.com>
>> >
>> > Stop scan before authentication or association to make sure
>> > that nothing interferes with connection flow.
>> >
>> this makes sense
>>
>> > Currently mac80211 defers RX auth and assoc packets (among other ones)
>> > until after the scan is complete, so auth during scan is likely to fail
>> > if scan took too much time.
>> >
>> but i don't think deferring the frame handling makes sense in case of
>> hw_scan (there was a similar case with BA request handling).
>> i should have such (one-liner) patch pending somewhere...
>
> We did consider that, but ultimately I think it's not desirable.
>
> The aggregation case is different, but during auth/assoc the relevant
> frames cannot be buffered by the AP. So unless you have a hardware
> mechanism that makes the device not continue the scan for say 30-50ms
> after the auth/assoc req frame went out, the connection process will be
> very unreliable.
>
i'm not sure. the AP will probably have some retransmissions, so the
station should have good chances to get the frames (assuming the
hardware will give this interface some airtime during the scan).

> So in a sense you could say deferring the handling doesn't make sense,
> but I'd still argue you shouldn't start this process while you're in the
> middle of scanning, hence this patch.
>
i agree the patch makes sense anyway, i just noted that mac80211
behavior seems wrong.
you should avoid scanning while connecting in first place, not defer
the handling.

btw, note that both this patch and ieee80211_iface_work() don't count
for the sched scan case, which is basically the same.

> We may have to do more exclusions (e.g. don't start a scan while
> authenticating or associating) here though, and ultimately not
> *processing* the frames will be irrelevant as they shouldn't be coming
> in anyway.
that's true for auth/assoc frames. but some other frames (e.g. deauth)
can come anytime, and i don't see why we should defer processing in
this case.

Eliad.

Re: [PATCH] mac80211: stop scan before connection

[Johannes Berg]

On Wed, 2015-03-18 at 12:11 +0200, Eliad Peller wrote:

> i'm not sure. the AP will probably have some retransmissions, 

It hopefully will, but they'll be real retransmissions (with retry bit),
usually occurring within a very short period of time.

> so the
> station should have good chances to get the frames (assuming the
> hardware will give this interface some airtime during the scan).

So the normal dwell time during even an active scan (say 30ms) will mean
that all of those retries will not be heard once you miss the first one.

> > So in a sense you could say deferring the handling doesn't make sense,
> > but I'd still argue you shouldn't start this process while you're in the
> > middle of scanning, hence this patch.
> >
> i agree the patch makes sense anyway, i just noted that mac80211
> behavior seems wrong.
> you should avoid scanning while connecting in first place, not defer
> the handling.

Yeah, agree.

> btw, note that both this patch and ieee80211_iface_work() don't count
> for the sched scan case, which is basically the same.

True, except we can't really cover that case since we don't even know
when we're really scanning while sched scan is enabled.

I'd expect drivers to either abort the scheduled scan in that case, or
protected the authentication/association in some way (like iwlwifi does,
for example, by giving a few hundred ms of guaranteed channel time in
the mgd_prepare_tx method)

> > We may have to do more exclusions (e.g. don't start a scan while
> > authenticating or associating) here though, and ultimately not
> > *processing* the frames will be irrelevant as they shouldn't be coming
> > in anyway.
> that's true for auth/assoc frames. but some other frames (e.g. deauth)
> can come anytime, and i don't see why we should defer processing in
> this case.

Yeah, true. This seems like something we should take a closer look at,
think about the design more and document the expectations better.

Still, I think this patch is one step along that way.

johannes

Re: [PATCH] mac80211: stop scan before connection

[Johannes Berg]

On Tue, 2015-03-17 at 19:58 +0200, Emmanuel Grumbach wrote:
> From: David Spinadel <david.spinadel@intel.com>
> 
> Stop scan before authentication or association to make sure
> that nothing interferes with connection flow.
> 
> Currently mac80211 defers RX auth and assoc packets (among other ones)
> until after the scan is complete, so auth during scan is likely to fail
> if scan took too much time.

Applied.

johannes