2021-03-28 22:56:44

by Maciej S. Szmigiero

[permalink] [raw]
Subject: rtlwifi/rtl8192cu AP mode broken with PS STA

Hi,

It looks like rtlwifi/rtl8192cu AP mode is broken when a STA is using PS,
since the driver does not update its beacon to account for TIM changes,
so a station that is sleeping will never learn that it has packets
buffered at the AP.

Looking at the code, the rtl8192cu driver implements neither the set_tim()
callback, nor does it explicitly update beacon data periodically, so it
has no way to learn that it had changed.

This results in the AP mode being virtually unusable with STAs that do
PS and don't allow for it to be disabled (IoT devices, mobile phones,
etc.).

I think the easiest fix here would be to implement set_tim() for example
the way rt2x00 driver does: queue a work or schedule a tasklet to update
the beacon data on the device.

Thanks,
Maciej


2021-04-06 21:05:48

by Maciej S. Szmigiero

[permalink] [raw]
Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA

On 06.04.2021 12:00, Kalle Valo wrote:
> "Maciej S. Szmigiero" <[email protected]> writes:
>
>> On 29.03.2021 00:54, Maciej S. Szmigiero wrote:
>>> Hi,
>>>
>>> It looks like rtlwifi/rtl8192cu AP mode is broken when a STA is using PS,
>>> since the driver does not update its beacon to account for TIM changes,
>>> so a station that is sleeping will never learn that it has packets
>>> buffered at the AP.
>>>
>>> Looking at the code, the rtl8192cu driver implements neither the set_tim()
>>> callback, nor does it explicitly update beacon data periodically, so it
>>> has no way to learn that it had changed.
>>>
>>> This results in the AP mode being virtually unusable with STAs that do
>>> PS and don't allow for it to be disabled (IoT devices, mobile phones,
>>> etc.).
>>>
>>> I think the easiest fix here would be to implement set_tim() for example
>>> the way rt2x00 driver does: queue a work or schedule a tasklet to update
>>> the beacon data on the device.
>>
>> Are there any plans to fix this?
>> The driver is listed as maintained by Ping-Ke.
>
> Yeah, power save is hard and I'm not surprised that there are drivers
> with broken power save mode support. If there's no fix available we
> should stop supporting AP mode in the driver.
>

https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/api
clearly documents that "For AP mode, it must (...) react to the set_tim()
callback or fetch each beacon from mac80211".

The driver isn't doing either so no wonder the beacon it is sending
isn't getting updated.

As I have said above, it seems to me that all that needs to be done here
is to queue a work in a set_tim() callback, then call
send_beacon_frame() from rtlwifi/core.c from this work.

But I don't know the exact device semantics, maybe it needs some other
notification that the beacon has changed, too, or even tries to
manage the TIM bitmap by itself.

It would be a shame to lose the AP mode for such minor thing, though.

I would play with this myself, but unfortunately I don't have time
to work on this right now.

That's where my question to Realtek comes: are there plans to actually
fix this?

Maciej

2021-04-19 20:47:20

by Maciej S. Szmigiero

[permalink] [raw]
Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA

On 19.04.2021 09:04, Pkshih wrote:
>
>> -----Original Message-----
>> From: Larry Finger [mailto:[email protected]] On Behalf Of Larry Finger
>> Sent: Monday, April 19, 2021 9:23 AM
>> To: Pkshih; Maciej S. Szmigiero
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA
>>
>> On 4/18/21 7:32 PM, Pkshih wrote:
>>>
>>>> -----Original Message-----
>>>> From: Maciej S. Szmigiero [mailto:[email protected]]
>>>> Sent: Sunday, April 18, 2021 2:08 AM
>>>> To: Pkshih
>>>> Cc: [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]; Larry Finger
>>>> Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA
>>>>
>>>> On 08.04.2021 21:04, Maciej S. Szmigiero wrote:
>>>>> On 08.04.2021 06:42, Pkshih wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Maciej S. Szmigiero [mailto:[email protected]]
>>>>>>> Sent: Thursday, April 08, 2021 4:53 AM
>>>>>>> To: Larry Finger; Pkshih
>>>>>>> Cc: [email protected]; [email protected]; [email protected];
>>>>>>> [email protected]; [email protected]
>>>>>>> Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA
>>>>>>>
>>>>> (...)
>>>>>>>> Maceij,
>>>>>>>>
>>>>>>>> Does this patch fix the problem?
>>>>>>>
>>>>>>> The beacon seems to be updating now and STAs no longer get stuck in PS
>>>>>>> mode.
>>>>>>> Although sometimes (every 2-3 minutes with continuous 1s interval pings)
>>>>>>> there is around 5s delay in updating the transmitted beacon - don't know
>>>>>>> why, maybe the NIC hardware still has the old version in queue?
>>>>>>
>>>>>> Since USB device doesn't update every beacon, dtim_count isn't updated neither.
>>>>>> It leads STA doesn't awake properly. Please try to fix dtim_period=1 in
>>>>>> hostapd.conf, which tells STA awakes every beacon interval.
>>>>>
>>>>> The situation is the same with dtim_period=1.
>>>>>
>>>> (...)
>>>>
>>>> Ping-Ke,
>>>> are you going to submit your set_tim() patch so at least the AP mode is
>>>> usable with PS STAs or are you waiting for a solution to the delayed
>>>> beacon update issue?
>>>>
>>>
>>> I'm still trying to get a 8192cu, and then I can reproduce the symptom you
>>> met. However, I'm busy now; maybe I have free time two weeks later.
>>>
>>> Do you think I submit the set_tim() patch with your Reported-by and Tested-by first?
>>
>> PK,
>>
>> I would say yes. Get the fix in as soon as possible.
>>
>
> I have sent a patch that only 8192cu, which is the only one USB device supported by rtlwifi,
> schedules a work to update beacon content to wifi card.
>
> https://lore.kernel.org/linux-wireless/[email protected]/T/#u

Thanks, I have tested the patch and it seems to work as good as the previous one.
It definitely improves things.

However, it would be great to eventually fix the update delay issue, too.
It looks to me like possibly just a missing beacon queue flush.

> --
> Ping-Ke
>

Maciej