netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* rtlwifi/rtl8192cu AP mode broken with PS STA
@ 2021-03-28 22:54 Maciej S. Szmigiero
  2021-04-04 18:06 ` Maciej S. Szmigiero
  0 siblings, 1 reply; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-03-28 22:54 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo
  Cc: Johannes Berg, linux-wireless, netdev, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-03-28 22:54 rtlwifi/rtl8192cu AP mode broken with PS STA Maciej S. Szmigiero
@ 2021-04-04 18:06 ` Maciej S. Szmigiero
  2021-04-06 10:00   ` Kalle Valo
  0 siblings, 1 reply; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-04-04 18:06 UTC (permalink / raw)
  To: Ping-Ke Shih
  Cc: Johannes Berg, linux-wireless, netdev, linux-kernel, Kalle Valo,
	Larry Finger

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.

Thanks,
Maciej

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-04 18:06 ` Maciej S. Szmigiero
@ 2021-04-06 10:00   ` Kalle Valo
  2021-04-06 12:06     ` Maciej S. Szmigiero
  0 siblings, 1 reply; 16+ messages in thread
From: Kalle Valo @ 2021-04-06 10:00 UTC (permalink / raw)
  To: Maciej S. Szmigiero
  Cc: Ping-Ke Shih, Johannes Berg, linux-wireless, netdev,
	linux-kernel, Larry Finger

"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://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-06 10:00   ` Kalle Valo
@ 2021-04-06 12:06     ` Maciej S. Szmigiero
  2021-04-06 16:25       ` Larry Finger
  0 siblings, 1 reply; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-04-06 12:06 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Ping-Ke Shih, Johannes Berg, linux-wireless, netdev,
	linux-kernel, Larry Finger

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?

Maciej

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-06 12:06     ` Maciej S. Szmigiero
@ 2021-04-06 16:25       ` Larry Finger
  2021-04-06 18:29         ` Maciej S. Szmigiero
  2021-04-07  2:48         ` Pkshih
  0 siblings, 2 replies; 16+ messages in thread
From: Larry Finger @ 2021-04-06 16:25 UTC (permalink / raw)
  To: Maciej S. Szmigiero, Kalle Valo
  Cc: Ping-Ke Shih, Johannes Berg, linux-wireless, netdev, linux-kernel

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.

