netdev - [PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220718222804.21708-1-linus.luessing@c0d3.blue>
Date:   Tue, 19 Jul 2022 00:28:04 +0200
From:   Linus Lüssing <linus.luessing@...3.blue>
To:     Johannes Berg <johannes.berg@...el.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Toke Høiland-Jørgensen <toke@...e.dk>,
        Kalle Valo <kvalo@...nel.org>, Felix Fietkau <nbd@....name>,
        Simon Wunderlich <sw@...onwunderlich.de>,
        Sven Eckelmann <sven@...fation.org>,
        Linus Lüssing <linus.luessing@...3.blue>,
        ath10k@...ts.infradead.org, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Linus Lüssing <ll@...onwunderlich.de>
Subject: [PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

From: Linus Lüssing <ll@...onwunderlich.de>

AR9003 based wifi chips have a hardware bug, they always report a
channel bandwidth of HT40 for any sub-frame of an aggregate which is
not the last one. Only the last sub-frame has correct channel bandwidth
information.

This can be easily reproduced by setting an ath9k based wifi to HT20 and
running an iperf test. Then "iw dev wlan0 station dump" will occasionally,
wrongly show something like:

  rx bitrate:     121.5 MBit/s MCS 6 40MHz

Debug output in ath9k_hw_process_rxdesc_edma() confirmed that it is
always frames with (rxs->rs_isaggr && !rxs->rs_moreaggr) and no others
which report RATE_INFO_BW_40.

Unfortunately we cannot easily fix this within ath9k as in ath9k we
cannot peek at the rate/bandwidth info of the last aggregate
sub-frame and there is no queueing within ath9k after receiving the
frame from the wifi chip, it is directly handed over to mac80211.

Therefore fixing this within mac80211: For an aggergated AMPDU only
update the RX "last_rate" variable from the last sub-frame of an
aggregate. In theory, without hardware bugs, rate/bandwidth info
should be the same for all sub-frames of an aggregate anyway.

This change only affects ath9k, ath9k-htc and ath10k as these are
currently the only drivers implementing the RX_FLAG_AMPDU_LAST_KNOWN
flag.

Tested-on: 8devices Lima board, QCA9531 WiFi

Cc: Sven Eckelmann <sven@...fation.org>
Cc: Simon Wunderlich <sw@...onwunderlich.de>
Cc: Linus Lüssing <linus.luessing@...3.blue>
Signed-off-by: Linus Lüssing <ll@...onwunderlich.de>
---
 net/mac80211/rx.c       |  8 ++++----
 net/mac80211/sta_info.h | 16 +++++++++++++---
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 304b9909f025..988dbf058489 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1723,6 +1723,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 	struct sk_buff *skb = rx->skb;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	u32 *last_rate = &sta->deflink.rx_stats.last_rate;
 	int i;
 
 	if (!sta)
@@ -1744,8 +1745,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 			sta->deflink.rx_stats.last_rx = jiffies;
 			if (ieee80211_is_data(hdr->frame_control) &&
 			    !is_multicast_ether_addr(hdr->addr1))
-				sta->deflink.rx_stats.last_rate =
-					sta_stats_encode_rate(status);
+				sta_stats_encode_rate(status, last_rate);
 		}
 	} else if (rx->sdata->vif.type == NL80211_IFTYPE_OCB) {
 		sta->deflink.rx_stats.last_rx = jiffies;
@@ -1757,7 +1757,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 		 */
 		sta->deflink.rx_stats.last_rx = jiffies;
 		if (ieee80211_is_data(hdr->frame_control))
-			sta->deflink.rx_stats.last_rate = sta_stats_encode_rate(status);
+			sta_stats_encode_rate(status, last_rate);
 	}
 
 	sta->deflink.rx_stats.fragments++;
@@ -4502,7 +4502,7 @@ static void ieee80211_rx_8023(struct ieee80211_rx_data *rx,
 	/* end of statistics */
 
 	stats->last_rx = jiffies;
-	stats->last_rate = sta_stats_encode_rate(status);
+	sta_stats_encode_rate(status, &stats->last_rate);
 
 	stats->fragments++;
 	stats->packets++;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 70ee55ec5518..67f9c1647567 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -941,10 +941,19 @@ enum sta_stats_type {
 
 #define STA_STATS_RATE_INVALID		0
 
-static inline u32 sta_stats_encode_rate(struct ieee80211_rx_status *s)
+static inline void
+sta_stats_encode_rate(struct ieee80211_rx_status *s, u32 *rate)
 {
 	u32 r;
 
+	/* some drivers (notably ath9k) only report a valid bandwidth
+	 * in the last subframe of an aggregate, skip the others
+	 * in that case
+	 */
+	if (s->flag & RX_FLAG_AMPDU_LAST_KNOWN &&
+	    !(s->flag & RX_FLAG_AMPDU_IS_LAST))
+		return;
+
 	r = STA_STATS_FIELD(BW, s->bw);
 
 	if (s->enc_flags & RX_ENC_FLAG_SHORT_GI)
@@ -975,10 +984,11 @@ static inline u32 sta_stats_encode_rate(struct ieee80211_rx_status *s)
 		break;
 	default:
 		WARN_ON(1);
-		return STA_STATS_RATE_INVALID;
+		*rate = STA_STATS_RATE_INVALID;
+		return;
 	}
 
-	return r;
+	*rate = r;
 }
 
 #endif /* STA_INFO_H */
-- 
2.36.1