Re: [PATCH] wifi: ath9k: track PS-Poll responses in the BlockAck window

[Toke Høiland-Jørgensen]

Evan Severson <eseverson@xxxxxxxxx> writes:

> ath_tx_setup_buffer() takes a sequence number from the TID for every
> QoS data frame, but a frame only enters the BlockAck window when it
> carries IEEE80211_TX_CTL_AMPDU. Since commit 592234e941f1 ("wifi:
> mac80211: Fix iTXQ AMPDU fragmentation handling"), mac80211 sets that
> flag only when dequeuing from a TXQ. Frames that reach ath_tx_start()
> directly while a session is running, such as PS-Poll responses for a
> dozing station, now use up session sequence numbers without being
> tracked.
>
> Once seq_next is more than baw_size ahead of seq_start, every frame
> the TXQ hands out is outside the window. ath_tx_get_tid_subframe()
> puts it back on the retry queue and returns -EINPROGRESS on every
> attempt, nothing ever moves the window, and the AP stops sending
> unicast data to the station until the session is torn down or the
> station reassociates.
>
> Seen on a QCA9563 AP (OpenWrt) with a station that stayed in power
> save for several seconds under downlink load: the window was empty
> and seq_next was about 2170 ahead of seq_start. With this change, a
> 90 minute run saw a run of more than 3800 such frames and no stall.
>
> Flag unfragmented QoS data frames of an active session as A-MPDU where
> the sequence number is assigned, so they are tracked like any other
> session frame. Fragments stay unflagged, as mac80211 intends. A frame
> from ath_tx_start() cannot wait for the window, so if it is already
> outside it, send it untracked as before rather than set a bit beyond
> the window.
>
> A PS-Poll response is sent on the UAPSD queue, and ath_tx_start() held
> that queue's lock while it took the TID's sequence number. The sequence
> counter and the window belong to the lock of the TID's own queue, which
> the TXQ path holds when it adds to or advances the window, so take that
> lock for the TID work and the UAPSD queue's lock only to hand the frame
> to the hardware, as ath9k_release_buffered_frames() already does.
>
> Fixes: 592234e941f1 ("wifi: mac80211: Fix iTXQ AMPDU fragmentation handling")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Evan Severson <eseverson@xxxxxxxxx>
> ---
>
> Notes:
>     An alternative would be to restore IEEE80211_TX_CTL_AMPDU on
>     non-fragmented PS-Poll responses in mac80211, which would also cover
>     other drivers that track the BA window in software. Happy to rework it
>     that way if that's preferred.

Johannes, WDYT?

-Toke