[PATCH 0/2] wifi: mac80211: make the phy tpt LED indicate real traffic

[PATCH 0/2] wifi: mac80211: make the phy tpt LED indicate real traffic

[Salah Ahmed]

An access point's phy throughput LED currently counts frames that are never
delivered to any local interface, and keeps blinking on the broadcast an idle AP
bridges, so it cannot answer the only question a user asks of it: is anything
actually being transferred?

Patch 1 counts the trigger from delivered data only, moving the RX accounting
past BSS/interface matching in the rx path. Patch 2 keeps the LED resting in its
baseline state below a small idle threshold, so the residual broadcast of a
zero-client AP no longer produces the slowest blink step forever.

Both were exercised on ath9k (QCA9563) and ath10k (QCA9888) access points,
including a zero-client idle-solid battery and a link-flap battery over several
weeks of production uptime.

Salah Ahmed (2):
  wifi: mac80211: count the tpt LED trigger from delivered data only
  wifi: mac80211: keep the tpt LED solid below an idle threshold

 net/mac80211/led.c | 16 ++++++++++++++++
 net/mac80211/rx.c  |  7 ++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.53.0

[PATCH 1/2] wifi: mac80211: count the tpt LED trigger from delivered data only

[Salah Ahmed]

The phy tpt trigger adds every received frame with the data-present bit
set to its RX byte count in ieee80211_rx_list(), before the frame is
matched to a BSS or delivered to an interface. On a busy band an access
point with zero associated clients still hears data frames from
neighbouring networks, so the throughput LED blinks continuously and
gives no indication of actual transfer. Management frames such as
beacons are already excluded by the data-present check, and the TX side
only counts data submitted through the interface transmit paths, so
overheard data is the only remaining source of idle blinking.

Move the RX count to the two places where a frame is actually handed to
an interface, right next to the dev_sw_netstats_rx_add() calls in
ieee80211_deliver_skb() and ieee80211_rx_8023() (the latter also covers
the fast-RX and the 802.3-offload delivery paths). The trigger then
counts exactly the data delivered to the wiphy's interfaces, in every
mode - AP, client or mesh - which matches what the per-interface
counters report and what users read from a throughput LED.

This scopes the RX side only. The TX side is untouched, and an access
point still transmits the broadcast and multicast it bridges with zero
associated stations - measured at roughly 2 KiB/s on a quiet LAN - which
keeps the counter moving and the blink table stepping, so an idle radio is
not solid yet. Treating a rate below an idle floor as no traffic is a
separate change.

Monitor copies are deliberately not counted: they are clones of frames
whose original delivery is already counted once at its own interface,
and counting them would double the reported rate whenever a monitor
interface is up.

The deleted block was the only user of the local hdr pointer in
ieee80211_rx_list(), so drop its declaration too - mac80211 is built
with -Werror=unused-variable and the file would not compile otherwise.

Signed-off-by: Salah Ahmed <haddad2inc@gmail.com>
---
 net/mac80211/rx.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 905fd7049..559cc2101 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2756,6 +2756,7 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 	xmit_skb = NULL;
 
 	dev_sw_netstats_rx_add(dev, skb->len);
+	ieee80211_tpt_led_trig_rx(rx->local, skb->len);
 
 	if (rx->sta) {
 		/* The seqno index has the same property as needed
@@ -4970,6 +4971,7 @@ static void ieee80211_rx_8023(struct ieee80211_rx_data *rx,
 	skb->dev = fast_rx->dev;
 
 	dev_sw_netstats_rx_add(fast_rx->dev, skb->len);
+	ieee80211_tpt_led_trig_rx(rx->local, skb->len);
 
 	/* The seqno index has the same property as needed
 	 * for the rx_msdu field, i.e. it is IEEE80211_NUM_TIDS
@@ -5583,7 +5585,6 @@ void ieee80211_rx_list(struct ieee80211_hw *hw,
 	struct ieee80211_rate *rate = NULL;
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 
 	WARN_ON_ONCE(softirq_count() == 0);
 
@@ -5727,10 +5728,6 @@ void ieee80211_rx_list(struct ieee80211_hw *hw,
 	if (!(status->flag & RX_FLAG_8023))
 		skb = ieee80211_rx_monitor(local, skb, rate);
 	if (skb) {
-		if ((status->flag & RX_FLAG_8023) ||
-			ieee80211_is_data_present(hdr->frame_control))
-			ieee80211_tpt_led_trig_rx(local, skb->len);
-
 		if (status->flag & RX_FLAG_8023)
 			__ieee80211_rx_handle_8023(hw, link_pubsta, skb, list);
 		else
-- 
2.53.0

[PATCH 2/2] wifi: mac80211: keep the tpt LED solid below an idle threshold

[Salah Ahmed]

The phy throughput LED blinks whenever the counted traffic is non-zero:
tpt_trig_timer() walks the blink table from the top and only falls back to
solid on when no entry matches, which needs tpt == 0 exactly. An access
point transmits the broadcast and multicast it bridges even with zero
associated stations - measured at roughly 2 KiB/s on a quiet LAN - so the
LED blinks at the slowest table step (334 ms) forever and stops indicating
whether anything is actually being transferred.

Count anything below 12 KiB/s (96 kbit/s) as idle instead. That is the
threshold the userspace activity monitors used on these devices were tuned
against: it is above the background of a quiet LAN and well below any real
transfer, so the LED is solid while nothing moves and still tracks the
blink table as soon as a station transfers data.

Signed-off-by: Salah Ahmed <haddad2inc@gmail.com>
---
 net/mac80211/led.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/net/mac80211/led.c b/net/mac80211/led.c
index b5600d223..78fffe122 100644
--- a/net/mac80211/led.c
+++ b/net/mac80211/led.c
@@ -9,6 +9,16 @@
 #include <linux/export.h>
 #include "led.h"
 
+/*
+ * Below this throughput the tpt LED stays solid on instead of blinking at the
+ * slowest step of the table. An access point transmits the LAN broadcast and
+ * multicast it bridges even with zero associated stations (measured ~2 KiB/s),
+ * which is enough to keep the LED blinking forever while nothing is actually
+ * transferred. 12 KiB/s (96 kbit/s) is the idle rate the userspace activity
+ * monitors this behaviour was derived from were tuned against.
+ */
+#define IEEE80211_TPT_LED_IDLE_KBIT	(12 * 8)
+
 void ieee80211_led_assoc(struct ieee80211_local *local, bool associated)
 {
 	if (!atomic_read(&local->assoc_led_active))
@@ -270,6 +280,12 @@ static void tpt_trig_timer(struct timer_list *t)
 
 	tpt = tpt_trig_traffic(local, tpt_trig);
 
+	/* Treat anything below the idle threshold as no traffic, so that the
+	 * LED is solid while the radio only carries background broadcast.
+	 */
+	if (tpt < IEEE80211_TPT_LED_IDLE_KBIT)
+		tpt = 0;
+
 	/* default to just solid on */
 	on = 1;
 	off = 0;
-- 
2.53.0