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

[Remi Pommarel <repk@triplefau.lt>]

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().

> 
> Seems we perhaps just need to store the timeout elsewhere?
> 

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 ?

> > @@ -374,14 +383,6 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
> >  			goto end;
> >  		}
> >  
> > -		ht_dbg_ratelimited(sta->sdata,
> > -				   "unexpected AddBA Req from %pM on tid %u\n",
> > -				   sta->sta.addr, tid);
> > -
> > -		/* delete existing Rx BA session on the same tid */
> > -		__ieee80211_stop_rx_ba_session(sta, tid, WLAN_BACK_RECIPIENT,
> > -					       WLAN_STATUS_UNSPECIFIED_QOS,
> > -					       false);
> >  	}
> 
> Also, nit, but this leaves a blank line at the end of the block.

Sure will remove that if we finally decide to keep the fix as is.

Thanks for the review.

-- 
Remi