[PATCH] wifi: mt76: mt7603: discard power-save loopbacks after wakeup

[PATCH] wifi: mt76: mt7603: discard power-save loopbacks after wakeup

[Cristian Papa]

A station wakeup drains its power-save queue once, while frames
redirected by the PSE can still be waiting on the MCU RX ring. Processing
those loopbacks after the wakeup queues them for an already awake station
and sets the driver's buffered indication again. The stale frames remain
queued until another service period or sleep/wake transition drains them.

Check ps_sleeping under ps_lock before buffering a loopback, and publish
the buffered indication under the same lock. The wakeup's state change
and queue drain use that lock too, so late frames cannot repopulate the
queue or leave a stale buffered indication after the wakeup.

Discard late frames rather than retransmitting their old sequence and
packet numbers after newer traffic or a wakeup BAR. This loses the late
frame; it does not recover delivery. Use the station's sleep state, not
the temporary hardware PS bypass used during U-APSD service periods.

Fixes: c8846e101502 ("mt76: add driver for MT7603E and MT7628/7688")
Assisted-by: LLM
Signed-off-by: Cristian Papa <pcristian292@gmail.com>
---
Based on Linux master 4056b3c9d3c92ea45955641dbafeb9b960e146ef.
The ps_sleeping field comes from mt76 commit 276db249c20a, corresponding
to Linux commit 636a883e2fee ("wifi: mt76: mt7603: restore hardware PS
buffering after a service period"). Older trees need that prerequisite.

Testing: extracted C functions from the unmodified base reproduce both a
late queue insertion and a buffered-notification race in a host harness.
The candidate passes these cases and controls for sleeping U-APSD clients,
frame formats, queue limit and malformed frames under ASan/UBSan. MMIO,
mac80211 services and locks are modeled. The changed source builds as a
MIPS object with W=1 and -Werror against Linux 6.18.44/backports 7.2.

Tested on MT7603E (TP-Link Archer XR500v, mt76 backported to Linux
6.18.44): a phone on the 2.4 GHz AP went through three 60 s screen-off/wake
cycles with a 1 Hz ping from the AP running throughout (about 8 minutes).
All 474 pings were answered, the phone never reassociated, and there were
no resets or kernel warnings. This exercises the power-save path under
traffic but does not demonstrate the late-loopback race itself, since no
counter records discarded late frames.

AI assistance: Codex reviewed the late-loopback path, proposed the change,
prepared the patch text, and implemented/ran host regression checks after
being asked to review pending power-save fixes and their upstream fit.

diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/dma.c b/drivers/net/wireless/mediatek/mt76/mt7603/dma.c
--- a/drivers/net/wireless/mediatek/mt76/mt7603/dma.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/dma.c
@@ -72,7 +72,6 @@
 	}
 
 	skb->priority = tid;
-	ieee80211_sta_set_buffered(sta, tid, true);
 
 	val = le32_to_cpu(txd[0]);
 	val &= ~(MT_TXD0_P_IDX | MT_TXD0_Q_IDX);
@@ -80,6 +79,13 @@
 	txd[0] = cpu_to_le32(val);
 
 	spin_lock_bh(&dev->ps_lock);
