diff mbox series

[v2] wifi: mac80211: Add support for management frames stats

Message ID 20250211202518.126305-1-muna.sinada@oss.qualcomm.com
State New
Headers show
Series [v2] wifi: mac80211: Add support for management frames stats | expand

Commit Message

Muna Sinada Feb. 11, 2025, 8:25 p.m. UTC 
From: Ramya Gnanasekar <ramya.gnanasekar@oss.qualcomm.com>

Currently there aren't statistics in mac80211 that keep track of the
management frames that are processed in both Tx and Rx. This type of
statistics is useful in tracking if management frames are successfully
transmitted or are dropped. These statistics are also needed to
provide information regarding how many management frames are received.

Add support to track number of management frames that are processed
in mac80211 driver and maintain counters for management Tx completion
status. These statistics are to be included as part of "Extra
statistics for TX/RX debugging". In order to enable them,
CONFIG_MAC80211_DEBUG_COUNTERS needs to be enabled in kernel
configuration.

This stat is a per phy device stat. It can be dumped using the below
command:

cat /sys/kernel/debug/ieee80211/phyX/statistics/mgmt_frame

When executing this command, dump management stats for all the virtual
interfaces of that particular phy device.

Signed-off-by: Ramya Gnanasekar <ramya.gnanasekar@oss.qualcomm.com>
Co-developed-by: Muna Sinada <muna.sinada@oss.qualcomm.com>
Signed-off-by: Muna Sinada <muna.sinada@oss.qualcomm.com>
---
v2:
 - posting as a PATCH instead of RFC
 - updated signoff emails to new email accounts

---
 net/mac80211/debugfs.c     | 64 ++++++++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h | 26 ++++++++++++++++
 net/mac80211/rx.c          |  2 ++
 net/mac80211/status.c      |  4 +++
 net/mac80211/tx.c          |  2 ++
 5 files changed, 98 insertions(+)

Comments

Johannes Berg Feb. 26, 2025, 2:41 p.m. UTC | #1 
On Tue, 2025-02-11 at 12:25 -0800, Muna Sinada wrote:
> Currently there aren't statistics in mac80211 that keep track of the
> management frames that are processed in both Tx and Rx. This type of
> statistics is useful in tracking if management frames are successfully
> transmitted or are dropped. These statistics are also needed to
> provide information regarding how many management frames are received.

A very, very long time ago, when a lot of statistics were being added
(and required by Android?) I suggested that perhaps we should add way to
run BPF programs on incoming frames to keep track of these things
instead of having counters across the code for all kinds of things all
the time. I suspect that in the time since that has become much simpler,
since now BPF programs can be attached to tracepoints and adding a
tracepoint or a few could be very easily done.
In this case that'd really only require a TX and TX status tracepoint,
where the latter is getting the 'acked' indication, so that's basically
trace point definition + 2 lines of code.

I wonder what you think about that. All of these counters add a pretty
large permanent memory (2 KiB here) and code cost, and having them in
debugfs _only_ can also be annoying for certain use cases.

johannes
Muna Sinada Feb. 27, 2025, 1:17 a.m. UTC | #2 
On 2/26/2025 6:41 AM, Johannes Berg wrote:
> A very, very long time ago, when a lot of statistics were being added
> (and required by Android?) I suggested that perhaps we should add way to
> run BPF programs on incoming frames to keep track of these things
> instead of having counters across the code for all kinds of things all
> the time. I suspect that in the time since that has become much simpler,
> since now BPF programs can be attached to tracepoints and adding a
> tracepoint or a few could be very easily done.
> In this case that'd really only require a TX and TX status tracepoint,
> where the latter is getting the 'acked' indication, so that's basically
> trace point definition + 2 lines of code.
>
> I wonder what you think about that. All of these counters add a pretty
> large permanent memory (2 KiB here) and code cost, and having them in
> debugfs _only_ can also be annoying for certain use cases.
>
> johannes
I agree with the points opposing the use of only debugfs.
Let me familiarize myself with BPF programs and get back to you
with my input on your recommendation.

I was wondering is there an existing feature/statistics that is a BPF
program commonly used in wireless you would recommend to use as
reference?

Muna
Johannes Berg Feb. 27, 2025, 7:33 a.m. UTC | #3 
On Wed, 2025-02-26 at 17:17 -0800, Muna Sinada wrote:
> 
> I was wondering is there an existing feature/statistics that is a BPF
> program commonly used in wireless you would recommend to use as
> reference?
> 

