diff mbox

[4/5] mac80211: Add more ethtools stats: survey, rates, etc

Message ID 1334248375-22967-5-git-send-email-greearb@candelatech.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Ben Greear April 12, 2012, 4:32 p.m. UTC
From: Ben Greear <greearb@candelatech.com>

The signal and noise are forced to be positive since ethtool
deals in unsigned 64-bit values and this number should be human
readable.  This gives easy access to some of the data formerly
exposed in the deprecated /proc/net/wireless file.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 b37fb0d... 99e3597... M	net/mac80211/cfg.c
 net/mac80211/cfg.c |  155 ++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 114 insertions(+), 41 deletions(-)

Comments

Florian Fainelli April 12, 2012, 4:42 p.m. UTC | #1
Hi,

Le 04/12/12 18:32, greearb@candelatech.com a écrit :
> From: Ben Greear<greearb@candelatech.com>
>
> The signal and noise are forced to be positive since ethtool
> deals in unsigned 64-bit values and this number should be human
> readable.  This gives easy access to some of the data formerly
> exposed in the deprecated /proc/net/wireless file.

Uh, that's misleading, the signal and noise values are typically 
negative, so one needs to think about mentally adding a minus sign if 
he/she wants to understand it. Does not ethtool know about 32-bits 
signed integers?

>
> Signed-off-by: Ben Greear<greearb@candelatech.com>
> ---
> :100644 100644 b37fb0d... 99e3597... M	net/mac80211/cfg.c
>   net/mac80211/cfg.c |  155 ++++++++++++++++++++++++++++++++++++++--------------
>   1 files changed, 114 insertions(+), 41 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index b37fb0d..99e3597 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -117,7 +117,9 @@ static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {
>   	"rx_duplicates", "rx_fragments", "rx_dropped",
>   	"tx_packets", "tx_bytes", "tx_fragments",
>   	"tx_filtered", "tx_retry_failed", "tx_retries",
> -	"beacon_loss"
> +	"beacon_loss", "txrate", "rxrate", "signal",
> +	"channel", "noise", "ch_time", "ch_time_busy",
> +	"ch_time_ext_busy", "ch_time_rx", "ch_time_tx"
>   };
>   #define STA_STATS_LEN	ARRAY_SIZE(ieee80211_gstrings_sta_stats)
>
> @@ -138,46 +140,6 @@ static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
>   	return rv;
>   }
>
> -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();
> -
> -	drv_get_et_stats(sdata, stats,&(data[STA_STATS_LEN]));
> -}
> -
>   static void ieee80211_get_et_strings(struct wiphy *wiphy,
>   				     struct net_device *dev,
>   				     u32 sset, u8 *data)
> @@ -531,6 +493,117 @@ static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
>   		sinfo->sta_flags.set |= BIT(NL80211_STA_FLAG_TDLS_PEER);
>   }
>
> +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;
> +	struct station_info sinfo;
> +	struct survey_info survey;
> +	bool do_once = true;
> +	int i;
> +#define STA_STATS_SURVEY_LEN 7
> +
> +	memset(data, 0, sizeof(u64) * STA_STATS_LEN);
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(sta,&local->sta_list, list) {
> +		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;
> +
> +		if (!do_once) {
> +			i += 3;
> +			goto after_once;
> +		}
> +
> +		do_once = false;
> +		sinfo.filled = 0;
> +		sta_set_sinfo(sta,&sinfo);
> +
> +		if (sinfo.filled | STATION_INFO_TX_BITRATE)
> +			data[i] = 100000 *
> +				cfg80211_calculate_bitrate(&sinfo.txrate);
> +		i++;
> +		if (sinfo.filled | STATION_INFO_RX_BITRATE)
> +			data[i] = 100000 *
> +				cfg80211_calculate_bitrate(&sinfo.rxrate);
> +		i++;
> +
> +		if (sinfo.filled | STATION_INFO_SIGNAL_AVG)
> +			data[i] = abs(sinfo.signal_avg);
> +		i++;
> +
> +after_once:
> +		if (WARN_ON(i != (STA_STATS_LEN - STA_STATS_SURVEY_LEN))) {
> +			rcu_read_unlock();
> +			return;
> +		}
> +	}
> +
> +	i = STA_STATS_LEN - STA_STATS_SURVEY_LEN;
> +	/* Get survey stats for current channel only */
> +	survey.filled = 0;
> +	if (drv_get_survey(local, 0,&survey) != 0) {
> +		survey.filled = 0;
> +		data[i++] = 0;
> +	} else {
> +		data[i++] = survey.channel->center_freq;
> +	}
> +
> +	if (survey.filled&  SURVEY_INFO_NOISE_DBM)
> +		data[i++] = abs(survey.noise);
> +	else
> +		data[i++] = -1LL;
> +	if (survey.filled&  SURVEY_INFO_CHANNEL_TIME)
> +		data[i++] = survey.channel_time;
> +	else
> +		data[i++] = -1LL;
> +	if (survey.filled&  SURVEY_INFO_CHANNEL_TIME_BUSY)
> +		data[i++] = survey.channel_time_busy;
> +	else
> +		data[i++] = -1LL;
> +	if (survey.filled&  SURVEY_INFO_CHANNEL_TIME_EXT_BUSY)
> +		data[i++] = survey.channel_time_ext_busy;
> +	else
> +		data[i++] = -1LL;
> +	if (survey.filled&  SURVEY_INFO_CHANNEL_TIME_RX)
> +		data[i++] = survey.channel_time_rx;
> +	else
> +		data[i++] = -1LL;
> +	if (survey.filled&  SURVEY_INFO_CHANNEL_TIME_TX)
> +		data[i++] = survey.channel_time_tx;
> +	else
> +		data[i++] = -1LL;
> +
> +	rcu_read_unlock();
> +
> +	if (WARN_ON(i != STA_STATS_LEN))
> +		return;
> +
> +	drv_get_et_stats(sdata, stats,&(data[STA_STATS_LEN]));
> +}
> +
>
>   static int ieee80211_dump_station(struct wiphy *wiphy, struct net_device *dev,
>   				 int idx, u8 *mac, struct station_info *sinfo)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Greear April 12, 2012, 4:51 p.m. UTC | #2
On 04/12/2012 09:42 AM, Florian Fainelli wrote:
> Hi,
>
> Le 04/12/12 18:32, greearb@candelatech.com a écrit :
>> From: Ben Greear<greearb@candelatech.com>
>>
>> The signal and noise are forced to be positive since ethtool
>> deals in unsigned 64-bit values and this number should be human
>> readable. This gives easy access to some of the data formerly
>> exposed in the deprecated /proc/net/wireless file.
>
> Uh, that's misleading, the signal and noise values are typically negative, so one needs to think about mentally adding a minus sign if he/she wants to
> understand it. Does not ethtool know about 32-bits signed integers?

