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

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

[Remi Pommarel]

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.

Fix this by only checking tid_ampdu_rx->timeout only when
SUPPORTS_REORDERING_BUFFER is not set.

Fixes: f89e07d4cf26 ("mac80211: agg-rx: refuse ADDBA Request with timeout update")
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
 net/mac80211/agg-rx.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 7da909d78c68..099a291723e6 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -353,7 +353,16 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
 	       buf_size, sta->sta.addr);
 
 	if (test_bit(tid, sta->ampdu_mlme.agg_session_valid)) {
-		if (sta->ampdu_mlme.tid_rx_token[tid] == dialog_token) {
+		if (sta->ampdu_mlme.tid_rx_token[tid] != dialog_token) {
+			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);
+		} else if (!ieee80211_hw_check(&local->hw, SUPPORTS_REORDERING_BUFFER)) {
 			struct tid_ampdu_rx *tid_rx;
 
 			ht_dbg_ratelimited(sta->sdata,
@@ -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);
 	}
 
 	if (ieee80211_hw_check(&local->hw, SUPPORTS_REORDERING_BUFFER)) {
-- 
2.52.0

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

[Johannes Berg]

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.

Seems we perhaps just need to store the timeout elsewhere?

> @@ -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.

johannes

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

[Remi Pommarel]

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

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

[Johannes Berg]

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

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

[Remi Pommarel]

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

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

[Pablo MARTIN-GOMEZ]

Hello,

On 17/02/2026 14:59, 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:
[...]
>> 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.
I've checked, and both mt76 and ath11k/ath12k drivers supports a BA 
update (but none of them supports the timeout though) through 
`IEEE80211_AMPDU_RX_START`. mt76 stop the session before starting one 
new (function `mt76_rx_aggr_start`) and ath11k/ath12k checks if a 
session is active to either modify it or create a new one (function 
`ath12k_dp_rx_peer_tid_setup`).
>
> johannes
>
Pablo MG

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

[Johannes Berg]

On Tue, 2026-02-17 at 15:38 +0100, Remi Pommarel wrote:
> 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?

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?

Pablo seems to have looked up that it _is_ supported - which I didn't
expect because it's not part of the API contract, so the drivers
implemented something that can't actually ever get hit? Seems odd. And
I'm pretty sure e.g. iwlwifi wouldn't support it.

But I basically also think it's not worth it in practice; why try to
support something that's never going to happen?

johannes

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

[Remi Pommarel]

On Tue, Feb 17, 2026 at 05:00:56PM +0100, Johannes Berg wrote:
> On Tue, 2026-02-17 at 15:38 +0100, Remi Pommarel wrote:
> > 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?
> 
> 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?

> Pablo seems to have looked up that it _is_ supported - which I didn't
> expect because it's not part of the API contract, so the drivers
> implemented something that can't actually ever get hit? Seems odd. And
> I'm pretty sure e.g. iwlwifi wouldn't support it.
> 
> But I basically also think it's not worth it in practice; why try to
> support something that's never going to happen?

Just to confirm, you mean that updating the timeout is not worth it, but
fixing this issue is still needed right?

-- 
Remi

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

[Johannes Berg]

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.

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?

> > Pablo seems to have looked up that it _is_ supported - which I didn't
> > expect because it's not part of the API contract, so the drivers
> > implemented something that can't actually ever get hit? Seems odd. And
> > I'm pretty sure e.g. iwlwifi wouldn't support it.
> > 
> > But I basically also think it's not worth it in practice; why try to
> > support something that's never going to happen?
> 
> Just to confirm, you mean that updating the timeout is not worth it, but
> fixing this issue is still needed right?

Right.

johannes

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

[Pablo MARTIN-GOMEZ]

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.
[...]
> johannes
Pablo MG

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

[Remi Pommarel]

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.

But I am, of course, unfortunately unable to reproduce that easily.

-- 
Remi

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

[Johannes Berg]

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