Not that I know of, but even the simplest examples of 'bcc' [1] are
about statistics on tracepoints.

[1] https://github.com/iovisor/bcc

johannes
diff mbox series

Patch

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index bf0a2902d93c..43e6d0f54c8e 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -633,6 +633,69 @@  DEBUGFS_DEVSTATS_FILE(dot11RTSFailureCount);
 DEBUGFS_DEVSTATS_FILE(dot11FCSErrorCount);
 DEBUGFS_DEVSTATS_FILE(dot11RTSSuccessCount);
 
+#ifdef CONFIG_MAC80211_DEBUG_COUNTERS
+static ssize_t mgmt_frame_read(struct file *file, char __user *userbuf,
+			       size_t count, loff_t *ppos)
+{
+	struct ieee80211_local *local = file->private_data;
+	int size = 1000;
+	int i, len = 0;
+	static const char *mgmt_frame[IEEE80211_MGMT_FRM_TYPE_NUM_MAX - 1] = {
+		"ASSOC REQ", "ASSOC RESP",
+		"REASSOC REQ", "REASSOC RESP",
+		"PROBE REQ", "PROBE RESP",
+		"TIMING ADV", NULL,
+		"BEACON", "ATIM",
+		"DISASSOC", "AUTH",
+		"DEAUTH", "ACTION",
+		"ACTION NO ACK"};
+
+	char *buf __free(kfree) = kzalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len += scnprintf(buf + len, size - len, "Tx mgmt stats:\n");
+	for (i = 0; i < IEEE80211_MGMT_FRM_TYPE_NUM_MAX - 1; i++) {
+		if (!mgmt_frame[i])
+			continue;
+		len += scnprintf(buf + len, size - len, "%s\t%u\n",
+				 mgmt_frame[i], local->tx_mgmt_pkts[i]);
+	}
+
+	len += scnprintf(buf + len, size - len, "\nRx mgmt stats:\n");
+	for (i = 0; i < IEEE80211_MGMT_FRM_TYPE_NUM_MAX - 1; i++) {
+		if (!mgmt_frame[i])
+			continue;
+		len += scnprintf(buf + len, size - len, "%s\t%u\n",
+				 mgmt_frame[i], local->rx_mgmt_pkts[i]);
+	}
+
+	len += scnprintf(buf + len, size - len, "\nTx mgmt completion success:\n");
+	for (i = 0; i < IEEE80211_MGMT_FRM_TYPE_NUM_MAX - 1; i++) {
+		if (!mgmt_frame[i])
+			continue;
+		len += scnprintf(buf + len, size - len, "%s\t%u\n",
+				 mgmt_frame[i], local->mgmt_tx_compl_succ[i]);
+	}
+
+	len += scnprintf(buf + len, size - len, "\nTx mgmt completion failure:\n");
+	for (i = 0; i < IEEE80211_MGMT_FRM_TYPE_NUM_MAX - 1; i++) {
+		if (!mgmt_frame[i])
+			continue;
+		len += scnprintf(buf + len, size - len, "%s\t%u\n",
+				 mgmt_frame[i], local->mgmt_tx_compl_fail[i]);
+	}
+
+	return simple_read_from_buffer(userbuf, count, ppos, buf, len);
+}
+
+static const struct file_operations stats_mgmt_frame_ops = {
+	.read = mgmt_frame_read,
+	.open = simple_open,
+	.llseek = default_llseek,
+};
+#endif
+
 void debugfs_hw_add(struct ieee80211_local *local)
 {
 	struct dentry *phyd = local->hw.wiphy->debugfsdir;
@@ -692,6 +755,7 @@  void debugfs_hw_add(struct ieee80211_local *local)
 	DEBUGFS_STATS_ADD(rx_expand_skb_head_defrag);
 	DEBUGFS_STATS_ADD(rx_handlers_fragments);
 	DEBUGFS_STATS_ADD(tx_status_drop);
+	DEBUGFS_DEVSTATS_ADD(mgmt_frame);
 #endif
 	DEBUGFS_DEVSTATS_ADD(dot11ACKFailureCount);
 	DEBUGFS_DEVSTATS_ADD(dot11RTSFailureCount);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e7dc3f0cfc9a..a4903adc1892 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1560,8 +1560,19 @@  struct ieee80211_local {
 	unsigned int rx_expand_skb_head_defrag;
 	unsigned int rx_handlers_fragments;
 	unsigned int tx_status_drop;
+
+	/* Management Frame statistics */
+#define IEEE80211_MGMT_FRM_TYPE_NUM_MAX	16
+	u32 tx_mgmt_pkts[IEEE80211_MGMT_FRM_TYPE_NUM_MAX];
+	u32 rx_mgmt_pkts[IEEE80211_MGMT_FRM_TYPE_NUM_MAX];
+	u32 mgmt_tx_compl_succ[IEEE80211_MGMT_FRM_TYPE_NUM_MAX];
+	u32 mgmt_tx_compl_fail[IEEE80211_MGMT_FRM_TYPE_NUM_MAX];
+
+#define I802_DEBUG_MGMT_STATS(_skb, _local, _type) \
+	ieee80211_mgmt_stats_update(_skb, (_local)->(_type))
 #define I802_DEBUG_INC(c) (c)++
 #else /* CONFIG_MAC80211_DEBUG_COUNTERS */
+#define I802_DEBUG_MGMT_STATS(skb, local, type) do { } while (0)
 #define I802_DEBUG_INC(c) do { } while (0)
 #endif /* CONFIG_MAC80211_DEBUG_COUNTERS */
 
@@ -2800,4 +2811,19 @@  void ieee80211_rearrange_tpe_psd(struct ieee80211_parsed_tpe_psd *psd,
 #define VISIBLE_IF_MAC80211_KUNIT static
 #endif
 
+#ifdef CONFIG_MAC80211_DEBUG_COUNTERS
+static inline void ieee80211_mgmt_stats_update(struct sk_buff *skb,
+					       u32 *mgmt_stats_type)
+{
+	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)skb->data;
+	u16 stype;
+
+	if (!ieee80211_is_mgmt(mgmt->frame_control))
+		return;
+
+	stype = le16_get_bits(mgmt->frame_control, IEEE80211_FCTL_STYPE);
+	mgmt_stats_type[stype]++;
+}
+#endif
+
 #endif /* IEEE80211_I_H */
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 1e28efe4203c..dfbe32f59180 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3427,6 +3427,8 @@  ieee80211_rx_h_mgmt_check(struct ieee80211_rx_data *rx)
 	if (!ieee80211_is_mgmt(mgmt->frame_control))
 		return RX_DROP_MONITOR;
 
+	I802_DEBUG_MGMT_STATS(rx->skb, rx->local, rx_mgmt_pkts);
+
 	/* drop too small action frames */
 	if (ieee80211_is_action(mgmt->frame_control) &&
 	    rx->skb->len < IEEE80211_MIN_ACTION_SIZE)
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 5f28f3633fa0..d4ccf3cb484b 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -1063,6 +1063,7 @@  static void __ieee80211_tx_status(struct ieee80211_hw *hw,
 			if (retry_count > 1)
 				I802_DEBUG_INC(local->dot11MultipleRetryCount);
 		}
+		I802_DEBUG_MGMT_STATS(skb, local, mgmt_tx_compl_succ);
 
 		/* This counter shall be incremented for an acknowledged MPDU
 		 * with an individual address in the address 1 field or an MPDU
@@ -1075,6 +1076,7 @@  static void __ieee80211_tx_status(struct ieee80211_hw *hw,
 	} else {
 		if (ieee80211_is_first_frag(hdr->seq_ctrl))
 			I802_DEBUG_INC(local->dot11FailedCount);
+		I802_DEBUG_MGMT_STATS(skb, local, mgmt_tx_compl_fail);
 	}
 
 	if (ieee80211_is_any_nullfunc(fc) &&
@@ -1238,6 +1240,7 @@  void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
 
 	if (acked || noack_success) {
 		I802_DEBUG_INC(local->dot11TransmittedFrameCount);
+		I802_DEBUG_MGMT_STATS(skb, local, mgmt_tx_compl_succ);
 		if (!pubsta)
 			I802_DEBUG_INC(local->dot11MulticastTransmittedFrameCount);
 		if (retry_count > 0)
@@ -1246,6 +1249,7 @@  void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
 			I802_DEBUG_INC(local->dot11MultipleRetryCount);
 	} else {
 		I802_DEBUG_INC(local->dot11FailedCount);
+		I802_DEBUG_MGMT_STATS(skb, local, mgmt_tx_compl_succ);
 	}
 
 free:
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index a24636bda679..173cdbca01ab 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1833,6 +1833,8 @@  static int invoke_tx_handlers_early(struct ieee80211_tx_data *tx)
 		return -1;
 	}
 
+	I802_DEBUG_MGMT_STATS(tx->skb, tx->local, tx_mgmt_pkts);
+
 	return 0;
 }