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@xxxxxxxxxxxxxxxxxx>

Hi Ping-Ke,

thanks for the detailed answers, and for the ATIM window pointer - I ran
that experiment today. All numbers below are from the same bench setup
as before (RTL8822BU USB2 AP, dtim_period=2, one associated Windows
client in power save, ~40 broadcast/multicast frames per second
generated on the AP, free page count read at 0x240, nothing touching
the client during the runs).

> Have you confirmed the broadcast frames ate all of them?

Yes, three ways. The drain tracks the storm linearly (1803 -> 16 in
~105 s) and only while it runs. With an instrumented build I counted
the high queue draining ~3 frames per DTIM, which at dtim_period=2 is
~15 frames/s against ~40/s coming in. And with a test build that keeps
the same storm off the high queue (routed to the AC queues instead),
the count never leaves 1803 - same traffic, only the queue changed.

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

It makes a dramatic difference. Same storm, only REG_ATIMWND changed:

  0x02 (default)  pool 1803 -> 16 in ~105 s, pinned  (reproduced twice)
  0x04            pool never leaves 1803 over 180 s
  0x08            same, never drops
  0x10            same, never drops  (reproduced twice)

So the default 2 TU window is what limits the drain to ~3 frames per
DTIM, and already 4 TU drains faster than this storm fills.

One more observation worth recording: with the client *actively
pinging* the AP the pool still drains to 16 at the default window.
Windows dynamic power save dozes between packets, so a client that
looks perfectly alive keeps the dozing condition asserted. That matches
our field failure: the link looked healthy and yet the AP died.

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

Yes. In both runs the pool was back to 1803 about 60 s after the storm
stopped, and the reconnects between the runs above all succeeded. In
the field neither condition stops (the chatter is mDNS/SSDP from the
clients' own segment), which is why it presents as a permanent lockup.

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

Understood, I have dropped that idea.

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

I looked at how the existing users time the release. ath9k_htc can do
it on USB only because its firmware sends an SWBA event at beacon time
(WMI_SWBA_EVENTID) and the driver pulls the buffered frames from that
handler; rt2500usb has no such event and explicitly refuses to set the
flag for that reason (see the comment in rt2500usb_probe_hw_mode).

Interestingly, the firmware seems to already have the needed event: the
vendor driver enables a beacon-early C2H report through a bit in the
SET_PWR_MODE H2C (SET_H2CCMD_PWRMODE_PARM_BCN_EARLY_C2H_RPT, C2H id
0x1E), though it only uses it for TDLS channel switching, i.e. in a
station power-save context. Do you know whether that report also works
in AP mode on the USB chips, and fires early enough to pace
ieee80211_get_buffered_bc()? If it does, this becomes the clean
long-term solution and I would be happy to prototype it.

For what it is worth, the vendor driver does not use any beacon event
for bmc delivery on USB either: it parks at most one filtered burst in
the high queue, lets the hardware pace it out after the DTIM beacon,
and refills only when the queue reads back empty - in other words, its
real protection is a hard bound on high queue occupancy, which is what
patch 1/2 below brings to rtw88.

> Maybe, check bound first, and then filter ?

Agreed. Here is how I would combine the four knobs:

  - bound (patch 1/2): hard cap on how many bmc frames may sit on the
    high queue (token bucket refilled at what the default window
    drains, overflow goes out on the AC queues awake-style). This is
    the guarantee: the pool stays healthy under any storm, including
    the ARP/DHCP bursts the filter admits.
  - filter (patch 2/2): admit only ARP, EAPOL and DHCP to the high
    queue, matching the vendor driver's default. Ordinary chatter never
    reaches the beacon-paced path, so the bound rarely engages.
  - ATIM window: given the measurements, a moderate raise (0x04
    already drains this storm, 0x10 gives headroom) would add drain
    capacity as a complement. I left it out of the series for now
    because the queue stays unbounded either way and a wider window
    costs every PS station awake time after each DTIM - I assume that
    is why the vendor driver keeps it small and filters instead. If
    the firmware is fine with a larger window on these chips I can add
    it as a third patch; is 0x055A safe to raise across the USB chips?
  - mac80211 BC buffering: the long-term correct PS delivery, gated on
    a beacon-time event as above; follow-up work, not part of this
    series.

If the plan looks right I will send the two patches (based on rtw-next
with the acked v3 applied) in the next days.

Best regards,
Mehmet