Larry


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-06 16:25       ` Larry Finger
@ 2021-04-06 18:29         ` Maciej S. Szmigiero
  2021-04-07  2:48         ` Pkshih
  1 sibling, 0 replies; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-04-06 18:29 UTC (permalink / raw)
  To: Larry Finger
  Cc: Ping-Ke Shih, Johannes Berg, linux-wireless, netdev,
	linux-kernel, Kalle Valo

On 06.04.2021 18:25, 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?"

I don't think I am an expert here - I've tried to use a rtl8192cu USB
dongle in AP mode but its STAs would become unreachable or disconnect
after a short while, so I have started investigating the reason for such
problems.
Ultimately, I have traced it to DTIM in beacons not indicating there are
frames buffered for connected stations.

Then I've looked how the beacon that is broadcast is supposed to get
updated when it changes and seen there seems to be no existing mechanism
for this in rtl8192cu driver.
However, I had to stop at this point and post my findings as I could not
commit more time to this issue due to other workload.

> The example in rx200 is not particularly useful, and I have not found any other examples.

That's why I thought it would be best if somebody from Realtek, with
deep knowledge of both the driver and the hardware, could voice their
opinion here.

As I have stated earlier, just uploading new beacon to the hardware
might not be enough for it to be (safely) updated.

> Larry
> 

Thanks,
Maciej

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-06 16:25       ` Larry Finger
  2021-04-06 18:29         ` Maciej S. Szmigiero
@ 2021-04-07  2:48         ` Pkshih
  2021-04-07  4:21           ` Larry Finger
  1 sibling, 1 reply; 16+ messages in thread
From: Pkshih @ 2021-04-07  2:48 UTC (permalink / raw)
  To: kvalo, mail, Larry.Finger; +Cc: linux-wireless, netdev, linux-kernel, johannes

[-- Attachment #1: Type: text/plain, Size: 3068 bytes --]

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.

---
Ping-Ke


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-rtlwifi-implement-set_tim-by-update-beacon-content.patch --]
[-- Type: text/x-patch; name="0001-rtlwifi-implement-set_tim-by-update-beacon-content.patch", Size: 5209 bytes --]

From 44be80232aa49737c035ee4656d20a22f573d33e Mon Sep 17 00:00:00 2001
From: Ping-Ke Shih <pkshih@realtek.com>
Date: Tue, 6 Apr 2021 19:55:59 +0800
Subject: [PATCH] rtlwifi: implement set_tim by update beacon content

Once beacon content is changed, we update the content to wifi card by
send_beacon_frame().

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtlwifi/core.c | 30 +++++++++++++++++++++
 drivers/net/wireless/realtek/rtlwifi/core.h |  1 +
 drivers/net/wireless/realtek/rtlwifi/pci.c  |  3 +++
 drivers/net/wireless/realtek/rtlwifi/usb.c  |  3 +++
 drivers/net/wireless/realtek/rtlwifi/wifi.h |  1 +
 5 files changed, 38 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
index 965bd9589045..ca47a70d9a86 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -1018,6 +1018,25 @@ static void send_beacon_frame(struct ieee80211_hw *hw,
 	}
 }
 
+void rtl_update_beacon_work_callback(struct work_struct *work)
+{
+	struct rtl_works *rtlworks =
+	    container_of(work, struct rtl_works, update_beacon_work);
+	struct ieee80211_hw *hw = rtlworks->hw;
+	struct rtl_priv *rtlpriv = rtl_priv(hw);
+	struct ieee80211_vif *vif = rtlpriv->mac80211.vif;
+
+	if (!vif) {
+		WARN_ONCE(true, "no vif to update beacon\n");
+		return;
+	}
+
+	mutex_lock(&rtlpriv->locks.conf_mutex);
+	send_beacon_frame(hw, vif);
+	mutex_unlock(&rtlpriv->locks.conf_mutex);
+}
+EXPORT_SYMBOL_GPL(rtl_update_beacon_work_callback);
+
 static void rtl_op_bss_info_changed(struct ieee80211_hw *hw,
 				    struct ieee80211_vif *vif,
 				    struct ieee80211_bss_conf *bss_conf,
@@ -1747,6 +1766,16 @@ static void rtl_op_flush(struct ieee80211_hw *hw,
 		rtlpriv->intf_ops->flush(hw, queues, drop);
 }
 
+static int rtl_op_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
+			  bool set)
+{
+	struct rtl_priv *rtlpriv = rtl_priv(hw);
+
+	schedule_work(&rtlpriv->works.update_beacon_work);
+
+	return 0;
+}
+
 /*	Description:
  *		This routine deals with the Power Configuration CMD
  *		 parsing for RTL8723/RTL8188E Series IC.
@@ -1903,6 +1932,7 @@ const struct ieee80211_ops rtl_ops = {
 	.sta_add = rtl_op_sta_add,
 	.sta_remove = rtl_op_sta_remove,
 	.flush = rtl_op_flush,
+	.set_tim = rtl_op_set_tim,
 };
 EXPORT_SYMBOL_GPL(rtl_ops);
 
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.h b/drivers/net/wireless/realtek/rtlwifi/core.h
index 7447ff456710..345161b47442 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.h
+++ b/drivers/net/wireless/realtek/rtlwifi/core.h
@@ -60,5 +60,6 @@ void rtl_bb_delay(struct ieee80211_hw *hw, u32 addr, u32 data);
 bool rtl_cmd_send_packet(struct ieee80211_hw *hw, struct sk_buff *skb);
 bool rtl_btc_status_false(void);
 void rtl_dm_diginit(struct ieee80211_hw *hw, u32 cur_igval);
+void rtl_update_beacon_work_callback(struct work_struct *work);
 
 #endif
diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index 3776495fd9d0..c7365354737e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -1200,6 +1200,8 @@ static void _rtl_pci_init_struct(struct ieee80211_hw *hw,
 		     _rtl_pci_prepare_bcn_tasklet);
 	INIT_WORK(&rtlpriv->works.lps_change_work,
 		  rtl_lps_change_work_callback);
+	INIT_WORK(&rtlpriv->works.update_beacon_work,
+		  rtl_update_beacon_work_callback);
 }
 
 static int _rtl_pci_init_tx_ring(struct ieee80211_hw *hw,
@@ -1742,6 +1744,7 @@ static void rtl_pci_deinit(struct ieee80211_hw *hw)
 	synchronize_irq(rtlpci->pdev->irq);
 	tasklet_kill(&rtlpriv->works.irq_tasklet);
 	cancel_work_sync(&rtlpriv->works.lps_change_work);
+	cancel_work_sync(&rtlpriv->works.update_beacon_work);
 
 	flush_workqueue(rtlpriv->works.rtl_wq);
 	destroy_workqueue(rtlpriv->works.rtl_wq);
diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
index 6c5e242b1bc5..b5e1f9403949 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -805,6 +805,7 @@ static void rtl_usb_stop(struct ieee80211_hw *hw)
 
 	tasklet_kill(&rtlusb->rx_work_tasklet);
 	cancel_work_sync(&rtlpriv->works.lps_change_work);
+	cancel_work_sync(&rtlpriv->works.update_beacon_work);
 
 	flush_workqueue(rtlpriv->works.rtl_wq);
 
@@ -1031,6 +1032,8 @@ int rtl_usb_probe(struct usb_interface *intf,
 		  rtl_fill_h2c_cmd_work_callback);
 	INIT_WORK(&rtlpriv->works.lps_change_work,
 		  rtl_lps_change_work_callback);
+	INIT_WORK(&rtlpriv->works.update_beacon_work,
+		  rtl_update_beacon_work_callback);
 
 	rtlpriv->usb_data_index = 0;
 	init_completion(&rtlpriv->firmware_loading_complete);
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index fdccfd29fd61..8152117c03ee 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -2487,6 +2487,7 @@ struct rtl_works {
 
 	struct work_struct lps_change_work;
 	struct work_struct fill_h2c_cmd;
+	struct work_struct update_beacon_work;
 };
 
 struct rtl_debug {
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-07  2:48         ` Pkshih
@ 2021-04-07  4:21           ` Larry Finger
  2021-04-07 20:53             ` Maciej S. Szmigiero
  0 siblings, 1 reply; 16+ messages in thread
