Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Larry Finger <larry.finger@gmail.com>
Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA
To:     Pkshih <pkshih@realtek.com>,
        "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "mail@maciej.szmigiero.name" <mail@maciej.szmigiero.name>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "johannes@sipsolutions.net" <johannes@sipsolutions.net>
References: <e2924d81-0e30-2dd0-292b-428fea199484@maciej.szmigiero.name>
 <846f6166-c570-01fc-6bbc-3e3b44e51327@maciej.szmigiero.name>
 <87r1jnohq6.fsf@codeaurora.org>
 <8e0434eb-d15f-065d-2ba7-b50c67877112@maciej.szmigiero.name>
 <a2003668-5108-27b9-95cd-9e1d5d1aa94d@lwfinger.net>
 <1617763692.9857.7.camel@realtek.com>
From:   Larry Finger <Larry.Finger@lwfinger.net>
Message-ID: <1dc7e487-b97b-8584-47f7-37f3385c7bf9@lwfinger.net>
Date:   Tue, 6 Apr 2021 23:21:00 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <1617763692.9857.7.camel@realtek.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 4/6/21 9:48 PM, Pkshih wrote:
> 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@maciej.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.

Maceij,

Does this patch fix the problem?

Larry