[patch] wifi: mt76: mt7925: fix AMPDU state handling in mt7925_tx_check_aggr

[Sean Wang]

Hi, Quan

On Wed, Nov 26, 2025 at 1:04 AM Quan Zhou <quan.zhou at mediatek.com> wrote:
>
> Previously, the AMPDU state bit for a given TID was set before attempting
> to start a BA session, which could result in the AMPDU state being marked
> active even if ieee80211_start_tx_ba_session() failed. This patch changes
> the logic to only set the AMPDU state bit after successfully starting a BA
> session, ensuring proper synchronization between AMPDU state and BA session
> status.
>
> This fixes potential issues with aggregation state tracking and improves
> compatibility with mac80211 BA session management.
>
> Fixes: 44eb173bdd4f ("wifi: mt76: mt7925: add link handling in mt7925_txwi_free")
> Cc: stable at vger.kernel.org
>
> Signed-off-by: Quan Zhou <quan.zhou at mediatek.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt7925/mac.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mac.c b/drivers/net/wireless/mediatek/mt76/mt7925/mac.c
> index 871b67101976..80f1d738ec22 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7925/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7925/mac.c
> @@ -881,8 +881,9 @@ static void mt7925_tx_check_aggr(struct ieee80211_sta *sta, struct sk_buff *skb,
>         else
>                 mlink = &msta->deflink;
>
> -       if (!test_and_set_bit(tid, &mlink->wcid.ampdu_state))
> -               ieee80211_start_tx_ba_session(sta, tid, 0);
> +       if (!test_bit(tid, &mlink->wcid.ampdu_state) &&
> +           !ieee80211_start_tx_ba_session(sta, tid, 0))
> +               set_bit(tid, &mlink->wcid.ampdu_state);
>  }

It is a nice catch. The change works today but switching from
test_and_set_bit() to a separate test_bit() + set_bit() weakens the
atomicity that this state transition was originally designed to have.
It might not break right now, but it creates a subtle trap. Future
code may assume the transition is atomic and accidentally reenter BA
start path which can lead to hard to debug regression.
To keep the logic solid and aligned with the current design, the safer
approach is to use atomic acquire with a rollback.

if (!test_and_set_bit(tid, &mlink->wcid.ampdu_state)) {
    if (ieee80211_start_tx_ba_session(sta, tid, 0))
        clear_bit(tid, &mlink->wcid.ampdu_state);
}

This keeps semantics clear and avoids future maintenance landmines.

>
>  static bool
> --
> 2.45.2
>
>