Re: When BackgroundScanning = false, connman's Scan() dbus method is broken

On 08/09/2017 04:24 PM, Jonah Petri wrote: > > Hi Daniel, > > Thanks for having a look. Comments below. > >> On Aug 9, 2017, at 6:35 AM, Daniel Wagner <wagi(a)monom.org&gt; wrote: >> >> Hi >> >> I added Jose to the list. He did some larger work in the recent days and >> might have an idea where the problem is. >> >> On 08/08/2017 05:10 PM, Jonah Petri wrote: >>> >>> Hello, >>> I want to report an issue with connman. We set BackgroundScanning = >>> false in main.conf. However, I have found that this also causes connman to >>> give an erroneous empty response to the Scan() dbus call, potentially >>> permanently disabling the device. >>> The key preconditions seem to be: >>> 1) wpa_supplicant reports a max_ssids > 1 >>> 2) the computer must have been previously associated to a SSID which is >>> no longer visible >> >> >> 'previously asscioated' is from a previous run? ConnMan has been >> restarted but not wpa_supplicant? Or the whole box has been restarted? > > > In my case, the whole box has been restarted. But, from code examination, > if get_latest_connections() doesn't return empty, then the #2 precondition > is satisfied. I'm not sure all the ways that could become the case, but at > least the following works: associating with a network, unplugging the > device, unplugging the AP, then booting the device. Okay, the state (already connected to some network) is preserved somewhere persistently. >>> >>> 3) as above, BackgroundScanning must be set to false >>> This causes the logic of wifi.c:wifi_scan() to fail. In particular: >>> 1) wifi_scan_simple is not used, due to the above preconditions. >>> 2) connman requests an active scan via wpa_supplicant for the ssids >>> returned by get_latest_connections. >>> 3) connman does not request a followup passive scan, as >>> BackgroundScanning is turned off, which causes start_autoscan() to exit >>> early. >> >> >> autoscan is supposed to emulate the background scanning in ConnMan. >> According the following comment it should even be removed eventually: >> >> /** >> * Used for autoscan "emulation". >> * Should be removed when wpa_s autoscan support will be by default. >> */ >> struct autoscan_params { >> int base; >> int limit; >> int interval; >> unsigned int timeout; >> }; >> >> But it looks like it also used for the active scanning these days. If >> this true maybe the following patch might help (shot into the dark): >> > > Thanks! I see what you're doing there. Isn't that change equivalent to > always setting BackgroundScanning to true? If so, that doesn't seem like > the right thing. Better to deprecate and ignore BackgroundScanning, I would > think. I'll await Jose's comments as well. Yeah, quite likely that my patch is not working. It should not be labled as PATCH, FAIL would match it better :) This code is horrible complex and quite hard to debug. It is a while since I looked at this code. I am still wondering how the active scan is supposed to work here.