Ethtool stats only supports u64.  I think it's easy enough for
humans or programs to add the negative sign.  Can signal or noise
ever be > 0?  If so, that could actually break something that depends
on flipping the value to negative....

Thanks,
Ben
Jan Ceuleers April 12, 2012, 5:56 p.m. UTC | #3
Ben Greear wrote:
> Ethtool stats only supports u64. I think it's easy enough for
> humans or programs to add the negative sign. Can signal or noise
> ever be > 0? If so, that could actually break something that depends
> on flipping the value to negative....

The unit of these quantities is dBm, right? So if the {signal|noise} is 
greater than 1mW the corresponding value in dBm would be positive.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings April 12, 2012, 7:30 p.m. UTC | #4
On Thu, 2012-04-12 at 09:51 -0700, Ben Greear wrote:
> On 04/12/2012 09:42 AM, Florian Fainelli wrote:
> > Hi,
> >
> > Le 04/12/12 18:32, greearb@candelatech.com a écrit :
> >> From: Ben Greear<greearb@candelatech.com>
> >>
> >> The signal and noise are forced to be positive since ethtool
> >> deals in unsigned 64-bit values and this number should be human
> >> readable. This gives easy access to some of the data formerly
> >> exposed in the deprecated /proc/net/wireless file.
> >
> > Uh, that's misleading, the signal and noise values are typically negative, so one needs to think about mentally adding a minus sign if he/she wants to
> > understand it. Does not ethtool know about 32-bits signed integers?
> 
> Ethtool stats only supports u64.  I think it's easy enough for
> humans or programs to add the negative sign.  Can signal or noise
> ever be > 0?  If so, that could actually break something that depends
> on flipping the value to negative....

