From: Ben Greear <[email protected]>
Signed-off-by: Ben Greear <[email protected]>
---
:100644 100644 9ed8021... d97c9da... M include/net/cfg80211.h
:100644 100644 9bde4d1... 7eecdf4... M net/wireless/ethtool.c
include/net/cfg80211.h | 7 +++++++
net/wireless/ethtool.c | 29 +++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9ed8021..d97c9da 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1689,6 +1689,13 @@ struct cfg80211_ops {
u16 noack_map);
struct ieee80211_channel *(*get_channel)(struct wiphy *wiphy);
+
+ int (*get_et_sset_count)(struct wiphy *wiphy,
+ struct net_device* dev, int sset);
+ void (*get_et_stats)(struct wiphy *wiphy, struct net_device* dev,
+ struct ethtool_stats *stats, u64 *data);
+ void (*get_et_strings)(struct wiphy *wiphy, struct net_device* dev,
+ u32 sset, u8 *data);
};
/*
diff --git a/net/wireless/ethtool.c b/net/wireless/ethtool.c
index 9bde4d1..7eecdf4 100644
--- a/net/wireless/ethtool.c
+++ b/net/wireless/ethtool.c
@@ -68,6 +68,32 @@ static int cfg80211_set_ringparam(struct net_device *dev,
return -ENOTSUPP;
}
+static int cfg80211_get_sset_count(struct net_device *dev, int sset)
+{
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+ if (rdev->ops->get_et_sset_count)
+ return rdev->ops->get_et_sset_count(wdev->wiphy, dev, sset);
+ return -EOPNOTSUPP;
+}
+
+static void cfg80211_get_stats(struct net_device *dev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+ if (rdev->ops->get_et_stats)
+ rdev->ops->get_et_stats(wdev->wiphy, dev, stats, data);
+}
+
+static void cfg80211_get_strings(struct net_device *dev, u32 sset, u8 *data)
+{
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+ if (rdev->ops->get_et_strings)
+ rdev->ops->get_et_strings(wdev->wiphy, dev, sset, data);
+}
+
const struct ethtool_ops cfg80211_ethtool_ops = {
.get_drvinfo = cfg80211_get_drvinfo,
.get_regs_len = cfg80211_get_regs_len,
@@ -75,4 +101,7 @@ const struct ethtool_ops cfg80211_ethtool_ops = {
.get_link = ethtool_op_get_link,
.get_ringparam = cfg80211_get_ringparam,
.set_ringparam = cfg80211_set_ringparam,
+ .get_strings = cfg80211_get_strings,
+ .get_ethtool_stats = cfg80211_get_stats,
+ .get_sset_count = cfg80211_get_sset_count,
};
--
1.7.3.4
On 03/16/2012 03:05 AM, Florian Fainelli wrote:
> Hi,
>
> I do not think this is particularly a good idea to make eththool report wireless interfaces statistics:
> - these are 802.11 interfaces and so they have specific statistics to report which are different from pure ethernet adapters
> - people will start adding more statistics to ethtool because this or that wifi-specific counter is not reported, it is abusing the tool imho.
The ethtool stats are designed to display custom driver stats in a somewhat standard manner. There is no
expectation as to what stats are or should be available..some new wired Intel drivers report great numbers of
stats, and some drivers report none.
But, assuming we do not change the stat names all the time (adding new ones or re-arranging order is fine),
a tool can be written to parse the stats on a per-driver basis without much difficulty.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 03/16/2012 09:36 AM, Felix Fietkau wrote:
> On 2012-03-16 5:23 PM, Ben Greear wrote:
>> On 03/16/2012 08:06 AM, Felix Fietkau wrote:
>> My personal goal is to give my users a better insight into their
>> wireless environment. I would like to be able to break out the
>> various low-level errors (as well as add them together to present
>> summary errors).
>>
>> For anyone reporting mysterious bugs on mailing lists and such, I'd like
>> to ask them to dump the stats and send it to me/list/whatever.
>> I am still of the mind that looking for patterns in various
>> counters might point to underlying problems, so anything that
>> makes it easier to get that data is a win in my mind.
> That's what debugfs is for, and it's not exactly hard to use.
> I think it would be bad if tools started depending on the counters in
> their current state, they're pretty messy.
With a bit of work, we could make the ethtool stats
available when debugfs is compiled out, which might give
you actual space savings and still allow at least much of
the stats. Either way, the ability to programatically
parse the ethtool stats is a big win for my use, at least.
>> If there are some stats that really don't work, maybe I
>> will notice, or maybe someone else will and we can fix
>> them. If you are aware of any specific ones that don't
>> work right, please let me know and I'll at least add some
>> comments to the code to mention they are questionable,
>> or fix them if I'm able.
> There's lots of confusion in the AMPDU/Aggregates counters (some count
> whole A-MPDUs, some count subframes).
That sounds fixable, if it's a problem at all.
> Also, many of these counters are useless unless you're doing live
> debugging and actually know what you're doing - especially the ones that
> display the current queue state.
I tried to skip all of those current-queue-state counters, but I
will double-check.
> Most of the PHY error counters aren't even tracked by the hw, nothing in
> the driver enables their use.
Well, any reason we cannot enable those?
> For some of these counters it might make sense to track them in
> mac80211, as they're sufficiently generic.
That sounds nice. Maybe add a 'get_stats' object to the driver
api? If you have a list of what you want to be made
generic I'll make a try at implementing it.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 03/16/2012 08:06 AM, Felix Fietkau wrote:
> On 2012-03-16 12:20 AM, [email protected] wrote:
>> 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]>
> I don't really like the idea just throwing every random counter into
> ethtool. Many of these counters are actually quite misleading, not
> properly tracked, or not even updated properly, and thus do not deserve
> to be made visible through ethtool.
>
> I think before something gets exported that way, it should be made clear
> what it's for and what the limitations are.
My personal goal is to give my users a better insight into their
wireless environment. I would like to be able to break out the
various low-level errors (as well as add them together to present
summary errors).
For anyone reporting mysterious bugs on mailing lists and such, I'd like
to ask them to dump the stats and send it to me/list/whatever.
I am still of the mind that looking for patterns in various
counters might point to underlying problems, so anything that
makes it easier to get that data is a win in my mind.
If there are some stats that really don't work, maybe I
will notice, or maybe someone else will and we can fix
them. If you are aware of any specific ones that don't
work right, please let me know and I'll at least add some
comments to the code to mention they are questionable,
or fix them if I'm able.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On Thu, 2012-03-15 at 16:20 -0700, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> This adds hooks to call into the driver to get additional
> stats for the ethtool API.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> :100644 100644 d49928b... e89a742... M include/net/mac80211.h
> :100644 100644 0aef0d2... 872e06e... M net/mac80211/cfg.c
> include/net/mac80211.h | 9 +++++++++
> net/mac80211/cfg.c | 23 +++++++++++++++++++++--
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index d49928b..e89a742 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -2236,6 +2236,15 @@ struct ieee80211_ops {
> u16 tids, int num_frames,
> enum ieee80211_frame_release_type reason,
> bool more_data);
> +
> + int (*get_et_sset_count)(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif, int sset);
> + void (*get_et_stats)(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct ethtool_stats *stats, u64 *data);
> + void (*get_et_strings)(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + u32 sset, u8 *data);
> };
>
> /**
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 0aef0d2..872e06e 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -125,8 +125,17 @@ static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
> struct net_device *dev,
> int sset)
> {
> - if (sset == ETH_SS_STATS)
> - return STA_STATS_LEN;
> + struct ieee80211_sub_if_data *sdata;
> + struct ieee80211_local *local;
> + if (sset == ETH_SS_STATS) {
> + int rv = STA_STATS_LEN;
> + sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + local = sdata->local;
> + if (local->ops->get_et_sset_count)
> + rv += local->ops->get_et_sset_count(&local->hw,
> + &sdata->vif, sset);
should there be tracing for this?
johannes
On 2012-03-16 5:51 PM, Ben Greear wrote:
> On 03/16/2012 09:36 AM, Felix Fietkau wrote:
>> On 2012-03-16 5:23 PM, Ben Greear wrote:
>>> On 03/16/2012 08:06 AM, Felix Fietkau wrote:
>
>>> My personal goal is to give my users a better insight into their
>>> wireless environment. I would like to be able to break out the
>>> various low-level errors (as well as add them together to present
>>> summary errors).
>>>
>>> For anyone reporting mysterious bugs on mailing lists and such, I'd like
>>> to ask them to dump the stats and send it to me/list/whatever.
>>> I am still of the mind that looking for patterns in various
>>> counters might point to underlying problems, so anything that
>>> makes it easier to get that data is a win in my mind.
>> That's what debugfs is for, and it's not exactly hard to use.
>> I think it would be bad if tools started depending on the counters in
>> their current state, they're pretty messy.
>
> With a bit of work, we could make the ethtool stats
> available when debugfs is compiled out, which might give
> you actual space savings and still allow at least much of
> the stats. Either way, the ability to programatically
> parse the ethtool stats is a big win for my use, at least.
I'm OK with exporting some stats in a way that can be easily parsed, but
it should be limited to data that actually makes sense and isn't
available through normal API.
>>> If there are some stats that really don't work, maybe I
>>> will notice, or maybe someone else will and we can fix
>>> them. If you are aware of any specific ones that don't
>>> work right, please let me know and I'll at least add some
>>> comments to the code to mention they are questionable,
>>> or fix them if I'm able.
>> There's lots of confusion in the AMPDU/Aggregates counters (some count
>> whole A-MPDUs, some count subframes).
> That sounds fixable, if it's a problem at all.
>
>> Also, many of these counters are useless unless you're doing live
>> debugging and actually know what you're doing - especially the ones that
>> display the current queue state.
>
> I tried to skip all of those current-queue-state counters, but I
> will double-check.
Yeah, seems like I misread something there.
>> Most of the PHY error counters aren't even tracked by the hw, nothing in
>> the driver enables their use.
>
> Well, any reason we cannot enable those?
The hardware has a limited number of counter registers available for
tracking PHY errors. Enabling PHY errors to be reported via DMA wastes
tons of precious CPU cycles.
>> For some of these counters it might make sense to track them in
>> mac80211, as they're sufficiently generic.
>
> That sounds nice. Maybe add a 'get_stats' object to the driver
> api? If you have a list of what you want to be made
> generic I'll make a try at implementing it.
I don't have a list.
- Felix
On Thu, 2012-03-15 at 16:20 -0700, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> :100644 100644 9ed8021... d97c9da... M include/net/cfg80211.h
> :100644 100644 9bde4d1... 7eecdf4... M net/wireless/ethtool.c
> include/net/cfg80211.h | 7 +++++++
> net/wireless/ethtool.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 9ed8021..d97c9da 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1689,6 +1689,13 @@ struct cfg80211_ops {
> u16 noack_map);
>
> struct ieee80211_channel *(*get_channel)(struct wiphy *wiphy);
> +
> + int (*get_et_sset_count)(struct wiphy *wiphy,
> + struct net_device* dev, int sset);
Do I really have to point out that the space should be before the
asterisk? ;-)
johannes
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]>
---
:100644 100644 4a00806... d46644a... M drivers/net/wireless/ath/ath9k/main.c
drivers/net/wireless/ath/ath9k/main.c | 183 +++++++++++++++++++++++++++++++++
1 files changed, 183 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 4a00806..d46644a 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2430,6 +2430,183 @@ 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_retries),
+ 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),
+ AMKSTR(d_hw_put_txbuf),
+ AMKSTR(d_tx_hw_start),
+ AMKSTR(d_tx_hw_proc_desc),
+ "d_rx_crc_err",
+ "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_underrun",
+ "d_rx_phyerr_timing",
+ "d_rx_phyerr_parity",
+ "d_rx_phyerr_rate",
+ "d_rx_phyerr_length",
+ "d_rx_phyerr_radar",
+ "d_rx_phyerr_service",
+ "d_rx_phyerr_tor",
+ "d_rx_phyerr_ofdm_timing",
+ "d_rx_phyerr_ofdm_signal_parity",
+ "d_rx_phyerr_ofdm_rate",
+ "d_rx_phyerr_ofdm_length",
+ "d_rx_phyerr_ofdm_power_drop",
+ "d_rx_phyerr_ofdm_service",
+ "d_rx_phyerr_ofdm_restart",
+ "d_rx_phyerr_false_radar_ext",
+ "d_rx_phyerr_cck_timing",
+ "d_rx_phyerr_cck_header_crc",
+ "d_rx_phyerr_cck_rate",
+ "d_rx_phyerr_cck_service",
+ "d_rx_phyerr_cck_restart",
+ "d_rx_phyerr_cck_length",
+ "d_rx_phyerr_cck_power_drop",
+ "d_rx_phyerr_ht_crc",
+ "d_rx_phyerr_ht_rate",
+};
+#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) \
+ data[i++] = sc->debug.stats.rxstats.elem;
+
+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(puttxbuf);
+ AWDATA(txstart);
+ AWDATA(txprocdesc);
+
+ 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_UNDERRUN]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_TIMING]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_PARITY]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_RATE]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_LENGTH]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_RADAR]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_SERVICE]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_TOR]);
+
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_TIMING]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_SIGNAL_PARITY]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_RATE_ILLEGAL]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_LENGTH_ILLEGAL]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_POWER_DROP]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_SERVICE]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_OFDM_RESTART]);
+
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_FALSE_RADAR_EXT]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_TIMING]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_HEADER_CRC]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_RATE_ILLEGAL]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_SERVICE]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_RESTART]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_LENGTH_ILLEGAL]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_CCK_POWER_DROP]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_HT_CRC_ERROR]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_HT_LENGTH_ILLEGAL]);
+ AWDATA_RX(phy_err_stats[ATH9K_PHYERR_HT_RATE_ILLEGAL]);
+
+ BUG_ON(i != ATH9K_SSTATS_LEN);
+}
+
+/* End of ethtool get-stats functions */
+
+#endif
+
+
struct ieee80211_ops ath9k_ops = {
.tx = ath9k_tx,
.start = ath9k_start,
@@ -2458,4 +2635,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
};
--
1.7.3.4
On 03/16/2012 10:06 AM, Felix Fietkau wrote:
>>> Most of the PHY error counters aren't even tracked by the hw, nothing in
>>> the driver enables their use.
>>
>> Well, any reason we cannot enable those?
> The hardware has a limited number of counter registers available for
> tracking PHY errors. Enabling PHY errors to be reported via DMA wastes
> tons of precious CPU cycles.
Am I correct in assuming that these are the only phy-errors that
the driver can currently report?
#define AR_PHY_ERR_DCHIRP 0x00000008
#define AR_PHY_ERR_RADAR 0x00000020
#define AR_PHY_ERR_OFDM_TIMING 0x00020000
#define AR_PHY_ERR_CCK_TIMING 0x02000000
With regard to DMA resources, do you mean that the
waste is in having the NIC receive the packets that
would otherwise be filtered?
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
From: Ben Greear <[email protected]>
This lets ethtool print out stats related to station
interfaces. Does not yet get stats from the underlying
driver.
Signed-off-by: Ben Greear <[email protected]>
---
:100644 100644 cf5b08a... 0aef0d2... M net/mac80211/cfg.c
net/mac80211/cfg.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index cf5b08a..0aef0d2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -112,6 +112,74 @@ static int ieee80211_set_noack_map(struct wiphy *wiphy,
return 0;
}
+static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {
+ "rx_packets", "rx_bytes", "wep_weak_iv_count",
+ "rx_duplicates", "rx_fragments", "rx_dropped",
+ "tx_packets", "tx_bytes", "tx_fragments",
+ "tx_filtered", "tx_retry_failed", "tx_retries",
+ "beacon_loss"
+};
+#define STA_STATS_LEN ARRAY_SIZE(ieee80211_gstrings_sta_stats)
+
+static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
+ struct net_device *dev,
+ int sset)
+{
+ if (sset == ETH_SS_STATS)
+ return STA_STATS_LEN;
+
+ return -EOPNOTSUPP;
+}
+
+static void ieee80211_get_et_stats(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct ethtool_stats *stats,
+ u64 *data)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct sta_info *sta;
+ struct ieee80211_local *local = sdata->local;
+
+ memset(data, 0, sizeof(u64) * STA_STATS_LEN);
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ int i = 0;
+
+ /* Make sure this station belongs to the proper dev */
+ if (sta->sdata->dev != dev)
+ continue;
+
+ data[i++] += sta->rx_packets;
+ data[i++] += sta->rx_bytes;
+ data[i++] += sta->wep_weak_iv_count;
+ data[i++] += sta->num_duplicates;
+ data[i++] += sta->rx_fragments;
+ data[i++] += sta->rx_dropped;
+
+ data[i++] += sta->tx_packets;
+ data[i++] += sta->tx_bytes;
+ data[i++] += sta->tx_fragments;
+ data[i++] += sta->tx_filtered_count;
+ data[i++] += sta->tx_retry_failed;
+ data[i++] += sta->tx_retry_count;
+ data[i++] += sta->beacon_loss_count;
+ BUG_ON(i != STA_STATS_LEN);
+ }
+ rcu_read_unlock();
+}
+
+static void ieee80211_get_et_strings(struct wiphy *wiphy,
+ struct net_device *dev,
+ u32 sset, u8 *data)
+{
+ if (sset == ETH_SS_STATS) {
+ int sz_sta_stats = sizeof(ieee80211_gstrings_sta_stats);
+ memcpy(data, *ieee80211_gstrings_sta_stats, sz_sta_stats);
+ }
+}
+
+
static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
u8 key_idx, bool pairwise, const u8 *mac_addr,
struct key_params *params)
@@ -2768,4 +2836,7 @@ struct cfg80211_ops mac80211_config_ops = {
.probe_client = ieee80211_probe_client,
.get_channel = ieee80211_wiphy_get_channel,
.set_noack_map = ieee80211_set_noack_map,
+ .get_et_sset_count = ieee80211_get_et_sset_count,
+ .get_et_stats = ieee80211_get_et_stats,
+ .get_et_strings = ieee80211_get_et_strings,
};
--
1.7.3.4
From: Ben Greear <[email protected]>
This adds hooks to call into the driver to get additional
stats for the ethtool API.
Signed-off-by: Ben Greear <[email protected]>
---
:100644 100644 d49928b... e89a742... M include/net/mac80211.h
:100644 100644 0aef0d2... 872e06e... M net/mac80211/cfg.c
include/net/mac80211.h | 9 +++++++++
net/mac80211/cfg.c | 23 +++++++++++++++++++++--
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d49928b..e89a742 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2236,6 +2236,15 @@ struct ieee80211_ops {
u16 tids, int num_frames,
enum ieee80211_frame_release_type reason,
bool more_data);
+
+ int (*get_et_sset_count)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif, int sset);
+ void (*get_et_stats)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ethtool_stats *stats, u64 *data);
+ void (*get_et_strings)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ u32 sset, u8 *data);
};
/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 0aef0d2..872e06e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -125,8 +125,17 @@ static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
struct net_device *dev,
int sset)
{
- if (sset == ETH_SS_STATS)
- return STA_STATS_LEN;
+ struct ieee80211_sub_if_data *sdata;
+ struct ieee80211_local *local;
+ if (sset == ETH_SS_STATS) {
+ int rv = STA_STATS_LEN;
+ sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ local = sdata->local;
+ if (local->ops->get_et_sset_count)
+ rv += local->ops->get_et_sset_count(&local->hw,
+ &sdata->vif, sset);
+ return rv;
+ }
return -EOPNOTSUPP;
}
@@ -167,15 +176,25 @@ static void ieee80211_get_et_stats(struct wiphy *wiphy,
BUG_ON(i != STA_STATS_LEN);
}
rcu_read_unlock();
+
+ if (local->ops->get_et_stats)
+ local->ops->get_et_stats(&local->hw, &sdata->vif,
+ stats, &(data[STA_STATS_LEN]));
}
static void ieee80211_get_et_strings(struct wiphy *wiphy,
struct net_device *dev,
u32 sset, u8 *data)
{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
if (sset == ETH_SS_STATS) {
int sz_sta_stats = sizeof(ieee80211_gstrings_sta_stats);
memcpy(data, *ieee80211_gstrings_sta_stats, sz_sta_stats);
+
+ if (local->ops->get_et_strings)
+ local->ops->get_et_strings(&local->hw, &sdata->vif,
+ sset, &(data[sz_sta_stats]));
}
}
--
1.7.3.4
On 2012-03-16 5:23 PM, Ben Greear wrote:
> On 03/16/2012 08:06 AM, Felix Fietkau wrote:
>> On 2012-03-16 12:20 AM, [email protected] wrote:
>>> 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]>
>> I don't really like the idea just throwing every random counter into
>> ethtool. Many of these counters are actually quite misleading, not
>> properly tracked, or not even updated properly, and thus do not deserve
>> to be made visible through ethtool.
>>
>> I think before something gets exported that way, it should be made clear
>> what it's for and what the limitations are.
>
> My personal goal is to give my users a better insight into their
> wireless environment. I would like to be able to break out the
> various low-level errors (as well as add them together to present
> summary errors).
>
> For anyone reporting mysterious bugs on mailing lists and such, I'd like
> to ask them to dump the stats and send it to me/list/whatever.
> I am still of the mind that looking for patterns in various
> counters might point to underlying problems, so anything that
> makes it easier to get that data is a win in my mind.
That's what debugfs is for, and it's not exactly hard to use.
I think it would be bad if tools started depending on the counters in
their current state, they're pretty messy.
> If there are some stats that really don't work, maybe I
> will notice, or maybe someone else will and we can fix
> them. If you are aware of any specific ones that don't
> work right, please let me know and I'll at least add some
> comments to the code to mention they are questionable,
> or fix them if I'm able.
There's lots of confusion in the AMPDU/Aggregates counters (some count
whole A-MPDUs, some count subframes).
Also, many of these counters are useless unless you're doing live
debugging and actually know what you're doing - especially the ones that
display the current queue state.
Most of the PHY error counters aren't even tracked by the hw, nothing in
the driver enables their use.
For some of these counters it might make sense to track them in
mac80211, as they're sufficiently generic.
If you really want to export more stats, please put some thought into
what actually makes sense for exporting, rather than carpet-bombing the
stats with every random debugfs counter crap you can find.
- Felix
On 2012-03-16 12:20 AM, [email protected] wrote:
> 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]>
I don't really like the idea just throwing every random counter into
ethtool. Many of these counters are actually quite misleading, not
properly tracked, or not even updated properly, and thus do not deserve
to be made visible through ethtool.
I think before something gets exported that way, it should be made clear
what it's for and what the limitations are.
- Felix
Hi,
Le 03/16/12 00:20, [email protected] a écrit :
> From: Ben Greear<[email protected]>
>
> Signed-off-by: Ben Greear<[email protected]>
> ---
> :100644 100644 9ed8021... d97c9da... M include/net/cfg80211.h
> :100644 100644 9bde4d1... 7eecdf4... M net/wireless/ethtool.c
> include/net/cfg80211.h | 7 +++++++
> net/wireless/ethtool.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 9ed8021..d97c9da 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1689,6 +1689,13 @@ struct cfg80211_ops {
> u16 noack_map);
>
> struct ieee80211_channel *(*get_channel)(struct wiphy *wiphy);
> +
> + int (*get_et_sset_count)(struct wiphy *wiphy,
> + struct net_device* dev, int sset);
> + void (*get_et_stats)(struct wiphy *wiphy, struct net_device* dev,
> + struct ethtool_stats *stats, u64 *data);
> + void (*get_et_strings)(struct wiphy *wiphy, struct net_device* dev,
> + u32 sset, u8 *data);
> };
>
> /*
> diff --git a/net/wireless/ethtool.c b/net/wireless/ethtool.c
> index 9bde4d1..7eecdf4 100644
> --- a/net/wireless/ethtool.c
> +++ b/net/wireless/ethtool.c
> @@ -68,6 +68,32 @@ static int cfg80211_set_ringparam(struct net_device *dev,
> return -ENOTSUPP;
> }
>
> +static int cfg80211_get_sset_count(struct net_device *dev, int sset)
> +{
> + struct wireless_dev *wdev = dev->ieee80211_ptr;
> + struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
> + if (rdev->ops->get_et_sset_count)
> + return rdev->ops->get_et_sset_count(wdev->wiphy, dev, sset);
> + return -EOPNOTSUPP;
> +}
> +
> +static void cfg80211_get_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct wireless_dev *wdev = dev->ieee80211_ptr;
> + struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
> + if (rdev->ops->get_et_stats)
> + rdev->ops->get_et_stats(wdev->wiphy, dev, stats, data);
> +}
> +
> +static void cfg80211_get_strings(struct net_device *dev, u32 sset, u8 *data)
> +{
> + struct wireless_dev *wdev = dev->ieee80211_ptr;
> + struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
> + if (rdev->ops->get_et_strings)
> + rdev->ops->get_et_strings(wdev->wiphy, dev, sset, data);
> +}
> +
> const struct ethtool_ops cfg80211_ethtool_ops = {
> .get_drvinfo = cfg80211_get_drvinfo,
> .get_regs_len = cfg80211_get_regs_len,
> @@ -75,4 +101,7 @@ const struct ethtool_ops cfg80211_ethtool_ops = {
> .get_link = ethtool_op_get_link,
> .get_ringparam = cfg80211_get_ringparam,
> .set_ringparam = cfg80211_set_ringparam,
> + .get_strings = cfg80211_get_strings,
> + .get_ethtool_stats = cfg80211_get_stats,
> + .get_sset_count = cfg80211_get_sset_count,
> };
I do not think this is particularly a good idea to make eththool report
wireless interfaces statistics:
- these are 802.11 interfaces and so they have specific statistics to
report which are different from pure ethernet adapters
- people will start adding more statistics to ethtool because this or
that wifi-specific counter is not reported, it is abusing the tool imho.
--
Florian