Re: [PATCH 0/4] Packet/beacon loss roaming improvements

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

> 
> We were seeing beacon loss events not resulting in an immediate
> disconnnect (as I have always expected), still eventually but after

If I recall correctly, Lost Beacon is sent after several beacons in succession 
were lost.  You are right that this could just be bad luck and doesn't actually 
mean that no packets are getting through.  However, in practice mac80211 almost 
always disconnected us soon after.  Didn't we test this pretty thoroughly?

My memory is fuzzy here, but I seem to recall that power save has an effect on 
how lost beacon events are treated by mac80211.  Maybe your recent power save 
patches had an effect?

> plenty of time to roam. I initially added handling for
> beacon loss identical to packet loss (try and find a better BSS) but
> noticed that if IWD did not find a better candidate it resulted in a
> disconnect 100% of the time. I watched a client for a full day and
> whenever beacon loss events arrived they were followed by a
> disconnect within ~5-6 seconds if IWD did not roam. This led me to
> believe that at least on ath10k a beacon loss is still very much a
> sign that a disconnect is going to come, we just have a bit more time
> than other drivers. This was the motivation behind re-using/re-naming
> the 'ap_directed_roam' flag in order to force IWD off the BSS.
> 

ath10k is still a mac80211 driver, no?  Given that we did test Lost Beacon event 
behavior before, I would like some more data points before being convinced it is 
a driver behavior change.

> Again, this is just one driver. Maybe other drivers can
> handle/recover from beacon loss. If we instead want to keep the
> behavior the same as packet loss I'm ok with that (I can keep the
> patch locally), or put the forced roam behavior behind a user
> option e.g. [Roam].ForceRoamOnBeaconLoss

If this is a driver behavior quirk, then this belongs in src/wiphy.c 
driver_infos table somehow.  I'd really rather not add a bazillion config 
options that address the bug-of-the-day.

> 
> James Prestwood (4):
>    station: rename ap_directed_roam to force_roam
>    station: start roam on beacon loss event
>    netdev: handle/send beacon loss event
>    station: rate limit packet loss roam scans
> 
>   src/netdev.c  |  6 ++++-
>   src/netdev.h  |  1 +
>   src/station.c | 61 +++++++++++++++++++++++++++++++++++++++++++--------
>   3 files changed, 58 insertions(+), 10 deletions(-)
> 

Regards,
-Denis