So far as I can see, the ethtool stats were expected to be counters,
which obviously cannot become negative (or fractional).  Maybe it's time
to define another command and string-set to cover other types of status
information.  The ethtool utility could ask for those typed statistics
as well, so 'ethtool -S' would get all of them.

Ben.
Georgiewskiy Yuriy April 12, 2012, 7:53 p.m. UTC | #5
On 2012-04-12 09:51 -0700, Ben Greear wrote Florian Fainelli:

BG>On 04/12/2012 09:42 AM, Florian Fainelli wrote:
BG>> Hi,
BG>> 
BG>> Le 04/12/12 18:32, greearb@candelatech.com a ?crit :
BG>> > From: Ben Greear<greearb@candelatech.com>
BG>> > 
BG>> > The signal and noise are forced to be positive since ethtool
BG>> > deals in unsigned 64-bit values and this number should be human
BG>> > readable. This gives easy access to some of the data formerly
BG>> > exposed in the deprecated /proc/net/wireless file.
BG>> 
BG>> Uh, that's misleading, the signal and noise values are typically negative,
BG>> so one needs to think about mentally adding a minus sign if he/she wants to
BG>> understand it. Does not ethtool know about 32-bits signed integers?
BG>
BG>Ethtool stats only supports u64.  I think it's easy enough for
BG>humans or programs to add the negative sign.  Can signal or noise
BG>ever be > 0?  If so, that could actually break something that depends
BG>on flipping the value to negative....

Don't know is this is a bug or it's reaaly can be positive, but:

iw dev mp0 station dump
Station 00:02:6f:b8:94:d3 (on mp0)
    inactive time:	49 ms
    rx bytes:	36318341
    rx packets:	271741
    tx bytes:	4180152
    tx packets:	35445
    tx retries:	7724
    tx failed:	123
    signal:  	1 dBm
    signal avg:	-2 dBm
    tx bitrate:	240.0 MBit/s MCS 13 40Mhz short GI
    rx bitrate:	180.0 MBit/s MCS 12 40Mhz short GI
    mesh llid:	8349
    mesh plid:	49801
    mesh plink:	ESTAB
    authorized:	yes
    authenticated:	yes
    preamble:	long
    WMM/WME:	yes
    FP:		no
    TDLS peer:	no
										    

C уважением                       With Best Regards
Георгиевский Юрий.                Georgiewskiy Yuriy
+7 4872 711666                    +7 4872 711666
факс +7 4872 711143               fax +7 4872 711143
Компания ООО "Ай Ти Сервис"       IT Service Ltd
http://nkoort.ru                  http://nkoort.ru
JID: GHhost@icf.org.ru            JID: GHhost@icf.org.ru
YG129-RIPE                        YG129-RIPE
Ben Greear April 12, 2012, 8:46 p.m. UTC | #6
On 04/12/2012 12:30 PM, Ben Hutchings wrote:
> On Thu, 2012-04-12 at 09:51 -0700, Ben Greear wrote:
>> On 04/12/2012 09:42 AM, Florian Fainelli wrote:
>>> Hi,
>>>
>>> Le 04/12/12 18:32, greearb@candelatech.com a écrit :
>>>> From: Ben Greear<greearb@candelatech.com>
>>>>
>>>> The signal and noise are forced to be positive since ethtool
>>>> deals in unsigned 64-bit values and this number should be human
>>>> readable. This gives easy access to some of the data formerly
>>>> exposed in the deprecated /proc/net/wireless file.
>>>
>>> Uh, that's misleading, the signal and noise values are typically negative, so one needs to think about mentally adding a minus sign if he/she wants to
>>> understand it. Does not ethtool know about 32-bits signed integers?
>>
>> Ethtool stats only supports u64.  I think it's easy enough for
>> humans or programs to add the negative sign.  Can signal or noise
>> ever be>  0?  If so, that could actually break something that depends
>> on flipping the value to negative....
>
> So far as I can see, the ethtool stats were expected to be counters,
> which obviously cannot become negative (or fractional).  Maybe it's time
> to define another command and string-set to cover other types of status
> information.  The ethtool utility could ask for those typed statistics
> as well, so 'ethtool -S' would get all of them.

One nice thing about ethtool stats API is that it is backwards and forwards
compatible for a long while.  Also, adding a new API means a second call
into the kernel (most likely) for the other set of strings, which effectively
doubles the cost of getting stats (and allows the stats to be updated
out-of-sync with each other more easily).

