Skip to contentSkip to conversation

ath9k: Support ethtool getstats api.

PATCH New greearb at candelatech.com 3 participants 6 messages May 7, 2012 – May 8, 2012 Qualcomm Atheros ATH9K wireless driver

lore
Share & tools
lore

greearb at candelatech.com

author
#0 lore raw

PATCH 1/1 ath9k: Support ethtool getstats api.

From: Ben Greear <[email protected]>

This returns many of the values that formerly could only be obtained from debugfs. This should be an improvement when trying to access these counters programatically. Currently this support is only enabled when DEBUGFS is enabled because otherwise these stats are not accumulated.

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

---

This is just a resend. Now that the other wifi ethtool patches are in, this patch can be applied.

:100644 100644 dfa78e8... 8a18b22... M drivers/net/wireless/ath/ath9k/main.c

 drivers/net/wireless/ath/ath9k/main.c |  134 +++++++++++++++++++++++++++++++++
 1 files changed, 134 insertions(+), 0 deletions(-)
drivers/net/wireless/ath/ath9k/main.c +134 -0
index dfa78e8..8a18b22 100644
@@ -2388,6 +2388,134 @@ static int ath9k_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant)
 	return 0;
 }
 
+#ifdef CONFIG_ATH9K_DEBUGFS
+
+/* Ethtool support for get-stats */
+
+#define AMKSTR(nm) #nm "_BE", #nm "_BK", #nm "_VI", #nm "_VO"
+static const char ath9k_gstrings_stats[][ETH_GSTRING_LEN] = {
+	"tx_pkts_nic",
+	"tx_bytes_nic",
+	"rx_pkts_nic",
+	"rx_bytes_nic",
+	AMKSTR(d_tx_pkts),
+	AMKSTR(d_tx_bytes),
+	AMKSTR(d_tx_mpdus_queued),
+	AMKSTR(d_tx_mpdus_completed),
+	AMKSTR(d_tx_mpdu_xretries),
+	AMKSTR(d_tx_aggregates),
+	AMKSTR(d_tx_ampdus_queued_hw),
+	AMKSTR(d_tx_ampdus_queued_sw),
+	AMKSTR(d_tx_ampdus_completed),
+	AMKSTR(d_tx_ampdu_retries),
+	AMKSTR(d_tx_ampdu_xretries),
+	AMKSTR(d_tx_fifo_underrun),
+	AMKSTR(d_tx_op_exceeded),
+	AMKSTR(d_tx_timer_expiry),
+	AMKSTR(d_tx_desc_cfg_err),
+	AMKSTR(d_tx_data_underrun),
+	AMKSTR(d_tx_delim_underrun),
+
+	"d_rx_decrypt_crc_err",
+	"d_rx_phy_err",
+	"d_rx_mic_err",
+	"d_rx_pre_delim_crc_err",
+	"d_rx_post_delim_crc_err",
+	"d_rx_decrypt_busy_err",
+
+	"d_rx_phyerr_radar",
+	"d_rx_phyerr_ofdm_timing",
+	"d_rx_phyerr_cck_timing",
+
+};
+#define ATH9K_SSTATS_LEN ARRAY_SIZE(ath9k_gstrings_stats)
+
+static void ath9k_get_et_strings(struct ieee80211_hw *hw,
+				 struct ieee80211_vif *vif,
+				 u32 sset, u8 *data)
+{
+	if (sset == ETH_SS_STATS)
+		memcpy(data, *ath9k_gstrings_stats,
+		       sizeof(ath9k_gstrings_stats));
+}
+
+static int ath9k_get_et_sset_count(struct ieee80211_hw *hw,
+				   struct ieee80211_vif *vif, int sset)
+{
+	if (sset == ETH_SS_STATS)
+		return ATH9K_SSTATS_LEN;
+	return 0;
+}
+
+#define PR_QNUM(_n) (sc->tx.txq_map[_n]->axq_qnum)
+#define AWDATA(elem)							\
+	do {								\
+		data[i++] = sc->debug.stats.txstats[PR_QNUM(WME_AC_BE)].elem; \
+		data[i++] = sc->debug.stats.txstats[PR_QNUM(WME_AC_BK)].elem; \
+		data[i++] = sc->debug.stats.txstats[PR_QNUM(WME_AC_VI)].elem; \
+		data[i++] = sc->debug.stats.txstats[PR_QNUM(WME_AC_VO)].elem; \
+	} while (0)
+
+#define AWDATA_RX(elem)						\
+	do {							\
+		data[i++] = sc->debug.stats.rxstats.elem;	\
+	} while (0)
+
+static void ath9k_get_et_stats(struct ieee80211_hw *hw,
+			       struct ieee80211_vif *vif,
+			       struct ethtool_stats *stats, u64 *data)
+{
+	struct ath_softc *sc = hw->priv;
+	int i = 0;
+
+	data[i++] = (sc->debug.stats.txstats[PR_QNUM(WME_AC_BE)].tx_pkts_all +
+		     sc->debug.stats.txstats[PR_QNUM(WME_AC_BK)].tx_pkts_all +
+		     sc->debug.stats.txstats[PR_QNUM(WME_AC_VI)].tx_pkts_all +
+		     sc->debug.stats.txstats[PR_QNUM(WME_AC_VO)].tx_pkts_all);
+	data[i++] = (sc->debug.stats.txstats[PR_QNUM(WME_AC_BE)].tx_bytes_all +
+		     sc->debug.stats.txstats[PR_QNUM(WME_AC_BK)].tx_bytes_all +
+		     sc->debug.stats.txstats[PR_QNUM(WME_AC_VI)].tx_bytes_all +
+		     sc->debug.stats.txstats[PR_QNUM(WME_AC_VO)].tx_bytes_all);
+	AWDATA_RX(rx_pkts_all);
+	AWDATA_RX(rx_bytes_all);
+
+	AWDATA(tx_pkts_all);
+	AWDATA(tx_bytes_all);
+	AWDATA(queued);
+	AWDATA(completed);
+	AWDATA(xretries);
+	AWDATA(a_aggr);
+	AWDATA(a_queued_hw);
+	AWDATA(a_queued_sw);
+	AWDATA(a_completed);
+	AWDATA(a_retries);
+	AWDATA(a_xretries);
+	AWDATA(fifo_underrun);
+	AWDATA(xtxop);
+	AWDATA(timer_exp);
+	AWDATA(desc_cfg_err);
+	AWDATA(data_underrun);
+	AWDATA(delim_underrun);
+
+	AWDATA_RX(decrypt_crc_err);
+	AWDATA_RX(phy_err);
+	AWDATA_RX(mic_err);
+	AWDATA_RX(pre_delim_crc_err);
+	AWDATA_RX(post_delim_crc_err);
+	AWDATA_RX(decrypt_busy_err);
+
+	AWDATA_RX(phy_err_stats[ATH9K_PHYERR_RADAR]);
+	AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_TIMING]);
+	AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_TIMING]);
+
+	BUG_ON(i != ATH9K_SSTATS_LEN);
+}
+
+/* End of ethtool get-stats functions */
+
+#endif
+
+
 struct ieee80211_ops ath9k_ops = {
 	.tx 		    = ath9k_tx,
 	.start 		    = ath9k_start,

@@ -2416,4 +2544,10 @@ struct ieee80211_ops ath9k_ops = {
 	.get_stats	    = ath9k_get_stats,
 	.set_antenna	    = ath9k_set_antenna,
 	.get_antenna	    = ath9k_get_antenna,
+
+#ifdef CONFIG_ATH9K_DEBUGFS
+	.get_et_sset_count  = ath9k_get_et_sset_count,
+	.get_et_stats  = ath9k_get_et_stats,
+	.get_et_strings  = ath9k_get_et_strings,
+#endif
 };
Signature · 1 line
1.7.3.4
#1 lore raw

PATCH 1/1 ath9k: Support ethtool getstats api.

Quoted 8 lines · greearb at candelatech.com

From: Ben Greear <[email protected]>

This returns many of the values that formerly could only be obtained from debugfs. This should be an improvement when trying to access these counters programatically. Currently this support is only enabled when DEBUGFS is enabled because otherwise these stats are not accumulated.

trivia:

[]

Quoted 1 line

+#define ATH9K_SSTATS_LEN ARRAY_SIZE(ath9k_gstrings_stats)

I think there's not much value in this #define

drivers/net/wireless/ath/ath9k/main.c static int ath9k_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant) · line 2448