From: Larry Finger @ 2021-04-07  4:21 UTC (permalink / raw)
  To: Pkshih, kvalo, mail; +Cc: linux-wireless, netdev, linux-kernel, johannes

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-07  4:21           ` Larry Finger
@ 2021-04-07 20:53             ` Maciej S. Szmigiero
  2021-04-08  4:42               ` Pkshih
  0 siblings, 1 reply; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-04-07 20:53 UTC (permalink / raw)
  To: Larry Finger, Pkshih
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo

On 07.04.2021 06:21, Larry Finger wrote:
> 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.

That's great, thanks!

>> 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.

It looks like PCI rtlwifi hardware uses a tasklet (specifically,
_rtl_pci_prepare_bcn_tasklet() in pci.c) to periodically transfer the
current beacon to the NIC.

This tasklet is scheduled on a RTL_IMR_BCNINT interrupt, which sounds
like a beacon interval interrupt.

>> I don't have a rtl8192cu dongle on hand, but I'll try to find one.
> 
> 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?

I doubt, however that this set_tim() callback should be added for every
rtlwifi device type.

As I have said above, PCI devices seem to already have a mechanism in
place to update their beacon each beacon interval.
Your test that RTL8821AE works without this patch confirms that (at
least for the rtl8821ae driver).

It seems this callback is only necessarily for USB rtlwifi devices.
Which currently means rtl8192cu only.

Thanks,
Maciej

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-07 20:53             ` Maciej S. Szmigiero
@ 2021-04-08  4:42               ` Pkshih
  2021-04-08 19:04                 ` Maciej S. Szmigiero
  0 siblings, 1 reply; 16+ messages in thread
From: Pkshih @ 2021-04-08  4:42 UTC (permalink / raw)
  To: Maciej S. Szmigiero, Larry Finger
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo



> -----Original Message-----
> From: Maciej S. Szmigiero [mailto:mail@maciej.szmigiero.name]
> Sent: Thursday, April 08, 2021 4:53 AM
> To: Larry Finger; Pkshih
> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> johannes@sipsolutions.net; kvalo@codeaurora.org
> Subject: Re: rtlwifi/rtl8192cu AP mode broken with PS STA
> 
> On 07.04.2021 06:21, Larry Finger wrote:
> > 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.
> 
> That's great, thanks!
> 
> >> 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.
> 
> It looks like PCI rtlwifi hardware uses a tasklet (specifically,
> _rtl_pci_prepare_bcn_tasklet() in pci.c) to periodically transfer the
> current beacon to the NIC.

Got it.

