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

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 10/30/23 10:37, James Prestwood wrote:
> Hi Denis,
> 
> On 10/30/23 8:00 AM, Denis Kenzior wrote:
>> 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?
> 
> Yes, it appears mac80211 by default waits for 7 missed beacons before sending 
> the event. It then probes the AP (either nullfunc or probe request) so 
> apparently the connection could be recovered if the AP responded. Unfortunately 
> we don't get any notification in userspace if the AP responded or not...

So this magic here?
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/tree/net/mac80211/mlme.c#n3215

> 
> I can't remember what hardware was tested. But there really wasn't a consistent 
> way to test this. The testing involved me disabling roaming and walking away 
> from the AP until I got disconnected. Sometimes this was due to beacon loss, 
> sometimes the AP disconnected explicitly. But what I do remember is when beacon 
> loss occurred, a local disconnect followed near immediately. This is why (I 
> think) we thought there was no reason to handle this event.

So what does your ath10k driver/hw do?  Does it send nullfuncs or probe requests?

> 
>>
>> 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?
> 
>  From what I can tell in mac80211 power save doesn't change handling. Its the 
> driver that tells mac80211 of the beacon loss but maybe the driver (or firmware) 
> could handle it differently depending on power save.
> 
> When I was watching this device power save was disabled.

Okay, fair enough.

>> 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.
> 
> Yeah, adding a driver specific quirk doesn't seem like the right route.
> 
> I think for now there is no harm in trying to roam on beacon loss, basically the 
> same handling as packet loss. If a disconnect comes immediately the scan would 
> be canceled. Otherwise maybe we get lucky and be able to roam.

So the problem is, we had the _exact_ same behavior you're proposing here.  We 
took it out.  See commit:
836beb1276d1 ("station/wsc: remove beacon loss handling")

So when we do that, alarm bells start going off.  Why did we get rid of it if it 
was useful?

7 consecutive lost beacons is actually a lot.  That's ~700ms with no connection 
with default settings.  And you can maintain the connection after that for 
another 5-6?  Something smells fishy.

If the kernel has a hard limit after which it expects the connection to be 
disconnected, we can start a timer for 2-4x that limit?  Looks like kernel uses 
probe_wait_ms parameter for this with a default of 500ms.  Is your setup using 
the default values for beacon_loss_count and probe_wait_ms?

Regards,
-Denis