Re: [PATCH 3/3] netdev: Separate connect_failed and disconnected paths

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 11/15/23 12:27, James Prestwood wrote:
> Hi Denis,
> 
> On 11/14/23 16:05, Denis Kenzior wrote:
>> Commit c59669a366c5 ("netdev: disambiguate between disconnection types")
>> introduced different paths for different types of disconnection
>> notifications from netdev.  Formalize this further by having
>> netdev_connect_failed only invoke connect_cb.
>>
>> Disconnections that could be triggered outside of connection
>> related events are now handled on a different code path.  For this
>> purpose, netdev_disconnected() is introduced.
>> ---
>>   src/netdev.c | 45 +++++++++++++++++++++++++++++++++------------
>>   1 file changed, 33 insertions(+), 12 deletions(-)
> 
> LGTM. You may have already thought of this, but I do still think we could have a 
> case where an AP could send a deauth during key setting right?. So key setting 
> could still succeed but station would get notified of the failure via the netdev 
> event (in netdev_disconnect_event), not the connect callback.
> 
Yes, it can happen at any time.  We have some inconsistencies in how we handle 
the corner cases.  For example:

- We Authenticate successfully, but AP Deauthenticates us before we Associate. 
This is handled in netdev_deauthenticate_event.  See 2bebb4bdc7ee ("netdev: 
Handle deauth frames prior to association").  We treat this as a connect 
failure, but I'm no longer sure that is right.
- We associate and handshake successfully, but AP decides to deauthenticate us 
prior to setting keys, or setting station.  Pretty strange case, but should our 
connection throttling logic apply?  Today we treat this case as a connection 
failure for initial connections, and don't handle it at all for the roaming 
case.  This is what is happening in Alvin's case.
- Firmware / kernel can deauthenticate us at any point.  Kernel probably doesn't 
actually do this, but the firmware can?  How should we handle these?


Also, I think we might still have some problems when we connect / roam 
successfully but get disconnected prior to netconfig finishing.  We should audit 
the code and make sure we're consistent.  For example, are d-bus method returns 
and state transitions generated at the right time?

Regards,
-Denis