> 
> This tasklet is scheduled on a RTL_IMR_BCNINT interrupt, which sounds
> like a beacon interval interrupt.
> 

Yes, PCI series update every beacon, so TIM and DTIM count maintained by
mac80211 work properly.

> >> I don't have a rtl8192cu dongle on hand, but I'll try to find one.
> >
> > 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.

> 
> I doubt, however that this set_tim() callback should be added for every
> rtlwifi device type.
> 
> As I have said above, PCI devices seem to already have a mechanism in
> place to update their beacon each beacon interval.
> Your test that RTL8821AE works without this patch confirms that (at
> least for the rtl8821ae driver).
> 
> It seems this callback is only necessarily for USB rtlwifi devices.
> Which currently means rtl8192cu only.
> 

I agree with you.

--
Ping-Ke



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-08  4:42               ` Pkshih
@ 2021-04-08 19:04                 ` Maciej S. Szmigiero
  2021-04-17 18:07                   ` Maciej S. Szmigiero
  0 siblings, 1 reply; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-04-08 19:04 UTC (permalink / raw)
  To: Pkshih
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo, Larry Finger

On 08.04.2021 06:42, Pkshih wrote:
>> -----Original Message-----
>> From: Maciej S. Szmigiero [mailto:mail@maciej.szmigiero.name]
>> Sent: Thursday, April 08, 2021 4:53 AM
>> To: Larry Finger; Pkshih
>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>> johannes@sipsolutions.net; kvalo@codeaurora.org
>> 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.

When I look at a packet trace (captured from a different NIC running in
monitor mode) it's obvious that the pinged STA wakes up almost
immediately once it sees its DTIM bit set in a beacon.

But many beacons pass (the network has beacon interval of 0.1s) between
where a ping request (ICMP echo request) is buffered on the AP and where
the transmitted beacon actually starts to have DTIM bit set.

I am observing delays up to 9 seconds, which means a delay up to 90
beacons between when DTIM bit should be set and when it actually gets
set.

As I said above, this happens only sometimes (every 2-3 minutes with
continuous 1s interval pings) but there is definitely something wrong
here.

My wild guess would be that if the NIC is prevented from sending a beacon
due to, for example, its radio channel being busy it keeps that beacon
in queue for the next transmission attempt and only then sends it and
removes from that queue.
Even thought there might be newer beacon data already available for
transmission.

And then these "unsent" beacons pile up in queue and the delays I am
observing are simply old queued beacons (that don't have the DTIM bit
set yet) being transmitted.

But that's just my hypothesis - I don't know how rtl8192cu hardware
actually works.

> --
> Ping-Ke

Thanks,
Maciej

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-08 19:04                 ` Maciej S. Szmigiero
@ 2021-04-17 18:07                   ` Maciej S. Szmigiero
  2021-04-19  0:32                     ` Pkshih
  0 siblings, 1 reply; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-04-17 18:07 UTC (permalink / raw)
  To: Pkshih
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo, Larry Finger

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:mail@maciej.szmigiero.name]
>>> Sent: Thursday, April 08, 2021 4:53 AM
>>> To: Larry Finger; Pkshih
>>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>>> johannes@sipsolutions.net; kvalo@codeaurora.org
>>> 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?

Thanks,
Maciej

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-17 18:07                   ` Maciej S. Szmigiero
@ 2021-04-19  0:32                     ` Pkshih
  2021-04-19  1:23                       ` Larry Finger
  0 siblings, 1 reply; 16+ messages in thread
From: Pkshih @ 2021-04-19  0:32 UTC (permalink / raw)
  To: Maciej S. Szmigiero
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo, Larry Finger


> -----Original Message-----
> From: Maciej S. Szmigiero [mailto:mail@maciej.szmigiero.name]
> Sent: Sunday, April 18, 2021 2:08 AM
> To: Pkshih
> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> johannes@sipsolutions.net; kvalo@codeaurora.org; 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:mail@maciej.szmigiero.name]
> >>> Sent: Thursday, April 08, 2021 4:53 AM
> >>> To: Larry Finger; Pkshih
> >>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> >>> johannes@sipsolutions.net; kvalo@codeaurora.org
> >>> 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?

Thanks 
Ping-Ke


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-19  0:32                     ` Pkshih
@ 2021-04-19  1:23                       ` Larry Finger
  2021-04-19  7:04                         ` Pkshih
  0 siblings, 1 reply; 16+ messages in thread