Quoted 7 lines
drivers/net/wireless/ath/ath9k/main.c +7 -0
+static int ath9k_get_et_sset_count(struct ieee80211_hw *hw,
+				   struct ieee80211_vif *vif, int sset)
+{
+	if (sset == ETH_SS_STATS)
+		return ATH9K_SSTATS_LEN;
+	return 0;
+}

Is this the only use?

author
#2 lore raw

PATCH 1/1 ath9k: Support ethtool getstats api.

Quoted 17 lines · Joe Perches
Quoted 8 lines · greearb at candelatech.com

From: Ben Greear<[email protected]>

This returns many of the values that formerly could only be obtained from debugfs. This should be an improvement when trying to access these counters programatically. Currently this support is only enabled when DEBUGFS is enabled because otherwise these stats are not accumulated.

trivia:

[]

Quoted 1 line

+#define ATH9K_SSTATS_LEN ARRAY_SIZE(ath9k_gstrings_stats)

I think there's not much value in this #define

Quoted 9 lines

drivers/net/wireless/ath/ath9k/main.c static int ath9k_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant) · line 2448

Quoted 7 lines
drivers/net/wireless/ath/ath9k/main.c +7 -0
+static int ath9k_get_et_sset_count(struct ieee80211_hw *hw,
+				   struct ieee80211_vif *vif, int sset)
+{
+	if (sset == ETH_SS_STATS)
+		return ATH9K_SSTATS_LEN;
+	return 0;
+}

