Re: [PATCH 1/2] wifi: mac80211: introduce ieee80211_refresh_tx_agg_session_timer()

[Ryder Lee <Ryder.Lee@mediatek.com>]

On Fri, 2023-02-17 at 19:53 +0100, Johannes Berg wrote:
> On Fri, 2023-02-17 at 18:43 +0000, Ryder Lee wrote:
> > On Fri, 2023-02-17 at 19:01 +0100, Johannes Berg wrote:
> > > On Sat, 2023-02-18 at 01:50 +0800, Ryder Lee wrote:
> > > > This allows low level drivers to refresh the tx agg session
> > > > timer,
> > > > based on
> > > > querying stats from the firmware usually. Especially for some
> > > > mt76
> > > > devices
> > > > support .net_fill_forward_path would bypass mac80211, which
> > > > leads
> > > > to tx BA
> > > > session timeout for certain clients.
> > > > 
> > > 
> > > Does it even matter? We could just request sessions without a
> > > timeout
> > > in
> > > the first place.
> > > 
> > 
> > I think we're already. Our main issue is performance periodically
> > drops
> > every few seconds when .net_fill_forward_path is enabled. Wireless
> > client have normal 500+ Mb/s iperf3 download speed for several
> > seconds.
> > Then it drops less than 100 Mb/s for several seconds. Then
> > everything
> > repeats. Issue occurs only on certain clients. (i.e. Intel cards
> > AX200,
> > AX1675, Advanced-N 6235 in Win11)
> > 
> 
> Strange. But how does this patch do anything about it, that should be
> completely client agnostic?
> 
> 

Since there's no any keep alive packet being received by host stack,
leads to mac80211 destrory BA sesion.

Ax200 series needs to update timer for each 5s period to maintain ba
session. We originally did this to workaround issue, but obviouly this
hack will not be accepted upstream, since it effectively completely
disables the session expiry timer without removing the code.

--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -568,10 +568,9 @@ static void
sta_tx_agg_session_timer_expired(struct timer_list *t)
 	}
 
 	timeout = tid_tx->last_tx + TU_TO_JIFFIES(tid_tx->timeout);
-	if (time_is_after_jiffies(timeout)) {
-		mod_timer(&tid_tx->session_timer, timeout);
-		return;
-	}
+	/* remove timerout handle for ax210 interoperability issue */
+	mod_timer(&tid_tx->session_timer, timeout);
+	return;

I'm not sure if there's a better way to fix this though.

Ryder