Re: [RFC PATCH] wifi: ath9k: clear destination mask for non-aggregate frames

[Toke Høiland-Jørgensen]

Tomas Tonhauser <t.tonhauser@xxxxxxxx> writes:

> Frames to stations without an A-MPDU session (e.g. non-WMM/non-HT
> clients) use the non-aggregate path. If the hardware returns such a
> frame with ATH9K_TXERR_FILT, ath_tx_complete_buf() reports it to
> mac80211 as IEEE80211_TX_STAT_TX_FILTERED. For a station that is not
> in powersave, ieee80211_handle_filtered_frame() retries it once and
> drops it if it is filtered again.
>
> The aggregate path re-queues filtered subframes on tid->retry_q and
> sets tid->clear_ps_filter, so they are retransmitted until they get
> through.
>
> The result is downlink loss to awake non-WMM stations, while WMM/HT
> stations on the same radio are unaffected. On a QCA9563 AP with
> ESP8266 clients this caused 20-45% ping loss, with two TXERR_FILT
> completions per lost frame and no XRETRY failures.
>
> Set CLRDMASK on every non-aggregate frame. This restores the
> behaviour from before commit 5519541d5a5f ("ath9k: fix powersave
> frame filtering/buffering in AP mode") for the non-aggregate path
> only. The trade-off is that non-aggregate frames already queued in
> hardware when a station enters powersave may be sent to the sleeping
> station instead of being filtered.
>
> Assisted-by: LLM
> Signed-off-by: Tomas Tonhauser <t.tonhauser@xxxxxxxx>
> ---
> RFC: is this direction acceptable, or is one of the alternatives
> below preferred?

Hmm, no, I don't think unconditionally clearing the filter is the right
thing to do. See below.

> Setup: TP-Link EAP225 v3 (QCA9563, 2.4 GHz), OpenWrt 25.12.5,
> backports 6.18.26. Clients: ESP8266 running Tasmota, modem sleep
> disabled, associated without WMM (legacy rates).
>
> Ping from the AP to one client, 30 packets, 14 lost:
>
>   station "tx failed":             +14
>   station "tx retries":            +2
>   ath9k/xmit BE "TXERR Filtered":  +27
>   ath9k/xmit BE "MPDUs XRetried":  +0
>
> A second run: 12 lost, Filtered +28, XRetried +0.
>
> Ruled out:
>  - Client powersave: PS_STA never set in debugfs station flags, and a
>    monitor capture showed no client frames with the PM bit set.
>  - airtime_flags=0, aql_enable=0, nohwcrypt=1: no change.
>  - RF: signal -50..-64 dBm; a WMM/HT client on the same radio has
>    0 tx failed out of ~10k frames; the same ESP8266 hardware behind
>    an ath10k AP has no loss.
>
> With the patch: 500 pings to each of five ESP8266 clients, 0 lost.
>
> Open question: what sets the destination mask for stations that are
> awake and never hit XRETRY? 5519541d5a5f removed an unconditional
> CLRDMASK commented "needed for crypto errors", so the hardware may
> set it in other cases too.

This is a good question. Maybe Felix knows?

Regardless, of your alternatives:

> Alternatives:
>  1. Set CLRDMASK only when the station is not sleeping.
>  2. Re-queue filtered non-aggregate frames in ath9k for awake
>     stations, like the aggregate path does.
>  3. Allow more than one software retry in
>     ieee80211_handle_filtered_frame() for awake stations.

I think 2. makes the most sense. In fact, AFAICT, there is currently
*no* software retry logic for non-aggregated frames in ath9k? So you hit
it due to this weird filtering issue, but presumably random interference
can also just lead to losses if they exhaust the hardware retry chain.
But maybe this is not something that's noticeable because the hardware
retry chain is usually enough? Do you notice any errors/retries in your
AP stats after applying the fix (in normal usage, not just with a ping
test)?

Anyway, I don't think it's unreasonable to add the same retry logic to
non-aggregated frames that we have for aggregated ones, and the retry_q
seems like the simplest way to do that.

-Toke