| Message ID | 7c3f72eac1c34921cd84a462e60d71e125862152.1676616450.git.ryder.lee@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] wifi: mac80211: introduce ieee80211_refresh_tx_agg_session_timer() | expand |
On Fri, 2023-02-17 at 19:02 +0000, Ryder Lee wrote: > 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. > More specifically, the BA session relies on client side's Tx data to maintain ... but the point is mac80211 can't get any data after .net_fill_forward_path being enabled (HW path). So, we need a way to notify mac80211 to refresh last_tx... I think my patch is needed for that case. Ryder
On Mon, 2023-02-20 at 02:55 +0000, Ryder Lee wrote: > On Fri, 2023-02-17 at 19:02 +0000, Ryder Lee wrote: > > 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. > > > > More specifically, the BA session relies on client side's Tx data to Typo... I mean *our side*. Something like this ieee80211_8023_xmit() if (tid_tx->timeout) tid_tx->last_tx = jiffies; Ryder
Hi, > > > Since there's no any keep alive packet being received by host > > > stack, leads to mac80211 destrory BA sesion. > > > > > > > More specifically, the BA session relies on client side's Tx data to > > Typo... I mean *our side*. Something like this Sorry. I'm just totally confused - I thought the initiator only set the timeout, but I see now that it's negotiated and the actual value used is from the client. Which explains basically everything. johannes
On Tue, 2023-02-21 at 10:57 +0100, Johannes Berg wrote: > Hi, > > > > > Since there's no any keep alive packet being received by host > > > > stack, leads to mac80211 destrory BA sesion. > > > > > > > > > > More specifically, the BA session relies on client side's Tx data > > > to > > > > Typo... I mean *our side*. Something like this > > Sorry. I'm just totally confused - I thought the initiator only set > the > timeout, but I see now that it's negotiated and the actual value used > is > from the client. > > Which explains basically everything. > > Yup ... after accepting the AddBA Response we activated a timer, *resetting it after each frame that we send* - sta_tx_agg_session_timer_expired(). The .net_fill_forward_path() offloads tx path to HW, so it can only rely on other way to reset as mac80211 isn't aware of that. Ryder
diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 76a12bec71d5..920ea31620cd 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -5993,6 +5993,18 @@ void ieee80211_queue_delayed_work(struct ieee80211_hw *hw, struct delayed_work *dwork, unsigned long delay); +/** + * ieee80211_refresh_tx_agg_session_timer - Refresh a tx agg session timer. + * @sta: the station for which to start a BA session + * @tid: the TID to BA on. + * + * This function allows low level driver to refresh tx agg session timer + * to maintain BA session, the session level will still be managed by the + * mac80211. + */ +void ieee80211_refresh_tx_agg_session_timer(struct ieee80211_sta *sta, + u16 tid); + /** * ieee80211_start_tx_ba_session - Start a tx Block Ack session. * @sta: the station for which to start a BA session diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index f9514bacbd4a..3b651e7f5a73 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -554,6 +554,23 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) ieee80211_send_addba_with_timeout(sta, tid_tx); } +void ieee80211_refresh_tx_agg_session_timer(struct ieee80211_sta *pubsta, + u16 tid) +{ + struct sta_info *sta = container_of(pubsta, struct sta_info, sta); + struct tid_ampdu_tx *tid_tx; + + if (WARN_ON_ONCE(tid >= IEEE80211_NUM_TIDS)) + return; + + tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[tid]); + if (!tid_tx) + return; + + tid_tx->last_tx = jiffies; +} +EXPORT_SYMBOL(ieee80211_refresh_tx_agg_session_timer); + /* * After accepting the AddBA Response we activated a timer, * resetting it after each frame that we send.
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. Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> --- include/net/mac80211.h | 12 ++++++++++++ net/mac80211/agg-tx.c | 17 +++++++++++++++++ 2 files changed, 29 insertions(+)