I wonder if instead we could add a convention where we add a short
prefix (or suffix) to a string to denote it's type (and cast the value
into u64).  So, an old ethtool might see:

foo:s32: 4294967295

while a new one understand that the s32: prefix is special, do
some casting, and could show:
foo: -1

Both are still at least somewhat human readable, and probably wouldn't
confuse anyone that is parsing the output of existing ethtool output.

Thanks,
Ben
Ben Hutchings April 12, 2012, 9:05 p.m. UTC | #7
On Thu, 2012-04-12 at 13:46 -0700, Ben Greear wrote:
> On 04/12/2012 12:30 PM, Ben Hutchings wrote:
> > On Thu, 2012-04-12 at 09:51 -0700, Ben Greear wrote:
> >> On 04/12/2012 09:42 AM, Florian Fainelli wrote:
> >>> Hi,
> >>>
> >>> Le 04/12/12 18:32, greearb@candelatech.com a écrit :
> >>>> From: Ben Greear<greearb@candelatech.com>
> >>>>
> >>>> The signal and noise are forced to be positive since ethtool
> >>>> deals in unsigned 64-bit values and this number should be human
> >>>> readable. This gives easy access to some of the data formerly
> >>>> exposed in the deprecated /proc/net/wireless file.
> >>>
> >>> Uh, that's misleading, the signal and noise values are typically negative, so one needs to think about mentally adding a minus sign if he/she wants to
> >>> understand it. Does not ethtool know about 32-bits signed integers?
> >>
> >> Ethtool stats only supports u64.  I think it's easy enough for
> >> humans or programs to add the negative sign.  Can signal or noise
> >> ever be>  0?  If so, that could actually break something that depends
> >> on flipping the value to negative....
> >
> > So far as I can see, the ethtool stats were expected to be counters,
> > which obviously cannot become negative (or fractional).  Maybe it's time
> > to define another command and string-set to cover other types of status
> > information.  The ethtool utility could ask for those typed statistics
> > as well, so 'ethtool -S' would get all of them.
> 
> One nice thing about ethtool stats API is that it is backwards and forwards
> compatible for a long while.

Agreed.

> Also, adding a new API means a second call
> into the kernel (most likely) for the other set of strings, which effectively
> doubles the cost of getting stats (and allows the stats to be updated
> out-of-sync with each other more easily).

It's generally not possible to take an atomic snapshot of all statistics
for a NIC, so that shouldn't be a consideration.

> I wonder if instead we could add a convention where we add a short
> prefix (or suffix) to a string to denote it's type (and cast the value
> into u64).  So, an old ethtool might see:
> 
> foo:s32: 4294967295

Actually it would be:

foo:s32: 18446744073709551615

> while a new one understand that the s32: prefix is special, do
> some casting, and could show:
> foo: -1
>
> Both are still at least somewhat human readable,

I don't think many humans can mentally substract 2^64.

> and probably wouldn't confuse anyone that is parsing the output of
> existing ethtool output.

I think you have this backwards: printing numbers in two different ways
(old and new versions of ethtool) is likely to confuse scripts that are
parsing and doing calculations with these numbers.  While I try to avoid
gratuitous changes in output formatting, scripts should use the ethtool
API if they really want interface stability.  It's not difficult (there
are at least Python and Perl bindings available) and it's a lot more
efficient.

Ben.
Ben Greear April 12, 2012, 9:21 p.m. UTC | #8
On 04/12/2012 02:05 PM, Ben Hutchings wrote:
> On Thu, 2012-04-12 at 13:46 -0700, Ben Greear wrote:
>> On 04/12/2012 12:30 PM, Ben Hutchings wrote:
>>> On Thu, 2012-04-12 at 09:51 -0700, Ben Greear wrote:
>>>> On 04/12/2012 09:42 AM, Florian Fainelli wrote:
>>>>> Hi,
>>>>>
>>>>> Le 04/12/12 18:32, greearb@candelatech.com a écrit :
>>>>>> From: Ben Greear<greearb@candelatech.com>
>>>>>>
>>>>>> The signal and noise are forced to be positive since ethtool
>>>>>> deals in unsigned 64-bit values and this number should be human
>>>>>> readable. This gives easy access to some of the data formerly
>>>>>> exposed in the deprecated /proc/net/wireless file.
>>>>>
>>>>> Uh, that's misleading, the signal and noise values are typically negative, so one needs to think about mentally adding a minus sign if he/she wants to
>>>>> understand it. Does not ethtool know about 32-bits signed integers?
>>>>
>>>> Ethtool stats only supports u64.  I think it's easy enough for
>>>> humans or programs to add the negative sign.  Can signal or noise
>>>> ever be>   0?  If so, that could actually break something that depends
>>>> on flipping the value to negative....
>>>
>>> So far as I can see, the ethtool stats were expected to be counters,
>>> which obviously cannot become negative (or fractional).  Maybe it's time
>>> to define another command and string-set to cover other types of status
>>> information.  The ethtool utility could ask for those typed statistics
>>> as well, so 'ethtool -S' would get all of them.
>>
>> One nice thing about ethtool stats API is that it is backwards and forwards
>> compatible for a long while.
>
> Agreed.

