From: Ben Greear <[email protected]>
Signed-off-by: Ben Greear <[email protected]>
---
:100644 100644 9ed8021... d97c9da... M include/net/cfg80211.h
:100644 100644 bfed851... bc75a32... M net/mac80211/sta_info.h
:100644 100644 9bde4d1... 7eecdf4... M net/wireless/ethtool.c
include/net/cfg80211.h | 7 +++++++
net/mac80211/sta_info.h | 2 ++
net/wireless/ethtool.c | 29 +++++++++++++++++++++++++++++
3 files changed, 38 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/mac80211/sta_info.h b/net/mac80211/sta_info.h
index bfed851..bc75a32 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -379,6 +379,8 @@ struct sta_info {
struct ieee80211_sta sta;
};
+#define NUM_STA_INFO_STATS 12
+
static inline enum nl80211_plink_state sta_plink_state(struct sta_info *sta)
{
#ifdef CONFIG_MAC80211_MESH
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/15/2012 11:50 AM, Johannes Berg wrote:
> On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
>
>> --- a/net/mac80211/sta_info.h
>> +++ b/net/mac80211/sta_info.h
>> @@ -379,6 +379,8 @@ struct sta_info {
>> struct ieee80211_sta sta;
>> };
>>
>> +#define NUM_STA_INFO_STATS 12
>> +
>
> That doesn't seem to belong into this patch?
Er, no..it does not. I'll fix that.
Ben
>
> johannes
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 03/16/2012 07:18 AM, Florian Fainelli wrote:
> Hello,
>
> Le 03/16/12 14:49, John W. Linville a écrit :
>> On Thu, Mar 15, 2012 at 11:56:24AM -0700, Ben Greear wrote:
>>> On 03/15/2012 11:52 AM, Johannes Berg wrote:
>>>> On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
>>>>> From: Ben Greear<[email protected]>
>>>>>
>>>>> This lets ethtool print out stats related to station
>>>>> interfaces. Does not yet get stats from the underlying
>>>>> driver.
>>>>
>>>> Hmm. What's the advantage of using ethtool over iw, which already has a
>>>> bunch of these numbers?
>>>
>>> Well, ethtool api might be easier for some apps to use,
>>> and perhaps easier for users to read if all they want
>>> are stats.
>>
>> And they can use the same tool for both wired and wireless interfaces
>> -- could be handy.
>
> iw already provides statistics which are relevant for wireless interfaces. I really don't see the point in also reporting them via ethtool, also it is going to
> be error prone if someone updates the netlink interface and forgets about the ethtool one.
Ethtool provides what it provides. If it's missing a stat, I or someone else can add it.
And if it's not there, then users can do without or get it elsewhere.
iw does not provide any way to get underlying wifi radio counters, and
the only other way I've found is to parse various debugfs files that
are subject to change at a whim. Ethtool is a step up from that at least.
iw is also not supposed to be screen-scraped, while ethtool stats are a
fairly easily parsed output so scripts and such could use that.
Thanks,
Ben
> --
> Florian
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 03/15/2012 11:52 AM, Johannes Berg wrote:
> On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
>> From: Ben Greear<[email protected]>
>>
>> This lets ethtool print out stats related to station
>> interfaces. Does not yet get stats from the underlying
>> driver.
>
> Hmm. What's the advantage of using ethtool over iw, which already has a
> bunch of these numbers?
Well, ethtool api might be easier for some apps to use,
and perhaps easier for users to read if all they want
are stats.
But, I also plan to add stats from the underlying
driver with and additional (optional) call down
to the driver, and I think we would not ever want to
add that to netlink API.
That way, we can add internal ath9k counters and
such instead of having to use debugfs.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On Thu, Mar 15, 2012 at 11:56:24AM -0700, Ben Greear wrote:
> On 03/15/2012 11:52 AM, Johannes Berg wrote:
> >On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
> >>From: Ben Greear<[email protected]>
> >>
> >>This lets ethtool print out stats related to station
> >>interfaces. Does not yet get stats from the underlying
> >>driver.
> >
> >Hmm. What's the advantage of using ethtool over iw, which already has a
> >bunch of these numbers?
>
> Well, ethtool api might be easier for some apps to use,
> and perhaps easier for users to read if all they want
> are stats.
And they can use the same tool for both wired and wireless interfaces
-- could be handy.
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Thu, 2012-03-15 at 12:03 -0700, Ben Greear wrote:
> >> + list_for_each_entry(sta,&sdata->local->sta_list, list) {
> >
> > That's not right -- these stats are per netdev so you should only
> > aggregate for the netdev, no?
>
> Ummm, maybe so. I had trouble figuring out how to find
> the sta entries that are associated with a netdev, and I'd
> like to sum up all station entries for an AP interfaces.
>
> If you know of any code that uses this, a pointer would
> be welcome.
Just compare sta->sdata?
> >> + BUG_ON(i != STA_STATS_LEN);
> >
> > That I really don't like much.
>
> Ok, I'll remove it from a final patch. It catches bugs
> in the meantime (miss a comma between strings and
> it blows up spectacularly :P).
Heh. Some verification would be nice I guess, but I can't really figure
out a good way either.
johannes
On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> This lets ethtool print out stats related to station
> interfaces. Does not yet get stats from the underlying
> driver.
Hmm. What's the advantage of using ethtool over iw, which already has a
bunch of these numbers?
johannes
On 03/16/2012 07:59 AM, Florian Fainelli wrote:
> Le 03/16/12 15:54, Ben Greear a écrit :
>> Ethtool provides what it provides. If it's missing a stat, I or someone else can add it.
>
> What if we don't want to bloat ethtool with new stats? I mean, someone else one day will see wireless stats in, and say, hey why don't I add atm, x25, or any
> protocol of the moment to ethtool, then what do we do?
Cheer and be happy! Or, make the code conditionally compiled
if you are very scarce on RAM.
>> And if it's not there, then users can do without or get it elsewhere.
>>
>> iw does not provide any way to get underlying wifi radio counters, and
>> the only other way I've found is to parse various debugfs files that
>> are subject to change at a whim.
>
> That's another topic, if debugfs interface changes you should complain to their maintainers.
>
>> Ethtool is a step up from that at least.
>>
>> iw is also not supposed to be screen-scraped, while ethtool stats are a
>> fairly easily parsed output so scripts and such could use that.
>
> Why not fix iw instead so its output gets nicely parsed?
debugfs and iw output is meant to be parsed by humans.
In particular, debugfs is not a 'real' api and can change
all the time.
> Would there be no reporting tool that I would not object, otherwise I prefer to keep things where they are consistent, that is, iw configures and reports stats
> for wireless, and so does ethtool for Ethernet interfaces.
Another problem with using iw: You have to modify user-space as well
as the kernel to get new stats. But, with ethtool, you can just modify
the kernel and any remotely modern ethtool executable can display
the new stats.
Note that my patches require NO changes to the ethtool binary to display
the new stats.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
Hello,
Le 03/16/12 14:49, John W. Linville a écrit :
> On Thu, Mar 15, 2012 at 11:56:24AM -0700, Ben Greear wrote:
>> On 03/15/2012 11:52 AM, Johannes Berg wrote:
>>> On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
>>>> From: Ben Greear<[email protected]>
>>>>
>>>> This lets ethtool print out stats related to station
>>>> interfaces. Does not yet get stats from the underlying
>>>> driver.
>>>
>>> Hmm. What's the advantage of using ethtool over iw, which already has a
>>> bunch of these numbers?
>>
>> Well, ethtool api might be easier for some apps to use,
>> and perhaps easier for users to read if all they want
>> are stats.
>
> And they can use the same tool for both wired and wireless interfaces
> -- could be handy.
iw already provides statistics which are relevant for wireless
interfaces. I really don't see the point in also reporting them via
ethtool, also it is going to be error prone if someone updates the
netlink interface and forgets about the ethtool one.
--
Florian
On 03/15/2012 11:55 AM, Johannes Berg wrote:
> Ok more technical look ...
>
>
> It doesn't look like NUM_STA_INFO_STATS gets used at all, or am I
> missing it?
>
>> +static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
>> + struct net_device *dev,
>> + int sset)
>> +{
>> + switch (sset) {
>> + case ETH_SS_STATS:
>> + return STA_STATS_LEN;
>
> Are there different "s* sets" that could be used to return different
> types of stats, or how does this work?
Yes, there are diag strings too, I think..but I wasn't
planning to add support for that, though perhaps someone
else might know how to do that.
>> + mutex_lock(&sdata->local->sta_mtx);
>
> Why not use RCU? The mutex doesn't really protect anything useful here
> except for the list iteration.
No reason. I'll try to make it RCU instead.
>> + list_for_each_entry(sta,&sdata->local->sta_list, list) {
>
> That's not right -- these stats are per netdev so you should only
> aggregate for the netdev, no?
Ummm, maybe so. I had trouble figuring out how to find
the sta entries that are associated with a netdev, and I'd
like to sum up all station entries for an AP interfaces.
If you know of any code that uses this, a pointer would
be welcome.
>> + BUG_ON(i != STA_STATS_LEN);
>
> That I really don't like much.
Ok, I'll remove it from a final patch. It catches bugs
in the meantime (miss a comma between strings and
it blows up spectacularly :P).
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
Le 03/16/12 15:54, Ben Greear a écrit :
> On 03/16/2012 07:18 AM, Florian Fainelli wrote:
>> Hello,
>>
>> Le 03/16/12 14:49, John W. Linville a écrit :
>>> On Thu, Mar 15, 2012 at 11:56:24AM -0700, Ben Greear wrote:
>>>> On 03/15/2012 11:52 AM, Johannes Berg wrote:
>>>>> On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
>>>>>> From: Ben Greear<[email protected]>
>>>>>>
>>>>>> This lets ethtool print out stats related to station
>>>>>> interfaces. Does not yet get stats from the underlying
>>>>>> driver.
>>>>>
>>>>> Hmm. What's the advantage of using ethtool over iw, which already
>>>>> has a
>>>>> bunch of these numbers?
>>>>
>>>> Well, ethtool api might be easier for some apps to use,
>>>> and perhaps easier for users to read if all they want
>>>> are stats.
>>>
>>> And they can use the same tool for both wired and wireless interfaces
>>> -- could be handy.
>>
>> iw already provides statistics which are relevant for wireless
>> interfaces. I really don't see the point in also reporting them via
>> ethtool, also it is going to
>> be error prone if someone updates the netlink interface and forgets
>> about the ethtool one.
>
> Ethtool provides what it provides. If it's missing a stat, I or
> someone else can add it.
What if we don't want to bloat ethtool with new stats? I mean, someone
else one day will see wireless stats in, and say, hey why don't I add
atm, x25, or any protocol of the moment to ethtool, then what do we do?
>
> And if it's not there, then users can do without or get it elsewhere.
>
> iw does not provide any way to get underlying wifi radio counters, and
> the only other way I've found is to parse various debugfs files that
> are subject to change at a whim.
That's another topic, if debugfs interface changes you should complain
to their maintainers.
> Ethtool is a step up from that at least.
>
> iw is also not supposed to be screen-scraped, while ethtool stats are a
> fairly easily parsed output so scripts and such could use that.
Why not fix iw instead so its output gets nicely parsed?
Would there be no reporting tool that I would not object, otherwise I
prefer to keep things where they are consistent, that is, iw configures
and reports stats for wireless, and so does ethtool for Ethernet interfaces.
--
Florian
On 03/16/2012 07:59 AM, Florian Fainelli wrote:
> Le 03/16/12 15:54, Ben Greear a écrit :
>> On 03/16/2012 07:18 AM, Florian Fainelli wrote:
>>> Hello,
>>>
>>> Le 03/16/12 14:49, John W. Linville a écrit :
>>>> On Thu, Mar 15, 2012 at 11:56:24AM -0700, Ben Greear wrote:
>>>>> On 03/15/2012 11:52 AM, Johannes Berg wrote:
>>>>>> On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
>>>>>>> From: Ben Greear<[email protected]>
>>>>>>>
>>>>>>> This lets ethtool print out stats related to station
>>>>>>> interfaces. Does not yet get stats from the underlying
>>>>>>> driver.
>>>>>>
>>>>>> Hmm. What's the advantage of using ethtool over iw, which already
>>>>>> has a bunch of these numbers?
>>>>>
>>>>> Well, ethtool api might be easier for some apps to use,
>>>>> and perhaps easier for users to read if all they want
>>>>> are stats.
>>>>
>>>> And they can use the same tool for both wired and wireless interfaces
>>>> -- could be handy.
>>>
>>> iw already provides statistics which are relevant for wireless
>>> interfaces. I really don't see the point in also reporting them via
>>> ethtool, also it is going to
>>> be error prone if someone updates the netlink interface and forgets
>>> about the ethtool one.
>>
>> Ethtool provides what it provides. If it's missing a stat, I or
>> someone else can add it.
>
> What if we don't want to bloat ethtool with new stats? I mean, someone
> else one day will see wireless stats in, and say, hey why don't I add
> atm, x25, or any protocol of the moment to ethtool, then what do we do?
Rejoice and perhaps add an alias to ethtool called nictool or linktool
or somesuch? As already pointed-out there is a non-trivial niceness to
not having to have umpteen different tools to get stats from a network
interface.
"What's in a name" may apply, but these are wireless *ethernet*
interfaces right?
rick jones
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... 2acd0ea... M net/mac80211/cfg.c
net/mac80211/cfg.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index cf5b08a..2acd0ea 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -112,6 +112,72 @@ 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)
+{
+ switch (sset) {
+ case ETH_SS_STATS:
+ return STA_STATS_LEN;
+ default:
+ 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;
+
+ memset(data, 0, sizeof(u64) * STA_STATS_LEN);
+
+ mutex_lock(&sdata->local->sta_mtx);
+ list_for_each_entry(sta, &sdata->local->sta_list, list) {
+ int i = 0;
+ 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);
+ }
+ mutex_unlock(&sdata->local->sta_mtx);
+}
+
+static void ieee80211_get_et_strings(struct wiphy *wiphy,
+ struct net_device *dev,
+ u32 sset, u8 *data)
+{
+ switch (sset) {
+ case ETH_SS_STATS:
+ memcpy(data, *ieee80211_gstrings_sta_stats,
+ sizeof(ieee80211_gstrings_sta_stats));
+ break;
+ }
+}
+
+
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 +2834,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
On Thu, 2012-03-15 at 10:39 -0700, [email protected] wrote:
> --- a/net/mac80211/sta_info.h
> +++ b/net/mac80211/sta_info.h
> @@ -379,6 +379,8 @@ struct sta_info {
> struct ieee80211_sta sta;
> };
>
> +#define NUM_STA_INFO_STATS 12
> +
That doesn't seem to belong into this patch?
johannes
Ok more technical look ...
It doesn't look like NUM_STA_INFO_STATS gets used at all, or am I
missing it?
> +static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
> + struct net_device *dev,
> + int sset)
> +{
> + switch (sset) {
> + case ETH_SS_STATS:
> + return STA_STATS_LEN;
Are there different "s* sets" that could be used to return different
types of stats, or how does this work?
> + mutex_lock(&sdata->local->sta_mtx);
Why not use RCU? The mutex doesn't really protect anything useful here
except for the list iteration.
> + list_for_each_entry(sta, &sdata->local->sta_list, list) {
That's not right -- these stats are per netdev so you should only
aggregate for the netdev, no?
> + BUG_ON(i != STA_STATS_LEN);
That I really don't like much.
johannes