[PATCH rtw-next v3] wifi: rtw88: usb: route bmc frames via the high queue only for DTIM delivery

[PATCH rtw-next v3] wifi: rtw88: usb: route bmc frames via the high queue only for DTIM delivery

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

In AP mode every broadcast and multicast data frame is routed to
TX_DESC_QSEL_HIGH, the after-DTIM queue, whether or not anybody is
asleep. The firmware drains that queue at beacon pace, a dozen or so
frames per second measured on RTL8822BU, while one associated client's
mDNS/SSDP chatter alone exceeds that. The excess accumulates inside
the chip until the shared TX page pool is exhausted (measured: 14 of
1803 pages left). From that point every host-sourced frame queues
behind the backlog: authentication responses reach the air seconds
after the client has given up, so no station can associate anymore,
and the beacon reserved-page download fails the BCN_VALID poll
("error beacon valid") because it needs pages from the same pool. The
AP keeps beaconing, so the failure looks like a silent RX stall and
only a reboot recovers.

mac80211 already decides when after-DTIM delivery is needed: it sets
IEEE80211_TX_CTL_SEND_AFTER_DTIM on bmc frames only while at least one
station is actually dozing. Honor that instead of routing
unconditionally: flagged frames keep going through the high queue with
the MORE_DATA and HGQMD handling introduced by commit 076f786a0ae1
("wifi: rtw88: Fix AP mode incorrect DTIM behavior"), everything else
leaves at line rate through the AC queues. This partially reverts the
usb.c hunk of that commit, whose unconditional routing is what lets
the backlog build up.

On a bench AP (USB2, 20 MHz, WPA2, hostapd, a Windows client driven
through disconnect/reconnect cycles): reconnects fail 0/5 on RTL8822BU
and 0/3 on RTL8821CU before this change, and pass 10/10 and 5/5 with
it, with the page pool staying healthy and no beacon errors logged.

Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
v3: drop the Fixes tag (Ping-Ke), reverse xmas order for the
declarations (Ping-Ke).
v2: honor IEEE80211_TX_CTL_SEND_AFTER_DTIM instead of routing bmc
unconditionally to the AC queues.

The flagged path is the code 076f786a0ae1 added and is unchanged by
this patch; I did not have a client entering powersave on this bench
to exercise it explicitly and will follow up with that measurement.

 drivers/net/wireless/realtek/rtw88/usb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 64e1c3420..43b9cdb4a 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -560,6 +560,7 @@ static int rtw_usb_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
 static u8 rtw_usb_tx_queue_mapping_to_qsel(struct sk_buff *skb)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	__le16 fc = hdr->frame_control;
 	u8 qsel;
 
@@ -567,7 +568,8 @@ static u8 rtw_usb_tx_queue_mapping_to_qsel(struct sk_buff *skb)
 		qsel = TX_DESC_QSEL_MGMT;
 	else if (is_broadcast_ether_addr(hdr->addr1) ||
 		 is_multicast_ether_addr(hdr->addr1))
-		qsel = TX_DESC_QSEL_HIGH;
+		qsel = (info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ?
+		       TX_DESC_QSEL_HIGH : skb->priority;
 	else if (skb_get_queue_mapping(skb) <= IEEE80211_AC_BK)
 		qsel = skb->priority;
 	else
-- 
2.54.0

Re: [PATCH rtw-next v3] wifi: rtw88: usb: route bmc frames via the high queue only for DTIM delivery

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

Hello Ping-Ke,

One more data point that may be useful for the firmware side,
separate from this patch.

While comparing against the vendor driver I noticed a difference in
how MORE_DATA is terminated. rtw88 sets the bit on every high queue
frame (rtw_tx_fill_tx_desc(): more_data = qsel == TX_DESC_QSEL_HIGH),
including the last one. The vendor driver releases the high queue as
an explicit batch and sets mdata = 1 on all frames of the batch
except the final one, which carries 0 (chk_bmc_sleepq_hdl() in
core/rtw_mlme_ext.c of the 88x2bu source).

On both RTL8822BU and RTL8821CU I measured the high queue draining at
roughly 3 frames per DTIM with BIT_TCR_UPDATE_HGQMD set, instead of
the whole buffered burst, so the burst fetch of 076f786a0ae1 does not
seem to engage on these chips. A never-terminated MORE_DATA chain is
my best guess for why, but that is a guess; whether the hardware
needs the 0 to conclude a burst is something only the firmware
documentation can answer.

Thanks,
Mehmet