From: Larry Finger @ 2021-04-19  1:23 UTC (permalink / raw)
  To: Pkshih, Maciej S. Szmigiero
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo

On 4/18/21 7:32 PM, Pkshih wrote:
> 
>> -----Original Message-----
>> From: Maciej S. Szmigiero [mailto:mail@maciej.szmigiero.name]
>> Sent: Sunday, April 18, 2021 2:08 AM
>> To: Pkshih
>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>> johannes@sipsolutions.net; kvalo@codeaurora.org; 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:mail@maciej.szmigiero.name]
>>>>> Sent: Thursday, April 08, 2021 4:53 AM
>>>>> To: Larry Finger; Pkshih
>>>>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>>>>> johannes@sipsolutions.net; kvalo@codeaurora.org
>>>>> 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.

Larry


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-19  1:23                       ` Larry Finger
@ 2021-04-19  7:04                         ` Pkshih
  2021-04-19 19:25                           ` Maciej S. Szmigiero
  0 siblings, 1 reply; 16+ messages in thread
From: Pkshih @ 2021-04-19  7:04 UTC (permalink / raw)
  To: Larry Finger, Maciej S. Szmigiero
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo


> -----Original Message-----
> From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry Finger
> Sent: Monday, April 19, 2021 9:23 AM
> To: Pkshih; Maciej S. Szmigiero
> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> johannes@sipsolutions.net; kvalo@codeaurora.org
> 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:mail@maciej.szmigiero.name]
> >> Sent: Sunday, April 18, 2021 2:08 AM
> >> To: Pkshih
> >> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> johannes@sipsolutions.net; kvalo@codeaurora.org; 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:mail@maciej.szmigiero.name]
> >>>>> Sent: Thursday, April 08, 2021 4:53 AM
> >>>>> To: Larry Finger; Pkshih
> >>>>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> >>>>> johannes@sipsolutions.net; kvalo@codeaurora.org
> >>>>> 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/20210419065956.6085-1-pkshih@realtek.com/T/#u

--
Ping-Ke


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rtlwifi/rtl8192cu AP mode broken with PS STA
  2021-04-19  7:04                         ` Pkshih
@ 2021-04-19 19:25                           ` Maciej S. Szmigiero
  0 siblings, 0 replies; 16+ messages in thread
From: Maciej S. Szmigiero @ 2021-04-19 19:25 UTC (permalink / raw)
  To: Pkshih
  Cc: linux-wireless, netdev, linux-kernel, johannes, kvalo, Larry Finger

On 19.04.2021 09:04, Pkshih wrote:
> 
>> -----Original Message-----
>> From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry Finger
>> Sent: Monday, April 19, 2021 9:23 AM
>> To: Pkshih; Maciej S. Szmigiero
>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>> johannes@sipsolutions.net; kvalo@codeaurora.org
>> 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:mail@maciej.szmigiero.name]
>>>> Sent: Sunday, April 18, 2021 2:08 AM
>>>> To: Pkshih
>>>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>>>> johannes@sipsolutions.net; kvalo@codeaurora.org; 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:mail@maciej.szmigiero.name]
>>>>>>> Sent: Thursday, April 08, 2021 4:53 AM
>>>>>>> To: Larry Finger; Pkshih
>>>>>>> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>>>>>>> johannes@sipsolutions.net; kvalo@codeaurora.org
>>>>>>> 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/20210419065956.6085-1-pkshih@realtek.com/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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-04-19 19:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-28 22:54 rtlwifi/rtl8192cu AP mode broken with PS STA Maciej S. Szmigiero
2021-04-04 18:06 ` Maciej S. Szmigiero
2021-04-06 10:00   ` Kalle Valo
2021-04-06 12:06     ` Maciej S. Szmigiero
2021-04-06 16:25       ` Larry Finger
2021-04-06 18:29         ` Maciej S. Szmigiero
2021-04-07  2:48         ` Pkshih
2021-04-07  4:21           ` Larry Finger
2021-04-07 20:53             ` Maciej S. Szmigiero
2021-04-08  4:42               ` Pkshih
2021-04-08 19:04                 ` Maciej S. Szmigiero
2021-04-17 18:07                   ` Maciej S. Szmigiero
2021-04-19  0:32                     ` Pkshih
2021-04-19  1:23                       ` Larry Finger
2021-04-19  7:04                         ` Pkshih
2021-04-19 19:25                           ` Maciej S. Szmigiero

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).