Re: [PATCH wireless] wifi: mac80211: Fix ADDBA update when HW supports reordering

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2026-02-26 at 16:49 +0100, Remi Pommarel wrote:
> On Mon, Feb 23, 2026 at 02:25:13PM +0100, Pablo MARTIN-GOMEZ wrote:
> > Hello,
> > 
> > On 23/02/2026 12:50, Johannes Berg wrote:
> > > On Sun, 2026-02-22 at 17:06 +0100, Remi Pommarel wrote:
> > > > > > That does make sense. However, if I understand correctly, it means that
> > > > > > even if we end up storing the timeout for drivers that support
> > > > > > reordering, a new IEEE80211_AMPDU_RX_UPDATE should still be introduced,
> > > > > > right?
> > > > > 
> > > > > I guess in order to do a no-op update that doesn't change the timeout,
> > > > > we could? But not sure it's all worth it?
> > > > 
> > > > I was going to say it would not be a no-op for a buf_size update but,
> > > > if I understand correctly, even when SUPPORTS_REORDERING_BUFFER is not
> > > > set the buf_size update is ignored completely and the reorder_buf is
> > > > not resized yet a successful addba response is sent. Don't we need to
> > > > check for buf_size change as well as timeout also?
> > > 
> > > I was going to say that I thought buf_size is not allowed to change, but
> > > (re)reading the spec doesn't seem to bear that out.
> > For once, the standard (802.11-2024) is really clear on this (10.25.2):
> > 
> > A block ack agreement may be modified by the originator by sending an
> > ADDBA Request frame (see 11.5.2, except that MLME-ADDBA primitives are
> > not used by the originator). All parameters of the agreement may be
> > modified except for the TID. If the request is not successful, the
> > existing agreement is not modified. If the request is successful, the
> > behavior is as if a DELBA frame for the block ack agreement had been
> > transmitted by the originator and received by the recipient immediately
> > prior to the ADDBA Request frame.
> > > 
> > > I guess we could just unconditionally reject any updates. I'm not sure
> > > now why we ever added the update in the first place. Perhaps some
> > > implementation was doing no-op updates and failing on negative
> > > responses?
> > > 
> > If the originator is compliant and trying to update, unconditionally
> > rejecing any update shouldn't have any side-effects. But if the
> > originator doesn't receive the ADDBA response, it is going to resend an
> > ADDBA request; for the originator, it is just a retry, but for the
> > recipient it's an update, so the originator is going to receive a non
> > success. And unless the originator sends a DELBA "by default" at some
> > point, it won't be able to create a new BA (for a given TID) with the
> > recipient.
> > [...]
> 
> I’m a bit concerned that ignoring all ADDBA updates might break STAs
> with finicky aggregation implementations. For example, I’ve observed an
> ath12k mesh point get stuck in a loop, repeatedly sending the same ADDBA
> request (with unchanged timeout or buffer size) only for us to keep
> reject it each time.

Sounds like you're both arguing for an "accept if no-op" rather than
unconditional reject, which seems fine to me. Just means storing the
values differently?

johannes