2023-10-07 21:30:14

by Ben Greear

[permalink] [raw]
Subject: [RFC] wifi: iwlwifi: Add ethtool rx-ampdu-count histogram.

From: Ben Greear <[email protected]>

Seems to mostly work, but I am seeing a lot more single ampdu and
2-10 ampdu counts that I'd expect, so maybe there are still bugs.
Perhaps some race if MQ means that ampdu_toggle cannot be used as I
am trying to use it?

Signed-off-by: Ben Greear <[email protected]>
---

This patch won't apply upstream, it needs other iwlwifi ethtool patches
I posted previously.

But, I'd appreciate any feedback if someone has time to review it.

.../net/wireless/intel/iwlwifi/mvm/mac80211.c | 19 +++++++
drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 5 ++
drivers/net/wireless/intel/iwlwifi/mvm/rx.c | 49 +++++++++++++++++++
drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 12 ++++-
drivers/net/wireless/intel/iwlwifi/mvm/tx.c | 1 +
5 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 2fa609a65749..71b8e774792c 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -6592,6 +6592,22 @@ static const char iwl_mvm_gstrings_stats[][ETH_GSTRING_LEN] = {
"rx_mcs_12",
"rx_mcs_13",

+ "rx_ampdu_len:0-1",
+ "rx_ampdu_len:2-10",
+ "rx_ampdu_len:11-19",
+ "rx_ampdu_len:20-28",
+ "rx_ampdu_len:29-37",
+ "rx_ampdu_len:38-46",
+ "rx_ampdu_len:47-55",
+ "rx_ampdu_len:56-79",
+ "rx_ampdu_len:80-103",
+ "rx_ampdu_len:104-127",
+ "rx_ampdu_len:128-151",
+ "rx_ampdu_len:152-175",
+ "rx_ampdu_len:176-199",
+ "rx_ampdu_len:200-223",
+ "rx_ampdu_len:224-247", /* and higher */
+
"rx_nss_1",
"rx_nss_2",
};
@@ -6712,6 +6728,9 @@ void iwl_mvm_get_et_stats(struct ieee80211_hw *hw,
for (i = 0; i < ARRAY_SIZE(mib->rx_mcs); i++)
data[ei++] = mib->rx_mcs[i];

+ for (i = 0; i < ARRAY_SIZE(mib->rx_ampdu_len); i++)
+ data[ei++] = mib->rx_ampdu_len[i];
+
for (i = 0; i < ARRAY_SIZE(mib->rx_nss); i++)
data[ei++] = mib->rx_nss[i];

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index b05eb752824d..1d88732899ea 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -616,6 +616,7 @@ struct iwl_mvm_ethtool_stats {
u64 rx_bw[5]; /* 20, 40, 80, 160, 320 */
u64 rx_bw_he_ru;
u64 rx_mcs[14]; /* mcs 0 to mcs 13 */
+ u64 rx_ampdu_len[15];
u64 rx_nss[2]; /* rx nss histogram */
};

@@ -981,6 +982,8 @@ struct iwl_mvm {

u8 cca_40mhz_workaround;

+ /* Accumulate the count for the number of frames in the ampdu */
+ u32 rx_this_ampdu_count;
u32 ampdu_ref;
bool ampdu_toggle;

@@ -1916,6 +1919,8 @@ int iwl_mvm_load_d3_fw(struct iwl_mvm *mvm);

int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm);

+void iwl_mvm_count_rx_histogram(struct iwl_mvm *mvm);
+
/*
* FW notifications / CMD responses handlers
* Convention: iwl_mvm_rx_<NAME OF THE CMD>
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rx.c b/drivers/net/wireless/intel/iwlwifi/mvm/rx.c
index ae9ce43a4461..7e9377b9f7aa 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rx.c
@@ -28,6 +28,9 @@ void iwl_mvm_rx_rx_phy_cmd(struct iwl_mvm *mvm, struct iwl_rx_cmd_buffer *rxb)
memcpy(&mvm->last_phy_info, pkt->data, sizeof(mvm->last_phy_info));
mvm->ampdu_ref++;

+ /* Add to histogram for last ampdu count */
+ iwl_mvm_count_rx_histogram(mvm);
+
#ifdef CONFIG_IWLWIFI_DEBUGFS
if (mvm->last_phy_info.phy_flags & cpu_to_le16(RX_RES_PHY_FLAGS_AGG)) {
spin_lock(&mvm->drv_stats_lock);
@@ -198,6 +201,48 @@ static u32 iwl_mvm_set_mac80211_rx_flag(struct iwl_mvm *mvm,
return 0;
}

+void iwl_mvm_count_rx_histogram(struct iwl_mvm *mvm)
+{
+ u32 count = mvm->rx_this_ampdu_count;
+
+ if (count == 0)
+ return;
+
+ /* rx-ampdu-len histogram, buckets match what mtk7915 supports. */
+ if (count <= 1)
+ mvm->ethtool_stats.rx_ampdu_len[0]++;
+ else if (count <= 10)
+ mvm->ethtool_stats.rx_ampdu_len[1]++;
+ else if (count <= 19)
+ mvm->ethtool_stats.rx_ampdu_len[2]++;
+ else if (count <= 28)
+ mvm->ethtool_stats.rx_ampdu_len[3]++;
+ else if (count <= 37)
+ mvm->ethtool_stats.rx_ampdu_len[4]++;
+ else if (count <= 46)
+ mvm->ethtool_stats.rx_ampdu_len[5]++;
+ else if (count <= 55)
+ mvm->ethtool_stats.rx_ampdu_len[6]++;
+ else if (count <= 79)
+ mvm->ethtool_stats.rx_ampdu_len[7]++;
+ else if (count <= 103)
+ mvm->ethtool_stats.rx_ampdu_len[8]++;
+ else if (count <= 127)
+ mvm->ethtool_stats.rx_ampdu_len[9]++;
+ else if (count <= 151)
+ mvm->ethtool_stats.rx_ampdu_len[10]++;
+ else if (count <= 175)
+ mvm->ethtool_stats.rx_ampdu_len[11]++;
+ else if (count <= 199)
+ mvm->ethtool_stats.rx_ampdu_len[12]++;
+ else if (count <= 223)
+ mvm->ethtool_stats.rx_ampdu_len[13]++;
+ else
+ mvm->ethtool_stats.rx_ampdu_len[14]++;
+
+ mvm->rx_this_ampdu_count = 0;
+}
+
static void iwl_mvm_rx_handle_tcm(struct iwl_mvm *mvm,
struct ieee80211_sta *sta,
struct ieee80211_hdr *hdr, u32 len,
@@ -486,6 +531,10 @@ void iwl_mvm_rx_rx_mpdu(struct iwl_mvm *mvm, struct napi_struct *napi,
*/
rx_status->flag |= RX_FLAG_AMPDU_DETAILS;
rx_status->ampdu_reference = mvm->ampdu_ref;
+ mvm->rx_this_ampdu_count++;
+ } else {
+ /* Add to histogram for last ampdu count */
+ iwl_mvm_count_rx_histogram(mvm);
}

/* Set up the HT phy flags */
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index a4ce27010dea..27508c504551 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -2525,11 +2525,11 @@ void iwl_mvm_rx_mpdu_mq(struct iwl_mvm *mvm, struct napi_struct *napi,
}

/* update aggregation data for monitor sake on default queue */
- if (!queue && (phy_data.phy_info & IWL_RX_MPDU_PHY_AMPDU)) {
+ if (phy_data.phy_info & IWL_RX_MPDU_PHY_AMPDU) {
bool toggle_bit;

toggle_bit = phy_data.phy_info & IWL_RX_MPDU_PHY_AMPDU_TOGGLE;
- rx_status->flag |= RX_FLAG_AMPDU_DETAILS;
+
/*
* Toggle is switched whenever new aggregation starts. Make
* sure ampdu_reference is never 0 so we can later use it to
@@ -2541,8 +2541,16 @@ void iwl_mvm_rx_mpdu_mq(struct iwl_mvm *mvm, struct napi_struct *napi,
mvm->ampdu_ref++;
mvm->ampdu_toggle = toggle_bit;
phy_data.first_subframe = true;
+ iwl_mvm_count_rx_histogram(mvm);
+ mvm->rx_this_ampdu_count = 1;
+ } else {
+ mvm->rx_this_ampdu_count++;
}
+ if (!queue)
+ rx_status->flag |= RX_FLAG_AMPDU_DETAILS;
rx_status->ampdu_reference = mvm->ampdu_ref;
+ } else {
+ iwl_mvm_count_rx_histogram(mvm);
}

rcu_read_lock();
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index b5d3538726b4..8fba19a7fd4f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -1849,6 +1849,7 @@ static void iwl_mvm_update_tx_ampdu_histogram(struct iwl_mvm *mvm, int freed)
mvm->ethtool_stats.tx_ampdu_len[13]++;
else
mvm->ethtool_stats.tx_ampdu_len[14]++;
+ // TODO: Consider higher buckets, quick experiment shows be200 freeing in the 250 range.
}

static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
--
2.40.0