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@vger.kernel.org
Signed-off-by: Quan Zhou quan.zhou@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); }
static bool
Hi, Quan
On Wed, Nov 26, 2025 at 1:04 AM Quan Zhou quan.zhou@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@vger.kernel.org
Signed-off-by: Quan Zhou quan.zhou@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
linux-stable-mirror@lists.linaro.org