iwm: always return scan results

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

iwm: always return scan results

Stefan Sperling-5
I found a way to reproduce the "hangs" people have been reporting
during 'ifconfig iwm0 scan'. The problem is actually quite trivial.

To reproduce:

  1) configure a network ID that does not exist around you
  2) put the interface up
  3) keep running ifconfig iwm0 scan; eventually ifconfig won't
     show results but just time out; and observe how no frames
     are received with: tcpdump -i iwm0 -y IEEE802_11_RADIO

The problem is that iwm_newstate does not restart scans when a SCAN->SCAN
transition happens and the firmware scan command is no longer busy.

SCAN->SCAN transitions will occur while we are looking for an access point,
which we always do while the interface is UP and not associated.

ifconfig waits until ieee80211_end_scan() is called (or until a fixed
timeout), but since the driver never restarts a scan during SCAN->SCAN
transitions, ieee80211_end_scan() is not called.

So this is what's been going on when people opened there laptops after
moving somwhere else, and tried to scan while the interface was still UP.

More precisely, SCAN->SCAN transitions occur in two situations:

  1) the net80211 stack wants to switch to the next channel during
     the scan; This is a no-op in iwm because the firwmare scans all
     channels at once, and this transition was correctly implemented.

  2) if we don't find an AP and restart the scanning loop; this is
     the case which I missed and is fixed by the diff below.

The only way to recover from this situation is to down/up the interface
or to change interface configuration (e.g. change network ID).

It's surprising that this took so much time to get noticed -- or perhaps
people just got used to running 'ifconfig iwm0 down' before scanning?

Index: if_iwm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.217
diff -u -p -r1.217 if_iwm.c
--- if_iwm.c 26 Oct 2017 15:00:28 -0000 1.217
+++ if_iwm.c 7 Dec 2017 12:39:13 -0000
@@ -5986,17 +5986,21 @@ iwm_newstate_task(void *psc)
     ieee80211_state_name[ostate],
     ieee80211_state_name[nstate]));
 
- if (nstate == ostate || (sc->sc_flags & IWM_FLAG_SHUTDOWN)) {
- /* No-op state change or iwm_stop() is waiting for us. */
+ if (sc->sc_flags & IWM_FLAG_SHUTDOWN) {
+ /* iwm_stop() is waiting for us. */
  refcnt_rele_wake(&sc->task_refs);
  splx(s);
  return;
  }
 
- if (ostate == IEEE80211_S_SCAN)
- iwm_led_blink_stop(sc);
+ if (ostate == IEEE80211_S_SCAN) {
+ if (nstate != ostate)
+ iwm_led_blink_stop(sc);
+ else if ((sc->sc_flags & IWM_FLAG_SCANNING) == 0)
+ goto next_scan;
+ }
 
- if (nstate < ostate) {
+ if (nstate <= ostate) {
  switch (ostate) {
  case IEEE80211_S_RUN:
  err = iwm_run_stop(sc);
@@ -6035,6 +6039,7 @@ iwm_newstate_task(void *psc)
  break;
 
  case IEEE80211_S_SCAN:
+next_scan:
  err = iwm_scan(sc);
  if (err)
  break;

Reply | Threaded
Open this post in threaded view
|

Re: iwm: always return scan results

Stefan Sperling-5
On Thu, Dec 07, 2017 at 02:10:02PM +0100, Stefan Sperling wrote:

> I found a way to reproduce the "hangs" people have been reporting
> during 'ifconfig iwm0 scan'. The problem is actually quite trivial.
>
> To reproduce:
>
>   1) configure a network ID that does not exist around you
>   2) put the interface up
>   3) keep running ifconfig iwm0 scan; eventually ifconfig won't
>      show results but just time out; and observe how no frames
>      are received with: tcpdump -i iwm0 -y IEEE802_11_RADIO
>
> The problem is that iwm_newstate does not restart scans when a SCAN->SCAN
> transition happens and the firmware scan command is no longer busy.

Fixed diff: The previous diff accidentally started a new scan even
while the firmware was still busy scanning.

Index: if_iwm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.217
diff -u -p -r1.217 if_iwm.c
--- if_iwm.c 26 Oct 2017 15:00:28 -0000 1.217
+++ if_iwm.c 7 Dec 2017 13:20:57 -0000
@@ -5986,17 +5986,27 @@ iwm_newstate_task(void *psc)
     ieee80211_state_name[ostate],
     ieee80211_state_name[nstate]));
 
- if (nstate == ostate || (sc->sc_flags & IWM_FLAG_SHUTDOWN)) {
- /* No-op state change or iwm_stop() is waiting for us. */
+ if (sc->sc_flags & IWM_FLAG_SHUTDOWN) {
+ /* iwm_stop() is waiting for us. */
  refcnt_rele_wake(&sc->task_refs);
  splx(s);
  return;
  }
 
- if (ostate == IEEE80211_S_SCAN)
- iwm_led_blink_stop(sc);
+ if (ostate == IEEE80211_S_SCAN) {
+ if (nstate == ostate) {
+ if (sc->sc_flags & IWM_FLAG_SCANNING) {
+ refcnt_rele_wake(&sc->task_refs);
+ splx(s);
+ return;
+ }
+ /* Firmware is no longer scanning. Do another scan. */
+ goto next_scan;
+ } else
+ iwm_led_blink_stop(sc);
+ }
 
- if (nstate < ostate) {
+ if (nstate <= ostate) {
  switch (ostate) {
  case IEEE80211_S_RUN:
  err = iwm_run_stop(sc);
@@ -6035,6 +6045,7 @@ iwm_newstate_task(void *psc)
  break;
 
  case IEEE80211_S_SCAN:
+next_scan:
  err = iwm_scan(sc);
  if (err)
  break;