Re: [PATCH v2 1/2] station: start roam on beacon loss event

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 11/7/23 08:11, James Prestwood wrote:
> Beacon loss handling was removed in the past because it was
> determined that this even always resulted in a disconnect. This
> was short sighted and not always true. The default kernel behavior
> waits for 7 lost beacons before emitting this event, then sends
> either a few nullfuncs or probe requests to the BSS to determine
> if its really gone. If these come back successfully the connection
> will remain alive. This can give IWD some time to roam in some
> cases so we should be handling this event.
> 
> Since beacon loss indicates a very poor connection the roam scan
> is delayed by a few seconds in order to give the kernel a chance
> to send the nullfuncs/probes or receive more beacons. This may
> result in a disconnect, but it would have happened anyways.
> Attempting a roam mainly handles the case when the connection can
> be maintained after beacon loss, but is still poor.
> ---
>   src/netdev.h  |  1 +
>   src/station.c | 18 ++++++++++++++++++
>   2 files changed, 19 insertions(+)
> 
> v2:
>   * Delay the roam to give the kernel a chance to salvage the
>     connection
> 
> FYI, I did attempt to add back the beacon loss test without much
> luck. Blocking beacons with hwsim did not result in a beacon loss
> event which may have been one of the reasons it was removed in the
> first place. Even blocking _all_ frames doesn't result in an event
> but instead just a local disconnect.

Good info, thanks for doing this.  Guess we'll have to rely on manual testing 
for this feature. :(

> 
> After looking more into it I see that actually only a few drivers
> even use ieee80211_beacon_loss/ieee80211_connection_loss:
> 
> iwlwifi,wl1251,intersil,mt76,wfx,ath10/11k,wcn36xx
> 
> Every other driver likely disconnects rather than attempts to
> handle beacon loss. So probably for the majority of users this
> actually didn't matter, but for these drivers it does make sense
> to handle the event.
> 

All applied, thanks.

Regards,
-Denis