> Actually it would be:
>
> foo:s32: 18446744073709551615
>
>> while a new one understand that the s32: prefix is special, do
>> some casting, and could show:
>> foo: -1
>>
>> Both are still at least somewhat human readable,
>
> I don't think many humans can mentally substract 2^64.

Well, if we add a new API, then anyone on older ethtool
won't see it at all, which is even more useless than a
large ugly number.

Those of us using ethtool API directly would have to add new
ioctl calls (and the performance is important to me, even
if atomicity isn't so important).  That is more work than
adding some logic to parse suffixes on the strings I think.

>> and probably wouldn't confuse anyone that is parsing the output of
>> existing ethtool output.
>
> I think you have this backwards: printing numbers in two different ways
> (old and new versions of ethtool) is likely to confuse scripts that are
> parsing and doing calculations with these numbers.  While I try to avoid
> gratuitous changes in output formatting, scripts should use the ethtool
> API if they really want interface stability.  It's not difficult (there
> are at least Python and Perl bindings available) and it's a lot more
> efficient.

If the new ethtool -S is going to nicely present things (ie, show "foo: -1"),
then the negative numbers are there anyway, so maybe the compatibility issue
for anyone parsing the output of 'ethtool -S' is moot.  Anyone parsing the
binary API sees no changes, but *could* update code to look at the suffix
if they cared.

That said, I *would* like a new 'ethool get-stats' API that took a 'verbose'
argument so that we could return more or less verbose results (dependent on the
driver to determine what that means).  That way, we could probe easy-to-obtain
information quickly and often, and if there is something more expensive to obtain,
that could be probed less often.  If this idea is worth pursuing, then perhaps
it could also include a new binary API that includes a type identifier.

Thanks,
Ben
Ben Greear April 13, 2012, 10:01 p.m. UTC | #9
On 04/12/2012 12:53 PM, Georgiewskiy Yuriy wrote:
> On 2012-04-12 09:51 -0700, Ben Greear wrote Florian Fainelli:
>
> BG>On 04/12/2012 09:42 AM, Florian Fainelli wrote:
> BG>>  Hi,
> BG>>
> BG>>  Le 04/12/12 18:32, greearb@candelatech.com a ?crit :
> BG>>  >  From: Ben Greear<greearb@candelatech.com>
> BG>>  >
> BG>>  >  The signal and noise are forced to be positive since ethtool
> BG>>  >  deals in unsigned 64-bit values and this number should be human
> BG>>  >  readable. This gives easy access to some of the data formerly
> BG>>  >  exposed in the deprecated /proc/net/wireless file.
> BG>>
> BG>>  Uh, that's misleading, the signal and noise values are typically negative,
> BG>>  so one needs to think about mentally adding a minus sign if he/she wants to
> BG>>  understand it. Does not ethtool know about 32-bits signed integers?
> BG>
> BG>Ethtool stats only supports u64.  I think it's easy enough for
> BG>humans or programs to add the negative sign.  Can signal or noise
> BG>ever be>  0?  If so, that could actually break something that depends
> BG>on flipping the value to negative....
>
> Don't know is this is a bug or it's reaaly can be positive, but:
>
> iw dev mp0 station dump
> Station 00:02:6f:b8:94:d3 (on mp0)
>      inactive time:	49 ms
>      rx bytes:	36318341
>      rx packets:	271741
>      tx bytes:	4180152
>      tx packets:	35445
>      tx retries:	7724
>      tx failed:	123
>      signal:  	1 dBm
>      signal avg:	-2 dBm

