Patchwork [3/3] wpa_supplicant: cancel scan work when starting a connection

login
register
mail settings
Submitter Ilan Peer
Date March 27, 2014, 6:58 a.m.
Message ID <1395903514-843-7-git-send-email-ilan.peer@intel.com>
Download mbox | patch
Permalink /patch/334231/
State Changes Requested
Headers show

Comments

Ilan Peer - March 27, 2014, 6:58 a.m.
Cancel a pending scan work if a connection is started, as otherwise
the connection flow would be blocked until the scan is complete.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
---
 wpa_supplicant/wpa_supplicant.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
Jouni Malinen - March 28, 2014, 5:42 p.m.
On Thu, Mar 27, 2014 at 08:58:34AM +0200, Ilan Peer wrote:
> Cancel a pending scan work if a connection is started, as otherwise
> the connection flow would be blocked until the scan is complete.

This looks a bit problematic since this seems to be able to drop a
special scan request that was issued through the control interface. Why
is this needed? Isn't it enough for sme_authenticate() to add a high
priority sme-connect radio work that ends up getting added to the
beginning of the queue, i.e., to be processed before the pending scan
radio work if one was there?
Ilan Peer - March 30, 2014, 6:09 a.m.
> -----Original Message-----
> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
> bounces@lists.shmoo.com] On Behalf Of Jouni Malinen
> Sent: Friday, March 28, 2014 20:43
> To: hostap@lists.shmoo.com
> Subject: Re: [PATCH 3/3] wpa_supplicant: cancel scan work when starting a
> connection
> 
> On Thu, Mar 27, 2014 at 08:58:34AM +0200, Ilan Peer wrote:
> > Cancel a pending scan work if a connection is started, as otherwise
> > the connection flow would be blocked until the scan is complete.
> 
> This looks a bit problematic since this seems to be able to drop a special scan
> request that was issued through the control interface. Why is this needed?
> Isn't it enough for sme_authenticate() to add a high priority sme-connect
> radio work that ends up getting added to the beginning of the queue, i.e., to
> be processed before the pending scan radio work if one was there?

Agree ... missed this point.

Do I need to do something special to abandon it?

Thanks,

Ilan.
Jouni Malinen - Feb. 1, 2015, 3:16 p.m.
On Sun, Mar 30, 2014 at 06:09:12AM +0000, Peer, Ilan wrote:
> > On Thu, Mar 27, 2014 at 08:58:34AM +0200, Ilan Peer wrote:
> > > Cancel a pending scan work if a connection is started, as otherwise
> > > the connection flow would be blocked until the scan is complete.
> > 
> > This looks a bit problematic since this seems to be able to drop a special scan
> > request that was issued through the control interface. Why is this needed?
> > Isn't it enough for sme_authenticate() to add a high priority sme-connect
> > radio work that ends up getting added to the beginning of the queue, i.e., to
> > be processed before the pending scan radio work if one was there?
> 
> Agree ... missed this point.
> 
> Do I need to do something special to abandon it?

I had left this pending to try to figure out what the proposed change
was trying to fix (that blocking part). I still don't see what the issue
here would be. If you think this is still needed, please provide a
wpa_supplicant debug log showing a blocking case where this change would
have helped.
Ilan Peer - Feb. 2, 2015, 8:41 a.m.
> -----Original Message-----
> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
> bounces@lists.shmoo.com] On Behalf Of Jouni Malinen
> Sent: Sunday, February 01, 2015 17:17
> To: hostap@lists.shmoo.com
> Subject: Re: [PATCH 3/3] wpa_supplicant: cancel scan work when starting a
> connection
> 
> On Sun, Mar 30, 2014 at 06:09:12AM +0000, Peer, Ilan wrote:
> > > On Thu, Mar 27, 2014 at 08:58:34AM +0200, Ilan Peer wrote:
> > > > Cancel a pending scan work if a connection is started, as
> > > > otherwise the connection flow would be blocked until the scan is
> complete.
> > >
> > > This looks a bit problematic since this seems to be able to drop a

> > > special scan request that was issued through the control interface. Why is
> this needed?
> > > Isn't it enough for sme_authenticate() to add a high priority
> > > sme-connect radio work that ends up getting added to the beginning
> > > of the queue, i.e., to be processed before the pending scan radio work if
> one was there?
> >
> > Agree ... missed this point.
> >
> > Do I need to do something special to abandon it?
> 
> I had left this pending to try to figure out what the proposed change was
> trying to fix (that blocking part). I still don't see what the issue here would be.
> If you think this is still needed, please provide a wpa_supplicant debug log
> showing a blocking case where this change would have helped.
> 

I do not fully remember what was the original motivation, but as far as the connection establishment is concerned, I think that this change is not required. However, it would be beneficial to cancel the scan work to ease post connection actions such dhcp and other higher layer protocol activities. 

Regard,

Ilan.

Patch

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 14b084a..a3000bc 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -1406,6 +1406,16 @@  void wpa_supplicant_associate(struct wpa_supplicant *wpa_s,
 
 	if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_SME) &&
 	    ssid->mode == IEEE80211_MODE_INFRA) {
+		if (radio_work_pending(wpa_s, "scan")) {
+			/*
+			 * Starting a connection flow. Remove any scan work that
+			 * might delay the connection.
+			 */
+			wpa_dbg(wpa_s, MSG_DEBUG,
+				"Remove previous pending scan work");
+			radio_remove_works(wpa_s, "scan", 0);
+		}
+
 		sme_authenticate(wpa_s, bss, ssid);
 		return;
 	}