diff mbox series

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

Message ID 20210324181441.13755-1-greearb@candelatech.com
State New
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

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com
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)) {