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

[Remi Pommarel <repk@triplefau.lt>]

On Tue, Feb 17, 2026 at 02:59:34PM +0100, Johannes Berg wrote:
> 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 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?

-- 
Remi