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

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2026-02-17 at 14:05 +0100, Remi Pommarel wrote:
> On Tue, Feb 17, 2026 at 12:30:08PM +0100, Johannes Berg wrote:
> > On Tue, 2026-02-17 at 11:36 +0100, Remi Pommarel wrote:
> > > Commit f89e07d4cf26 ("mac80211: agg-rx: refuse ADDBA Request with timeout
> > > update") added a check to fail when ADDBA update would change the
> > > timeout param.
> > > 
> > > This param is kept in tid_ampdu_rx context which is only allocated on HW
> > > that do not set SUPPORTS_REORDERING_BUFFER. Because the timeout check
> > > was done regardless of this param, ADDBA update always failed on those
> > > HW.
> > 
> > Seems like a legit problem, but
> > 
> > > Fix this by only checking tid_ampdu_rx->timeout only when
> > > SUPPORTS_REORDERING_BUFFER is not set.
> > 
> > that doesn't seem right? Especially the way you implemented it, it won't
> > even respond at all when it's an update and SUPPORTS_REORDERING_BUFFER
> > is set.
> 
> I could be wrong but I think the patch format here make it difficult to
> read. If it's an update and SUPPORTS_REORDERING_BUFFER is set, the
> following "if" in the code (not fully visible in the diff here) will end
> calling drv_ampdu_action().

Yes, but it will be IEEE80211_AMPDU_RX_START which isn't really intended
by the state machine/API between mac80211/driver. So that doesn't seem
right.

> That is another way of fixing that yes, but the question here is, don't
> we want the driver to decide if it wants to support timeout update ?

Not really, there's no driver that _can_ to support it now, and it seems
unlikely any driver would _want_ to support it.

And if so it should probably not be IEEE80211_AMPDU_RX_START but rather
some new IEEE80211_AMPDU_RX_UPDATE thing. But I don't think any driver
would implement it, I don't see why anyone would want to, this flow is
in the spec but never really used.

johannes