+	/* The wake-up may have drained psq before this loopback arrived. */
+	if (!msta->ps_sleeping) {
+		spin_unlock_bh(&dev->ps_lock);
+		goto free;
+	}
+
+	ieee80211_sta_set_buffered(sta, tid, true);
 	__skb_queue_tail(&msta->psq, skb);
 	if (skb_queue_len(&msta->psq) >= 64) {
 		skb = __skb_dequeue(&msta->psq);

Re: [PATCH] wifi: mt76: mt7603: discard power-save loopbacks after wakeup

[Felix Fietkau]

On 14.10.26 08:22, Cristian Papa wrote:
> A station wakeup drains its power-save queue once, while frames
> redirected by the PSE can still be waiting on the MCU RX ring. Processing
> those loopbacks after the wakeup queues them for an already awake station
> and sets the driver's buffered indication again. The stale frames remain
> queued until another service period or sleep/wake transition drains them.
> 
> Check ps_sleeping under ps_lock before buffering a loopback, and publish
> the buffered indication under the same lock. The wakeup's state change
> and queue drain use that lock too, so late frames cannot repopulate the
> queue or leave a stale buffered indication after the wakeup.
> 
> Discard late frames rather than retransmitting their old sequence and
> packet numbers after newer traffic or a wakeup BAR. This loses the late
> frame; it does not recover delivery. Use the station's sleep state, not
> the temporary hardware PS bypass used during U-APSD service periods.
Why did you decide to drop them instead of pushing them to immediate tx?

- Felix

Re: [PATCH] wifi: mt76: mt7603: discard power-save loopbacks after wakeup

[Cristian Papa]

On 20.10.26 11:48, Felix Fietkau wrote:
> On 14.10.26 08:22, Cristian Papa wrote:
>> Discard late frames rather than retransmitting their old sequence and
>> packet numbers after newer traffic or a wakeup BAR. This loses the late
>> frame; it does not recover delivery. Use the station's sleep state, not
>> the temporary hardware PS bypass used during U-APSD service periods.
> Why did you decide to drop them instead of pushing them to immediate tx?

Hi Felix, thanks for looking at it.

Because the looped-back frame keeps the TXD it was first queued with.
mt7603_mac_write_txwi() puts the 802.11 sequence number in the TXD with
SN_VALID, and with a hardware key it also takes the PN from key->tx_pn at
that point. mt7603_rx_loopback_skb() only rewrites Q_IDX, so a late frame
pushed to immediate TX would go out with its original SN and PN.

On wakeup, mt7603_sta_ps() sends the psq ahead of the newer traffic, so
the order holds there. A loopback that arrives after that drain is older
than frames that may already have gone out with newer SN/PN. With CCMP the
peer's replay check would drop it, in an aggregation session it can fall
behind the window after the newer frames or the BAR, and on an open link
it would arrive out of order. So I expected immediate TX to mostly spend
airtime on a frame the peer discards.

I haven't measured how late these loopbacks arrive, though. One that
arrives before the newer frames go out would still be delivered in order.
If you'd rather push them to immediate TX, I can send a v2 that does that.

Cristian

Re: [PATCH] wifi: mt76: mt7603: discard power-save loopbacks after wakeup

[Felix Fietkau]

On 20.10.26 18:15, Cristian Papa wrote:
> On 20.10.26 11:48, Felix Fietkau wrote:
>> On 14.10.26 08:22, Cristian Papa wrote:
>>> Discard late frames rather than retransmitting their old sequence and
>>> packet numbers after newer traffic or a wakeup BAR. This loses the late
>>> frame; it does not recover delivery. Use the station's sleep state, not
>>> the temporary hardware PS bypass used during U-APSD service periods.
>> Why did you decide to drop them instead of pushing them to immediate tx?
> 
> Hi Felix, thanks for looking at it.
> 
> Because the looped-back frame keeps the TXD it was first queued with.
> mt7603_mac_write_txwi() puts the 802.11 sequence number in the TXD with
> SN_VALID, and with a hardware key it also takes the PN from key->tx_pn at
> that point. mt7603_rx_loopback_skb() only rewrites Q_IDX, so a late frame
> pushed to immediate TX would go out with its original SN and PN.
> 
> On wakeup, mt7603_sta_ps() sends the psq ahead of the newer traffic, so
> the order holds there. A loopback that arrives after that drain is older
> than frames that may already have gone out with newer SN/PN. With CCMP the
> peer's replay check would drop it, in an aggregation session it can fall
> behind the window after the newer frames or the BAR, and on an open link
> it would arrive out of order. So I expected immediate TX to mostly spend
> airtime on a frame the peer discards.
> 
> I haven't measured how late these loopbacks arrive, though. One that
> arrives before the newer frames go out would still be delivered in order.
> If you'd rather push them to immediate TX, I can send a v2 that does that.
I'm not 100% sure which approach is the right one either way, I just 
wanted it considered.

Not rewriting SN and PN is intentional. If the frame with its SN was 
part of an active aggregation session, receiver side reordering should 
ensure that it arrives in the right order.

It all depends on whether the receiver has moved on and considers the 
frame lost, or if there is value in sending it to avoid the reorder 
timeout (which would cause some jitter). Either way, we should likely 
only allow immediate-tx if aggregation is enabled.

If you have some time, could you test, which variant works better?

Thanks,

- Felix