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

[Ping-Ke Shih]

Mehmet Fide <mehmet.fide@xxxxxxxxx> wrote:
> Hi Ping-Ke, Bitterblue,
> 
> thanks for the ack on this one. It has not been applied yet, and before
> I post anything on top of it I would like your opinion, because I now
> have a case where the same lockup comes back with this patch in place.
> 
> What happens
> ============
> 
> The patch made the after-DTIM routing conditional, but nothing bounds
> that path. mac80211 sets IEEE80211_TX_CTL_SEND_AFTER_DTIM on every bmc
> frame while a single station is dozing, so one client in power save is
> enough to put all broadcast and multicast traffic back on the high
> queue. The chip drains that queue at beacon pace while the frames
> occupy the shared TX page pool, so ordinary chatter such as mDNS
> exhausts the pool and with it every other transmit, including the auth
> and assoc responses other clients need. The AP still beacons and looks
> alive, but nothing can join until the device is rebooted. That is what
> our field report looked like.
> 
> Measured on an AM62 based AP with an RTL8822BU, one associated client
> in power save, and 40 broadcast frames per second generated on the AP
> itself. Free page count at 0x240:
> 
>   t=0     1803
>   t=30s   1148
>   t=60s    716
>   t=90s     16   <- 0.9 % left
>   ...       16   pinned for as long as the traffic runs

Have you confirmed the broadcast frames ate all of them?

The HIQ packets only send out right after beacon within ATIM
window controlled by REG_ATIMWND (0x055A). Can you try to
enlarge the size to see if it will be different?

> 
> and at that point dmesg starts printing "error beacon valid" and
> "failed to download drv rsvd page". When the client wakes or the
> traffic stops the pool recovers; in the field both conditions persist,
> so the AP stays dead.

If you stop 40 broadcast frames per second, will AP become available?

> 
> How to reproduce
> ================
> 
>   - any rtw88 USB device as an AP, one client associated
>   - put the client into power save (a phone with the screen off will
>     do; on Windows set "Wireless Adapter Settings -> Power Saving Mode"
>     to maximum and reconnect)
>   - from the AP itself send roughly 40 multicast or broadcast packets
>     per second, for example in a loop:
>       socat -u - UDP4-DATAGRAM:224.0.0.251:5353,ttl=1
>   - watch the free page count:
>       echo "0x240 4" > /sys/kernel/debug/ieee80211/phyX/rtw88/read_reg
>       cat /sys/kernel/debug/ieee80211/phyX/rtw88/read_reg
> 
> Do not ping the client while watching. That wakes it, mac80211 flushes
> the queue and the pool recovers, which is what hid the problem from me
> in the first run.
> 
> What I would like to do
> =======================
> 
> Mirror the vendor driver. It never lets chatter into the hardware high
> queue: xmitframe_hiq_filter() in core/rtw_xmit.c admits only ARP, EAPOL
> and DHCP (CONFIG_RTW_HIQ_FILTER, default "allow special"), everything
> else leaves on its access category queue. With the same rule in rtw88
> the pool stays at 1803 in the test above, a DHCP flood still takes the
> after-DTIM path, so the filter is selective rather than a blanket
> disable, and plain join/ping cycling without power save is unchanged
> (10/10 here).
> 
> The trade-off is that a dozing station can miss non-special multicast,
> since we do not buffer it in software the way the vendor does.

Is it possible to declare IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING and
call ieee80211_get_buffered_bc() to get the BC packets to send?

> 
> Before I post a patch:
> 
>   1. Is such a filter acceptable as the fix, or would you rather see a
>      hard bound on how many bmc frames may sit on the high queue, which
>      would also cover an ARP or DHCP broadcast storm?

I think it depends on which kind of packets get lost we can accept. 

The vendor driver considers ARP, EAPOL and DHCP as important ones, because
user experience becomes bad if they get lost, but actually they have
their own re-transmit.

Consider multicast streaming, I guess it has error correction (like FEC),
so maybe it has more tolerant to handle packet loss. (I'm not familiar with
streaming, and no much experience with playing multicast streaming).

> 
>   2. Or both, filter first and bound on top?

With the filter, I think it will not reason bound, because there will not
be much that kind of packets. Maybe, check bound first, and then filter ?

> 
>   3. Is there a way to learn the DTIM boundary on USB that I have
>      missed? With one the driver could hand the chip a single burst per
>      DTIM and the pool could not run dry at all.

I think this is not possible, because USB needs time to transmit packets 
from host to WiFi card, and then it needs to wait for next DTIM. Though
driver can prepare packets earlier, but it will be inaccurate 
(still buffered in hardware HIQ).


As above, we have ATIM window, BC buffer in mac80211, HIQ filter,
and HIQ bound. But I don't have clear picture how to combine them to
deal with this case. Since you have spent time to dig the problem,
you may have more sense and better idea. :)