Message-ID: <1617763692.9857.7.camel@realtek.com>
Date:   Wed, 7 Apr 2021 02:48:13 +0000
From:   Pkshih <pkshih@...ltek.com>
To:     "kvalo@...eaurora.org" <kvalo@...eaurora.org>,
        "mail@...iej.szmigiero.name" <mail@...iej.szmigiero.name>,
        "Larry.Finger@...inger.net" <Larry.Finger@...inger.net>
CC:     "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "johannes@...solutions.net" <johannes@...solutions.net>
Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA

On Tue, 2021-04-06 at 11:25 -0500, Larry Finger wrote:
> On 4/6/21 7:06 AM, Maciej S. Szmigiero wrote:
> > On 06.04.2021 12:00, Kalle Valo wrote:
> >> "Maciej S. Szmigiero" <mail@...iej.szmigiero.name> 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?
> 
> Yes, I am working on this. My only question is "if you are such an expert on the 
> problem, why do you not fix it?"
> 
> The example in rx200 is not particularly useful, and I have not found any other 
> examples.
> 

Hi Larry,

I have a draft patch that forks a work to do send_beacon_frame(), whose
behavior like Maciej mentioned.
I did test on RTL8821AE; it works well. But, it seems already work well even
I don't apply this patch, and I'm still digging why.

I don't have a rtl8192cu dongle on hand, but I'll try to find one.

---
Ping-Ke


View attachment "0001-rtlwifi-implement-set_tim-by-update-beacon-content.patch" of type "text/x-patch" (5209 bytes)

Powered by blists - more mailing lists