Re: [PATCH] remove AP always on device disconnect

[Ludwig Nussel <ludwig nussel suse de>]

Dan Williams wrote:
> On Tue, 2011-10-11 at 16:51 +0200, Ludwig Nussel wrote:
>> Dan Williams wrote:
>>> On Fri, 2011-10-07 at 15:21 +0200, Ludwig Nussel wrote:
>>>> This avoids immediate reconnect after link timeout to an AP that may no longer exist (down/out of range). This also avoids needless prompting for a password for the no longer existing AP.
>>>
>>> This could work, but I think we need to restrict the AP removal to cases
>>> where the supplicant times out after we've already been successfully
>>> connected, which is basically the case you're talking about.  For
>>> example, in the case of an explicit disconnect (via the menu item or
>>> D-Bus call) we'd remove a perfectly good AP and might not find it again
>>> until the next scan happened a few minutes after the disconnect.
>>
>> Yes. Is there a way to find out whether the request was triggered by
>> an event from wpa_supplicant or by the user?
>>
>>> Maybe instead of doing it during deactivate, remove the AP in
>>> supplicant_connection_timeout_cb() right before moving to the
>>> DISCONNECTED state?
>>
>> Doesn't that callback trigger too late? Ie after NM has already
>> prompted for a new password?
> 
> That function *should* be the place where disconnects after successful
> connection are handled.  But yeah, the code is wrong in
> supplicant_iface_state_cb() (probably from a6f7dfff233de118) so that
> case doesn't get hit correctly.  I'll fix that, but in the end, here's
> what should happen:

I think it's been that way before and intentionally so. To handle the
first case you listed below.

> 1) disconnect during connection process: if we can tell that the
> password is wrong (need help from wpa_supplicant here) ask for new
> secrets; otherwise if we've connected to this SSID before (ie, timestamp
>> 0) then don't ask for secrets, fail with an error code that the
> secrets might be wrong.  If we haven't connected successfully before,
> then possibly ask for secrets.

Sounds good. That's where supplicant_iface_state_cb() is involved I think.

> 2) disconnect after successful connect: start a timer and wait for the
> supplicant to reassociate; it that doesn't happen, then when the timer
> fires, deactivate the device and mark the connection as "invalid" for a
> short period of time (at least until new scan results come in)

Yes. That's what I wanted to achieve, ie prevent reconnect to a no longer
existing AP. From my limited experiments I think this case happens when
the AP gets switched off, goes out of range or kicks the client out.
That's where link_timeout_cb() fires.

Besides the AP vanishing from the list on explicit user disconnect do you
see any other bad side effect of my patch? The deadline for openSUSE
12.1 is near. So even if the patch is not acceptable for upstream I'd
like to add it to our package anyways and accept that minor drawback to
avoid users whining about the PK popups.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)