Is this the only use?

No, there is a check later that does a BUG_ON if our we have screwed up the indexing of the stats.

That helps catch trivial problems with adding a stat but not the string, or vice versa.

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#3 lore raw

PATCH 1/1 ath9k: Support ethtool getstats api.

Quoted 2 lines · Ben Greear

No, there is a check later that does a BUG_ON if our we have screwed up the indexing of the stats.

Please, no BUG_ON() calls in wifi drivers. They just make users life miserable. WARN_ON() with a safe bailout is enough.

Signature · 1 line
Kalle Valo
author
#4 lore raw

PATCH 1/1 ath9k: Support ethtool getstats api.

Quoted 7 lines · Kalle Valo
Quoted 2 lines · Ben Greear

No, there is a check later that does a BUG_ON if our we have screwed up the indexing of the stats.

Please, no BUG_ON() calls in wifi drivers. They just make users life miserable. WARN_ON() with a safe bailout is enough.

Ok, I can change that.

The reason I left it at BUG_ON before is because a mistake here could easily be a buffer overflow, so I wanted to make sure it was caught in development.

But, a warn-on should be sufficient.

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#5 lore raw

PATCH 1/1 ath9k: Support ethtool getstats api.

In reply to Kalle Valo
Quoted 7 lines · Kalle Valo
Quoted 2 lines · Ben Greear

No, there is a check later that does a BUG_ON if our we have screwed up the indexing of the stats.

Please, no BUG_ON() calls in wifi drivers. They just make users life miserable. WARN_ON() with a safe bailout is enough.

$ git grep -w -E "BUG|BUG_ON" drivers/net/wireless | wc -l
209
$ git grep -w -E "BUG_ON" drivers/net/wireless | wc -l
183
$ git grep -w -E "BUG_ON" drivers/net/wireless/ath | wc -l
34

Maybe something for you to look at?