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

[Ping-Ke Shih <pkshih@realtek.com>]

Mehmet Fide <mehmet.fide@gmail.com> wrote:
> > 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)

FYI. I checked vendor driver. Normally REG_ATIMWND sets 0xa or 0xc in AP mode
for chips.

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

As you saw it is for TDLS channel switching (timeslot sharing), I
don't think it can work in AP mode...

I will ask USB experts internally to see if there is an interrupt
to notify driver about TBTT (or beacon early). 

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

I think yes. The in AP mode, the value is 0xa or 0xc no matter which
HCI type is.

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

I will check internally to see if USB has an interrupt for TBTT.