2022-06-28 15:16:31

by Issam Hamdi

Subject: [PATCH] wifi: mac80211: fix get inactive time for station mode

In station mode, the value last_rx is not updated often.
Therefore the calculation of the inactive time will be wrong (will give
values between 1000 ms to 30000 ms).
To fix this add the update of "sta->deflink.rx_stats.last_rx" for
station mode in the response of the packets process
(ieee80211_rx_h_sta_process).

Signed-off-by: Issam Hamdi <[email protected]>
---
net/mac80211/rx.c | 3 +++
net/mac80211/sta_info.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index d017ad14d7db..6ce899b18614 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1756,6 +1756,9 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
sta->deflink.rx_stats.last_rate = sta_stats_encode_rate(status);
}

+ if (rx->sdata->vif.type == NL80211_IFTYPE_STATION)
+ sta->deflink.rx_stats.last_rx = jiffies;
+
sta->deflink.rx_stats.fragments++;

u64_stats_update_begin(&rx->sta->deflink.rx_stats.syncp);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 014032369994..88a9f81e7083 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2430,7 +2430,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
sinfo->connected_time = ktime_get_seconds() - sta->last_connected;
sinfo->assoc_at = sta->assoc_at;
sinfo->inactive_time =
- jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta));
+ jiffies_to_msecs(abs(jiffies - ieee80211_sta_last_active(sta)));

if (!(sinfo->filled & (BIT_ULL(NL80211_STA_INFO_TX_BYTES64) |
BIT_ULL(NL80211_STA_INFO_TX_BYTES)))) {
--
2.30.2

2022-07-01 09:59:05

by Johannes Berg

Subject: Re: [PATCH] wifi: mac80211: fix get inactive time for station mode

On Tue, 2022-06-28 at 17:05 +0200, Issam Hamdi wrote:
> In station mode, the value last_rx is not updated often.
> Therefore the calculation of the inactive time will be wrong (will give
> values between 1000 ms to 30000 ms).
> To fix this add the update of "sta->deflink.rx_stats.last_rx" for
> station mode in the response of the packets process
> (ieee80211_rx_h_sta_process).

Not really sure I understand that.

> +++ b/net/mac80211/rx.c
> @@ -1756,6 +1756,9 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
> sta->deflink.rx_stats.last_rate = sta_stats_encode_rate(status);
> }
>
> + if (rx->sdata->vif.type == NL80211_IFTYPE_STATION)
> + sta->deflink.rx_stats.last_rx = jiffies;

Why the interface type check? It's a per-station thing, so why would it
not apply here for other types?

> sinfo->inactive_time =
> - jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta));
> + jiffies_to_msecs(abs(jiffies - ieee80211_sta_last_active(sta)));
>

That seems ... rather questionable.

johannes