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

[Felix Fietkau <nbd@nbd.name>]

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