So, how about I just cast it to u8 and pass that to user-space.
Let user-space apps that care just understand the data is really just
a signed 8-bit number for now.

For the future, we can figure out a way to make ethtool
API deal with different data types better.


	if (survey.filled & SURVEY_INFO_NOISE_DBM)
		data[i++] = (u8)(survey.noise & 0xff);
	else

...

Thanks,
Ben
diff mbox

Patch

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b37fb0d..99e3597 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -117,7 +117,9 @@  static const char ieee80211_gstrings_sta_stats[][ETH_GSTRING_LEN] = {
 	"rx_duplicates", "rx_fragments", "rx_dropped",
 	"tx_packets", "tx_bytes", "tx_fragments",
 	"tx_filtered", "tx_retry_failed", "tx_retries",
-	"beacon_loss"
+	"beacon_loss", "txrate", "rxrate", "signal",
+	"channel", "noise", "ch_time", "ch_time_busy",
+	"ch_time_ext_busy", "ch_time_rx", "ch_time_tx"
 };
 #define STA_STATS_LEN	ARRAY_SIZE(ieee80211_gstrings_sta_stats)
 
@@ -138,46 +140,6 @@  static int ieee80211_get_et_sset_count(struct wiphy *wiphy,
 	return rv;
 }
 
-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();
-
-	drv_get_et_stats(sdata, stats, &(data[STA_STATS_LEN]));
-}
-
 static void ieee80211_get_et_strings(struct wiphy *wiphy,
 				     struct net_device *dev,
 				     u32 sset, u8 *data)
@@ -531,6 +493,117 @@  static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
 		sinfo->sta_flags.set |= BIT(NL80211_STA_FLAG_TDLS_PEER);
 }
 
+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;
+	struct station_info sinfo;
+	struct survey_info survey;
+	bool do_once = true;
+	int i;
+#define STA_STATS_SURVEY_LEN 7
+
+	memset(data, 0, sizeof(u64) * STA_STATS_LEN);
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sta, &local->sta_list, list) {
+		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;
+
+		if (!do_once) {
+			i += 3;
+			goto after_once;
+		}
+
+		do_once = false;
+		sinfo.filled = 0;
+		sta_set_sinfo(sta, &sinfo);
+
+		if (sinfo.filled | STATION_INFO_TX_BITRATE)
+			data[i] = 100000 *
+				cfg80211_calculate_bitrate(&sinfo.txrate);
+		i++;
+		if (sinfo.filled | STATION_INFO_RX_BITRATE)
+			data[i] = 100000 *
+				cfg80211_calculate_bitrate(&sinfo.rxrate);
+		i++;
+
+		if (sinfo.filled | STATION_INFO_SIGNAL_AVG)
+			data[i] = abs(sinfo.signal_avg);
+		i++;
+
+after_once:
+		if (WARN_ON(i != (STA_STATS_LEN - STA_STATS_SURVEY_LEN))) {
+			rcu_read_unlock();
+			return;
+		}
+	}
+
+	i = STA_STATS_LEN - STA_STATS_SURVEY_LEN;
+	/* Get survey stats for current channel only */
+	survey.filled = 0;
+	if (drv_get_survey(local, 0, &survey) != 0) {
+		survey.filled = 0;
+		data[i++] = 0;
+	} else {
+		data[i++] = survey.channel->center_freq;
+	}
+
+	if (survey.filled & SURVEY_INFO_NOISE_DBM)
+		data[i++] = abs(survey.noise);
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME)
+		data[i++] = survey.channel_time;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_BUSY)
+		data[i++] = survey.channel_time_busy;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_EXT_BUSY)
+		data[i++] = survey.channel_time_ext_busy;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_RX)
+		data[i++] = survey.channel_time_rx;
+	else
+		data[i++] = -1LL;
+	if (survey.filled & SURVEY_INFO_CHANNEL_TIME_TX)
+		data[i++] = survey.channel_time_tx;
+	else
+		data[i++] = -1LL;
+
+	rcu_read_unlock();
+
+	if (WARN_ON(i != STA_STATS_LEN))
+		return;
+
+	drv_get_et_stats(sdata, stats, &(data[STA_STATS_LEN]));
+}
+
 
 static int ieee80211_dump_station(struct wiphy *wiphy, struct net_device *dev,
 				 int idx, u8 *mac, struct station_info *sinfo)