Patchwork Fix scanning state when sched_scan is stopped explicitly

login
register
mail settings
Submitter Dmitry Shmidt
Date April 9, 2014, 10:48 p.m.
Message ID <20140409225048.F13EA140263@ushik.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/337944/
State New
Headers show

Comments

Dmitry Shmidt - April 9, 2014, 10:48 p.m.
Change-Id: I6deca387f7c64e4125e85ecfd585e1cff6931ab1
Signed-off-by: Dmitry Shmidt <dimitrysh@google.com>
---
 wpa_supplicant/scan.c | 3 +++
 1 file changed, 3 insertions(+)
Jouni Malinen - April 11, 2014, 4:09 p.m.
On Wed, Apr 09, 2014 at 03:48:07PM -0700, Dmitry Shmidt wrote:
> --- a/wpa_supplicant/scan.c
> +++ b/wpa_supplicant/scan.c
> @@ -261,6 +261,9 @@ int wpa_supplicant_stop_sched_scan(struct wpa_supplicant *wpa_s)
>  		wpa_dbg(wpa_s, MSG_DEBUG, "stopping sched_scan failed!");
>  		/* TODO: what to do if stopping fails? */
>  		return -1;
> +	} else {
> +		wpa_s->sched_scanning = 0;
> +		wpa_supplicant_notify_scanning(wpa_s, 0);
>  	}

It looks like the current implementation assumes that there will be a
driver event to indicate that sched_scan was stopped and
wpa_s->sched_scanning is cleared to 0 when that event is processed. Is
there a driver that does not end up indicating that event?

I would assume there was a reason for postponing, but then again, I now
remember that there were open questions on a related area.. I think
there was a request to revert commit
cf70d2981df1be7005fb90c2532e737ab39bc119 and handle some of the changes
in mac80211 to make operations synchronous. I guess we should get open
item resolved as well.. I'm not completely sure whether these two
changes have any dependencies to each other, though.
Dmitry Shmidt - April 11, 2014, 4:51 p.m.
On Fri, Apr 11, 2014 at 9:09 AM, Jouni Malinen <j@w1.fi> wrote:
> On Wed, Apr 09, 2014 at 03:48:07PM -0700, Dmitry Shmidt wrote:
>> --- a/wpa_supplicant/scan.c
>> +++ b/wpa_supplicant/scan.c
>> @@ -261,6 +261,9 @@ int wpa_supplicant_stop_sched_scan(struct wpa_supplicant *wpa_s)
>>               wpa_dbg(wpa_s, MSG_DEBUG, "stopping sched_scan failed!");
>>               /* TODO: what to do if stopping fails? */
>>               return -1;
>> +     } else {
>> +             wpa_s->sched_scanning = 0;
>> +             wpa_supplicant_notify_scanning(wpa_s, 0);
>>       }
>
> It looks like the current implementation assumes that there will be a
> driver event to indicate that sched_scan was stopped and
> wpa_s->sched_scanning is cleared to 0 when that event is processed. Is
> there a driver that does not end up indicating that event?

Indeed, driver is not calling cfg80211_sched_scan_stopped() to send
NL80211_CMD_SCHED_SCAN_STOPPED message.
It is calling cfg80211_scan_done(aborted) to send
NL80211_CMD_SCAN_ABORTED that is doing EVENT_SCAN_RESULTS
and finally cleans wpa_s->scanning state.

However, I tried to resolve next problem that is not disappearing even if we
send message above from the driver:

wpas_stop_pno()  {
     wpa_supplicant_stop_sched_scan()
      if (wpa_s->wpa_state == WPA_SCANNING)
                wpa_supplicant_req_scan(wpa_s, 0, 0);  <<< usual scan
}

wpa_supplicant_scan() {
...
        if (wpa_s->scanning) {
                ...
                wpa_supplicant_req_scan(wpa_s, 1, 0);  <<< postponing
scan for 1 sec
                return;
        }
...
}

Scan is postponed for 1 sec and I just tried to eliminate it. It is
slightly different
from problem below where Raja tried to sync between two sched scan requests.

>
> I would assume there was a reason for postponing, but then again, I now
> remember that there were open questions on a related area.. I think
> there was a request to revert commit
> cf70d2981df1be7005fb90c2532e737ab39bc119 and handle some of the changes
> in mac80211 to make operations synchronous. I guess we should get open
> item resolved as well.. I'm not completely sure whether these two
> changes have any dependencies to each other, though.

I think Raja's change is related to "coordination" between sched scan
by wpa_supplicant
and explicit one.

>
> --
> Jouni Malinen                                            PGP id EFC895FA

Patch

diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
index 1d4e6e5..af34887 100644
--- a/wpa_supplicant/scan.c
+++ b/wpa_supplicant/scan.c
@@ -261,6 +261,9 @@  int wpa_supplicant_stop_sched_scan(struct wpa_supplicant *wpa_s)
 		wpa_dbg(wpa_s, MSG_DEBUG, "stopping sched_scan failed!");
 		/* TODO: what to do if stopping fails? */
 		return -1;
+	} else {
+		wpa_s->sched_scanning = 0;
+		wpa_supplicant_notify_scanning(wpa_s, 0);
 	}
 
 	return ret;