diff mbox series

[PATCH-v2,1/6] mac80211: Fix station rx-packets counters.

Message ID 20210324181441.13755-1-greearb@candelatech.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series [PATCH-v2,1/6] mac80211: Fix station rx-packets counters. | expand

Commit Message

Ben Greear March 24, 2021, 6:14 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

I noticed 'iw dev wlan6 station dump' showed almost no rx-packets
one one of my radios.  The rx-amsdu path did not appear to gather
any stats, and after code inspection, neither did the rx-data
handler.

Add common method to deal with these stats.  Verified in AX
and /a mode, stats look at least generally correct now.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/rx.c | 54 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 17 deletions(-)

Comments

Ben Greear April 1, 2021, 4:26 a.m. UTC | #1 
On 3/24/21 11:14 AM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> I noticed 'iw dev wlan6 station dump' showed almost no rx-packets
> one one of my radios.  The rx-amsdu path did not appear to gather
> any stats, and after code inspection, neither did the rx-data
> handler.
> 
> Add common method to deal with these stats.  Verified in AX
> and /a mode, stats look at least generally correct now.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---

> @@ -2706,6 +2727,8 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
>   	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
>   	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>   	__le16 fc = hdr->frame_control;
> +	ieee80211_rx_result rv;
> +	int orig_len = skb->len;
>   
>   	if (!(status->rx_flags & IEEE80211_RX_AMSDU))
>   		return RX_CONTINUE;
> @@ -2734,7 +2757,12 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
>   	if (is_multicast_ether_addr(hdr->addr1))
>   		return RX_DROP_UNUSABLE;
>   
> -	return __ieee80211_rx_h_amsdu(rx, 0);
> +	rv = __ieee80211_rx_h_amsdu(rx, 0);
> +	if ((rv == RX_QUEUED) && (rx->sta)) {
> +		struct ieee80211_sta_rx_stats *stats = &rx->sta->rx_stats;
> +		ieee80211_update_data_rx_stats(rx, stats, status, orig_len);
> +	}
> +	return rv;
>   }

I noticed this is buggy in several ways (potential use-after-free, bogus
status field).

I noticed too that upcoming changes changed some API that made later patches
in this series not apply cleanly..needed a bit of re-work.

Maybe best if I wait and rebase it against 5.13 when it is ready, unless
someone is actually interested enough in this to want to apply it earlier?

Thanks,
Ben
diff mbox series

Patch

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index eb8225209005..4a64c2183a27 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1713,6 +1713,27 @@  ieee80211_rx_h_uapsd_and_pspoll(struct ieee80211_rx_data *rx)
 	return RX_CONTINUE;
 }
 
+static void ieee80211_update_data_rx_stats(struct ieee80211_rx_data *rx,
+					   struct ieee80211_sta_rx_stats *stats,
+					   struct ieee80211_rx_status *status,
+					   int skb_len)
+{
+	stats->fragments++;
+	stats->packets++;
+	stats->last_rx = jiffies;
+	stats->last_rate = sta_stats_encode_rate(status);
+
+	/* The seqno index has the same property as needed
+	 * for the rx_msdu field, i.e. it is IEEE80211_NUM_TIDS
+	 * for non-QoS-data frames. Here we know it's a data
+	 * frame, so count MSDUs.
+	 */
+	u64_stats_update_begin(&stats->syncp);
+	stats->msdu[rx->seqno_idx]++;
+	stats->bytes += skb_len;
+	u64_stats_update_end(&stats->syncp);
+}
+
 static ieee80211_rx_result debug_noinline
 ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 {
@@ -2706,6 +2727,8 @@  ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	__le16 fc = hdr->frame_control;
+	ieee80211_rx_result rv;
+	int orig_len = skb->len;
 
 	if (!(status->rx_flags & IEEE80211_RX_AMSDU))
 		return RX_CONTINUE;
@@ -2734,7 +2757,12 @@  ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	if (is_multicast_ether_addr(hdr->addr1))
 		return RX_DROP_UNUSABLE;
 
-	return __ieee80211_rx_h_amsdu(rx, 0);
+	rv = __ieee80211_rx_h_amsdu(rx, 0);
+	if ((rv == RX_QUEUED) && (rx->sta)) {
+		struct ieee80211_sta_rx_stats *stats = &rx->sta->rx_stats;
+		ieee80211_update_data_rx_stats(rx, stats, status, orig_len);
+	}
+	return rv;
 }
 
 #ifdef CONFIG_MAC80211_MESH
@@ -2958,6 +2986,13 @@  ieee80211_rx_h_data(struct ieee80211_rx_data *rx)
 		mod_timer(&local->dynamic_ps_timer, jiffies +
 			  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
 
+	
+	if (rx->sta) {
+		struct ieee80211_sta_rx_stats *stats = &rx->sta->rx_stats;
+		struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(rx->skb);
+		ieee80211_update_data_rx_stats(rx, stats, status, rx->skb->len);
+	}
+
 	ieee80211_deliver_skb(rx);
 
 	return RX_QUEUED;
@@ -4400,12 +4435,6 @@  static bool ieee80211_invoke_fast_rx(struct ieee80211_rx_data *rx,
 		return true;
 	}
 
-	stats->last_rx = jiffies;
-	stats->last_rate = sta_stats_encode_rate(status);
-
-	stats->fragments++;
-	stats->packets++;
-
 	/* do the header conversion - first grab the addresses */
 	ether_addr_copy(addrs.da, skb->data + fast_rx->da_offs);
 	ether_addr_copy(addrs.sa, skb->data + fast_rx->sa_offs);
@@ -4416,18 +4445,9 @@  static bool ieee80211_invoke_fast_rx(struct ieee80211_rx_data *rx,
 
 	skb->dev = fast_rx->dev;
 
+	ieee80211_update_data_rx_stats(rx, stats, status, orig_len);
 	dev_sw_netstats_rx_add(fast_rx->dev, skb->len);
 
-	/* The seqno index has the same property as needed
-	 * for the rx_msdu field, i.e. it is IEEE80211_NUM_TIDS
-	 * for non-QoS-data frames. Here we know it's a data
-	 * frame, so count MSDUs.
-	 */
-	u64_stats_update_begin(&stats->syncp);
-	stats->msdu[rx->seqno_idx]++;
-	stats->bytes += orig_len;
-	u64_stats_update_end(&stats->syncp);
-
 	if (fast_rx->internal_forward) {
 		struct sk_buff *xmit_skb = NULL;
 		if (is_multicast_ether_addr(addrs.da)) {