diff mbox series

[RFC,v2,2/2] mwifiex: add NL80211_STA_INFO_RX_BITRATE support

Message ID 20181207001249.165477-2-briannorris@chromium.org (mailing list archive)
State Accepted
Commit ed0b2b067bad9d923842ac4c68c6b652b5885b84
Delegated to: Kalle Valo
Headers show
Series [RFC,v2,1/2] mwifiex: refactor mwifiex_parse_htinfo() for reuse | expand

Commit Message

Brian Norris Dec. 7, 2018, 12:12 a.m. UTC
Comparing the existing TX_BITRATE parsing code (in
mwifiex_parse_htinfo()) with the RX bitrate histograms in debugfs.c, it
appears that the rxpd_rate and rxpd_htinfo fields have the same format.
At least, they give reasonable results when I parse them this way.

So this patch adds support for RX_BITRATE to our station info dump.

Along the way, I add legacy bitrate parsing into the same function,
using the debugfs code (mwifiex_histogram_read() and
mwifiex_adjust_data_rate()) as reference.

Additionally, to satisfy the requirements of
NL80211_STA_INFO_RX_BITRATE, I skip logging the bitrate of multicast
packets. This shouldn't add a lot of overhead to the RX path, as there
are already several similar 802.3 header checks in this same codepath.
We can also bias the branch behavior to favor unicast, as that's the
common performance-sensitive case.

I'd consider this support somewhat experimental, as I have zero
documentation from Marvell. But the existing driver code gives me good
reason to think this is correct.

I've tested this on a few different 802.11{a,b,g,n,ac} networks, and the
reported bitrates look good to me.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
RFC: I'd appreciate it if someone from Marvell could double check my work
here.

v2:
 * no change - just split unrelated (debugfs) patch to its own series
---
 .../net/wireless/marvell/mwifiex/cfg80211.c   | 26 +++++++++++++++++++
 drivers/net/wireless/marvell/mwifiex/sta_rx.c | 13 ++++++----
 2 files changed, 34 insertions(+), 5 deletions(-)

Comments

Kalle Valo Dec. 7, 2018, 10:50 a.m. UTC | #1
Brian Norris <briannorris@chromium.org> writes:

> Comparing the existing TX_BITRATE parsing code (in
> mwifiex_parse_htinfo()) with the RX bitrate histograms in debugfs.c, it
> appears that the rxpd_rate and rxpd_htinfo fields have the same format.
> At least, they give reasonable results when I parse them this way.
>
> So this patch adds support for RX_BITRATE to our station info dump.
>
> Along the way, I add legacy bitrate parsing into the same function,
> using the debugfs code (mwifiex_histogram_read() and
> mwifiex_adjust_data_rate()) as reference.
>
> Additionally, to satisfy the requirements of
> NL80211_STA_INFO_RX_BITRATE, I skip logging the bitrate of multicast
> packets. This shouldn't add a lot of overhead to the RX path, as there
> are already several similar 802.3 header checks in this same codepath.
> We can also bias the branch behavior to favor unicast, as that's the
> common performance-sensitive case.
>
> I'd consider this support somewhat experimental, as I have zero
> documentation from Marvell. But the existing driver code gives me good
> reason to think this is correct.
>
> I've tested this on a few different 802.11{a,b,g,n,ac} networks, and the
> reported bitrates look good to me.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> RFC: I'd appreciate it if someone from Marvell could double check my work
> here.

BTW, if we don't hear anything from Marvell I'm going to apply these
anyway.
Ganapathi Bhat Dec. 7, 2018, 11:55 a.m. UTC | #2
Hi Kalle/Brian,

>> RFC: I'd appreciate it if someone from Marvell could double check my work
>> here.

>BTW, if we don't hear anything from Marvell I'm going to apply these
>anyway.

This patch series looks good to us.

Thanks,
Ganapathi
diff mbox series

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 02b80ea232a7..1467af22e394 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -1333,6 +1333,28 @@  mwifiex_parse_htinfo(struct mwifiex_private *priv, u8 rateinfo, u8 htinfo,
 				rate->flags |= RATE_INFO_FLAGS_SHORT_GI;
 		}
 	}
+
+	/* Decode legacy rates for non-HT. */
+	if (!(htinfo & (BIT(0) | BIT(1)))) {
+		/* Bitrates in multiples of 100kb/s. */
+		static const int legacy_rates[] = {
+			[0] = 10,
+			[1] = 20,
+			[2] = 55,
+			[3] = 110,
+			[4] = 60, /* MWIFIEX_RATE_INDEX_OFDM0 */
+			[5] = 60,
+			[6] = 90,
+			[7] = 120,
+			[8] = 180,
+			[9] = 240,
+			[10] = 360,
+			[11] = 480,
+			[12] = 540,
+		};
+		if (rateinfo < ARRAY_SIZE(legacy_rates))
+			rate->legacy = legacy_rates[rateinfo];
+	}
 }
 
 /*
@@ -1414,6 +1436,10 @@  mwifiex_dump_station_info(struct mwifiex_private *priv,
 	/* bit rate is in 500 kb/s units. Convert it to 100kb/s units */
 	sinfo->txrate.legacy = rate * 5;
 
+	sinfo->filled |= BIT(NL80211_STA_INFO_RX_BITRATE);
+	mwifiex_parse_htinfo(priv, priv->rxpd_rate, priv->rxpd_htinfo,
+			     &sinfo->rxrate);
+
 	if (priv->bss_mode == NL80211_IFTYPE_STATION) {
 		sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BSS_PARAM);
 		sinfo->bss_param.flags = 0;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
index 00fcbda09349..fb28a5c7f441 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
@@ -152,14 +152,17 @@  int mwifiex_process_rx_packet(struct mwifiex_private *priv,
 		mwifiex_process_tdls_action_frame(priv, offset, rx_pkt_len);
 	}
 
-	priv->rxpd_rate = local_rx_pd->rx_rate;
-
-	priv->rxpd_htinfo = local_rx_pd->ht_info;
+	/* Only stash RX bitrate for unicast packets. */
+	if (likely(!is_multicast_ether_addr(rx_pkt_hdr->eth803_hdr.h_dest))) {
+		priv->rxpd_rate = local_rx_pd->rx_rate;
+		priv->rxpd_htinfo = local_rx_pd->ht_info;
+	}
 
 	if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA ||
 	    GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP) {
-		adj_rx_rate = mwifiex_adjust_data_rate(priv, priv->rxpd_rate,
-						       priv->rxpd_htinfo);
+		adj_rx_rate = mwifiex_adjust_data_rate(priv,
+						       local_rx_pd->rx_rate,
+						       local_rx_pd->ht_info);
 		mwifiex_hist_data_add(priv, adj_rx_rate, local_rx_pd->snr,
 				      local_rx_pd->nf);
 	}