[PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Manish Dharanenthiran]

From: Hari Naraayana Desikan Kannan <hnaraaya@qti.qualcomm.com>

Subsequent ADDBA requests from a non-AP MLD can be rejected with status
0x0025 (Request declined), preventing BA establishment. This happens
because mac80211 checks the per-TID A-MPDU session's tid_rx pointer to
detect timeout changes on ADDBA updates. For drivers that set
SUPPORT_REORDERING_BUFFER, the reordering state is owned by the driver
and tid_rx will be NULL, causing mac80211 to incorrectly decline the
update.

This can occur during MLO link reconfiguration, where a non-AP MLD may
remove one of its links while a Block Ack (BA) session is active. After
the link is removed, ADDBA update requests sent on the remaining links
are rejected, preventing BA establishment.

Fix this by calling drv_ampdu_action() on ADDBA updates when
SUPPORT_REORDERING_BUFFER is set, so the driver can accept or reject the
update based on its capabilities and allow BA to be established on the
remaining links.

Signed-off-by: Hari Naraayana Desikan Kannan <hnaraaya@qti.qualcomm.com>
Signed-off-by: Manish Dharanenthiran <manish.dharanenthiran@oss.qualcomm.com>
---
Note: A similar fix has been proposed in [1]. This patch also fixes the
issue mentioned there. The difference in approach is to keep a similar
flow when the dialog_token matches. Previously only the timeout value
is checked, updated that to check the timeout only for the hardware that
doesn't set SUPPORT_REORDERING_BUFFER. In [1], it was changed to invoke
driver unconditionally when SUPPORT_REORDERING_BUFFER is set.

https://lore.kernel.org/all/5806bab7e46506d3c300ab4eb66989d42936aeb0.1771323902.git.repk@triplefau.lt/
---
 net/mac80211/agg-rx.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 0140ac826b23..20c849f7930e 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -376,6 +376,23 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta,
 			ht_dbg_ratelimited(sta->sdata,
 					   "updated AddBA Req from %pM on tid %u\n",
 					   sta->sta.addr, tid);
+			/*
+			 * For drivers with SUPPORTS_REORDERING_BUFFER set, let
+			 * the driver handle the BA update, as it manages the BA
+			 * state.
+			 */
+			if (ieee80211_hw_check(&local->hw,
+					       SUPPORTS_REORDERING_BUFFER)) {
+				ret = drv_ampdu_action(local, sta->sdata,
+						       &params);
+				ht_dbg_ratelimited(sta->sdata,
+						   "Updated AddBA Req result %d\n",
+						   ret);
+				if (!ret)
+					status = WLAN_STATUS_SUCCESS;
+				goto end;
+			}
+
 			/* We have no API to update the timeout value in the
 			 * driver so reject the timeout update if the timeout
 			 * changed. If it did not change, i.e., no real update,

---
base-commit: dbd94b9831bc52a1efb7ff3de841ffc3457428ce
change-id: 20260330-addba-req-0673c746c9d5

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Johannes Berg]

On Wed, 2026-04-15 at 12:21 +0530, Manish Dharanenthiran wrote:
> From: Hari Naraayana Desikan Kannan <hnaraaya@qti.qualcomm.com>
> 
> Subsequent ADDBA requests from a non-AP MLD can be rejected with status
> 0x0025 (Request declined), preventing BA establishment. This happens
> because mac80211 checks the per-TID A-MPDU session's tid_rx pointer to
> detect timeout changes on ADDBA updates. For drivers that set
> SUPPORT_REORDERING_BUFFER, the reordering state is owned by the driver
> and tid_rx will be NULL, causing mac80211 to incorrectly decline the
> update.
> 
> This can occur during MLO link reconfiguration, where a non-AP MLD may
> remove one of its links while a Block Ack (BA) session is active. After
> the link is removed, ADDBA update requests sent on the remaining links
> are rejected, preventing BA establishment.
> 
> Fix this by calling drv_ampdu_action() on ADDBA updates when
> SUPPORT_REORDERING_BUFFER is set, so the driver can accept or reject the
> update based on its capabilities and allow BA to be established on the
> remaining links.

Pretty sure this will break drivers, particularly iwlwifi, if you don't
fix those first.

> Note: A similar fix has been proposed in [1]. This patch also fixes the
> issue mentioned there. The difference in approach is to keep a similar
> flow when the dialog_token matches. Previously only the timeout value
> is checked, updated that to check the timeout only for the hardware that
> doesn't set SUPPORT_REORDERING_BUFFER. In [1], it was changed to invoke
> driver unconditionally when SUPPORT_REORDERING_BUFFER is set.
> 
> https://lore.kernel.org/all/5806bab7e46506d3c300ab4eb66989d42936aeb0.1771323902.git.repk@triplefau.lt/
> 
What was wrong with the approach we discussed there, other than nobody
implementing it?

johannes

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Manish Dharanenthiran]

On 4/27/2026 3:30 PM, Johannes Berg wrote:
> On Wed, 2026-04-15 at 12:21 +0530, Manish Dharanenthiran wrote:
>> From: Hari Naraayana Desikan Kannan <hnaraaya@qti.qualcomm.com>
>>
>> Subsequent ADDBA requests from a non-AP MLD can be rejected with status
>> 0x0025 (Request declined), preventing BA establishment. This happens
>> because mac80211 checks the per-TID A-MPDU session's tid_rx pointer to
>> detect timeout changes on ADDBA updates. For drivers that set
>> SUPPORT_REORDERING_BUFFER, the reordering state is owned by the driver
>> and tid_rx will be NULL, causing mac80211 to incorrectly decline the
>> update.
>>
>> This can occur during MLO link reconfiguration, where a non-AP MLD may
>> remove one of its links while a Block Ack (BA) session is active. After
>> the link is removed, ADDBA update requests sent on the remaining links
>> are rejected, preventing BA establishment.
>>
>> Fix this by calling drv_ampdu_action() on ADDBA updates when
>> SUPPORT_REORDERING_BUFFER is set, so the driver can accept or reject the
>> update based on its capabilities and allow BA to be established on the
>> remaining links.
> 
> Pretty sure this will break drivers, particularly iwlwifi, if you don't
> fix those first.
> 


With link reconfiguration, not handling the update requests sent on the 
remaining links causes throughput drop, that's why went with handling 
the update requests even-though the session is valid.
Please let us know if we need to clear the old session and then proceed 
with the update request to avoid breaking the other drivers.
>> Note: A similar fix has been proposed in [1]. This patch also fixes the
>> issue mentioned there. The difference in approach is to keep a similar
>> flow when the dialog_token matches. Previously only the timeout value
>> is checked, updated that to check the timeout only for the hardware that
>> doesn't set SUPPORT_REORDERING_BUFFER. In [1], it was changed to invoke
>> driver unconditionally when SUPPORT_REORDERING_BUFFER is set.
>>
>> https://lore.kernel.org/all/5806bab7e46506d3c300ab4eb66989d42936aeb0.1771323902.git.repk@triplefau.lt/
>>
> What was wrong with the approach we discussed there, other than nobody
> implementing it?
> 
> johannes

Not a major different between the other change, thought of keeping the 
similar version that we validated in the internal tree.

Regards
Manish Dharanenthiran

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Johannes Berg]

Hi,

Sorry, I've definitely fallen behind on some topics.

> With link reconfiguration, not handling the update requests sent on the 
> remaining links causes throughput drop, that's why went with handling 
> the update requests even-though the session is valid.

I'm not sure I see the relation right now, how does link reconfiguration
relate to AddBA re-negotiation? Are there implementations that
necessarily combine the two somehow?

> Please let us know if we need to clear the old session and then proceed 
> with the update request to avoid breaking the other drivers.

I'm not sure what you're trying to achieve, so I'm not entirely sure
what to say. I thought we were starting from "we should accept no-op
updates".

> > > Note: A similar fix has been proposed in [1]. This patch also fixes the
> > > issue mentioned there. The difference in approach is to keep a similar
> > > flow when the dialog_token matches. Previously only the timeout value
> > > is checked, updated that to check the timeout only for the hardware that
> > > doesn't set SUPPORT_REORDERING_BUFFER. In [1], it was changed to invoke
> > > driver unconditionally when SUPPORT_REORDERING_BUFFER is set.
> > > 
> > > https://lore.kernel.org/all/5806bab7e46506d3c300ab4eb66989d42936aeb0.1771323902.git.repk@triplefau.lt/
> > > 
> > What was wrong with the approach we discussed there, other than nobody
> > implementing it?
> > 
> > johannes
> 
> Not a major different between the other change, thought of keeping the 
> similar version that we validated in the internal tree.

I don't think it _is_ that similar though? Or maybe it is, but I didn't
think the other patch was correct (either).

Basically I'm concerned that calling drv_ampdu_action() with
IEEE80211_AMPDU_RX_START when the session is already active may break
things depending on the driver.

If you think there's a need to _really_ update parameters, then we
should probably add a IEEE80211_AMPDU_RX_UPDATE operation, and implement
it in drivers, and of course in some drivers that might just be
equivalent to IEEE80211_AMPDU_RX_START (though that seems unlikely in
general, given you may have to allocate reorder buffers etc.)

Regardless of that, if you think we need to accept no-op updates (I
don't think we've seen the need so far), then the code shouldn't really
(need to) call the driver at all, since things don't really change. For
that I'd argue we should go with the approached discussed in the thread
of [1], but not really that change itself.

[1] https://lore.kernel.org/all/5806bab7e46506d3c300ab4eb66989d42936aeb0.1771323902.git.repk@triplefau.lt/

I'm perfectly willing to believe the need for both, but I'd argue that
 - no-op updates can be handled by mac80211 entirely by itself, as
   discussed in that thread, just someone needs to implement it
 - updates with changes need a separate call to the driver, which might
   fail (either because of the change, or because it's not implemented)

As is, I don't think the code is safe in general. It may be that you
validated it against your driver, but then I'd argue that it worked for
your driver only because a new IEEE80211_AMPDU_RX_UPDATE would be
equivalent to IEEE80211_AMPDU_RX_START for that driver for some reason,
not because there's a general reason that this should work.

johannes

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Manish Dharanenthiran]

On 5/7/2026 11:56 PM, Johannes Berg wrote:
> Hi,
> 
> Sorry, I've definitely fallen behind on some topics.
> 
No problem :)

>> With link reconfiguration, not handling the update requests sent on the
>> remaining links causes throughput drop, that's why went with handling
>> the update requests even-though the session is valid.
> 
> I'm not sure I see the relation right now, how does link reconfiguration
> relate to AddBA re-negotiation? Are there implementations that
> necessarily combine the two somehow?
> 

No, there is no implementations that combine these two, but there are 
cases where the update AddBA request can be received from the station 
with link reconfiguration.

For instance, if a station associated in 2 GHz, later with link 
reconfiguration station can either move to 5 GHz or it can add 5 GHz (as 
MLD). Station then can send a AddBA request to update the window size or 
other related parameters. Current change tries to address this by 
treating the request as START and ath12k driver internally takes care of 
updating the existing session. But, as you mentioned, we feel that it 
should be treated as an UPDATE instead of START.

>> Please let us know if we need to clear the old session and then proceed
>> with the update request to avoid breaking the other drivers.
> 
> I'm not sure what you're trying to achieve, so I'm not entirely sure
> what to say. I thought we were starting from "we should accept no-op
> updates".
> 
>>>> Note: A similar fix has been proposed in [1]. This patch also fixes the
>>>> issue mentioned there. The difference in approach is to keep a similar
>>>> flow when the dialog_token matches. Previously only the timeout value
>>>> is checked, updated that to check the timeout only for the hardware that
>>>> doesn't set SUPPORT_REORDERING_BUFFER. In [1], it was changed to invoke
>>>> driver unconditionally when SUPPORT_REORDERING_BUFFER is set.
>>>>
>>>> https://lore.kernel.org/all/5806bab7e46506d3c300ab4eb66989d42936aeb0.1771323902.git.repk@triplefau.lt/
>>>>
>>> What was wrong with the approach we discussed there, other than nobody
>>> implementing it?
>>>
>>> johannes
>>
>> Not a major different between the other change, thought of keeping the
>> similar version that we validated in the internal tree.
> 
> I don't think it _is_ that similar though? Or maybe it is, but I didn't
> think the other patch was correct (either).
> 
> Basically I'm concerned that calling drv_ampdu_action() with
> IEEE80211_AMPDU_RX_START when the session is already active may break
> things depending on the driver.
> 
> If you think there's a need to _really_ update parameters, then we
> should probably add a IEEE80211_AMPDU_RX_UPDATE operation, and implement
> it in drivers, and of course in some drivers that might just be
> equivalent to IEEE80211_AMPDU_RX_START (though that seems unlikely in
> general, given you may have to allocate reorder buffers etc.)
> 
Agree.

> Regardless of that, if you think we need to accept no-op updates (I
> don't think we've seen the need so far), then the code shouldn't really
> (need to) call the driver at all, since things don't really change. For
> that I'd argue we should go with the approached discussed in the thread
> of [1], but not really that change itself.
> 
> [1] https://lore.kernel.org/all/5806bab7e46506d3c300ab4eb66989d42936aeb0.1771323902.git.repk@triplefau.lt/
> 
We believe that a no-op update is not required (or at-least we couldn't 
think of a case in which that is actually needed) as there can might be 
an actual change in the subsequent AddBA request.

> I'm perfectly willing to believe the need for both, but I'd argue that
>   - no-op updates can be handled by mac80211 entirely by itself, as
>     discussed in that thread, just someone needs to implement it
>   - updates with changes need a separate call to the driver, which might
>     fail (either because of the change, or because it's not implemented)
> 
> As is, I don't think the code is safe in general. It may be that you
> validated it against your driver, but then I'd argue that it worked for
> your driver only because a new IEEE80211_AMPDU_RX_UPDATE would be
> equivalent to IEEE80211_AMPDU_RX_START for that driver for some reason,
> not because there's a general reason that this should work.
> 
> johannes

Yes, based on the case that we encountered, IEEE80211_AMPDU_RX_UPDATE is 
the right approach for the handling those requests safely for other 
driver(s) as well instead of relying on IEEE80211_AMPDU_RX_START

Regards
Manish Dharanenthiran

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Johannes Berg]

On Mon, 2026-05-11 at 11:56 +0530, Manish Dharanenthiran wrote:
> No, there is no implementations that combine these two, but there are 
> cases where the update AddBA request can be received from the station 
> with link reconfiguration.
> 
> For instance, if a station associated in 2 GHz, later with link 
> reconfiguration station can either move to 5 GHz or it can add 5 GHz (as 
> MLD). Station then can send a AddBA request to update the window size or 
> other related parameters.

Ah, well, OK - technically an implementation can do that all the time
(and technically we can refuse it all the time), but I guess then that
some implementations do it with link reconfiguration, and also don't
like the refusal :)

> 
> We believe that a no-op update is not required (or at-least we couldn't 
> think of a case in which that is actually needed) as there can might be 
> an actual change in the subsequent AddBA request.

I just think that once we require an UPDATE call from the driver, that
raises the question of whether we should even call it for a no-op. This
seems a bit strange? And if we don't then we wouldn't require it for no-
ops either, which is probably generally good for drivers that don't
(immediately) implement the UPDATE.

johannes

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Manish Dharanenthiran]

On 5/11/2026 2:16 PM, Johannes Berg wrote:
> On Mon, 2026-05-11 at 11:56 +0530, Manish Dharanenthiran wrote:
>> No, there is no implementations that combine these two, but there are
>> cases where the update AddBA request can be received from the station
>> with link reconfiguration.
>>
>> For instance, if a station associated in 2 GHz, later with link
>> reconfiguration station can either move to 5 GHz or it can add 5 GHz (as
>> MLD). Station then can send a AddBA request to update the window size or
>> other related parameters.
> 
> Ah, well, OK - technically an implementation can do that all the time
> (and technically we can refuse it all the time), but I guess then that
> some implementations do it with link reconfiguration, and also don't
> like the refusal :)
> 
Yes :)

>>
>> We believe that a no-op update is not required (or at-least we couldn't
>> think of a case in which that is actually needed) as there can might be
>> an actual change in the subsequent AddBA request.
> 
> I just think that once we require an UPDATE call from the driver, that
> raises the question of whether we should even call it for a no-op. This
> seems a bit strange? And if we don't then we wouldn't require it for no-
> ops either, which is probably generally good for drivers that don't
> (immediately) implement the UPDATE.
> 
> johannes

Got it! Then, we should store the parameters differently as you 
mentioned in [1], which can be used to identify the no-op.

[1] 
https://lore.kernel.org/all/ab5b9eb76e4a94745c7cf1bfa886f067618a54b6.camel@sipsolutions.net/#t

Even-then, if there is a actual change, it goes to invoke the UPDATE. 
For the driver(s) which didn't implement the UPDATE yet, should we use 
additional flags to notify the UPDATE support or returning a failure 
from driver should be suffice?

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Johannes Berg]

On Mon, 2026-05-11 at 15:40 +0530, Manish Dharanenthiran wrote:
> 
> Even-then, if there is a actual change, it goes to invoke the UPDATE. 
> For the driver(s) which didn't implement the UPDATE yet, should we use 
> additional flags to notify the UPDATE support or returning a failure 
> from driver should be suffice?

I hope drivers would refuse unknown operations, so I think returning a
failure is fine. We can quickly review the drivers that handle this to
make sure, but I'd prefer not to have a feature flag.

johannes

Re: [PATCH wireless-next] wifi: mac80211: Fix ADDBA request rejection after MLD link removal

[Manish Dharanenthiran]

On 5/11/2026 3:41 PM, Johannes Berg wrote:
> On Mon, 2026-05-11 at 15:40 +0530, Manish Dharanenthiran wrote:
>>
>> Even-then, if there is a actual change, it goes to invoke the UPDATE.
>> For the driver(s) which didn't implement the UPDATE yet, should we use
>> additional flags to notify the UPDATE support or returning a failure
>> from driver should be suffice?
> 
> I hope drivers would refuse unknown operations, so I think returning a
> failure is fine. We can quickly review the drivers that handle this to
> make sure, but I'd prefer not to have a feature flag.
> 
> johannes

Quickly checked the drivers which sets 'SUPPORTS_REORDERING_BUFFER',

  - ath11k/ath12k drivers silently returns a failure if the 'op' is not 
handled.

  - Intel drivers do a 'WARN_ON_ONCE(1)' as default. Would need a change 
to avoid calling warning for UPDATE operation.

  - And Mediatek drivers return success as default. (I guess, this 
should be fine, but would request comment from them about keeping this 
behavior for UPDATE operation)