All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Greear <greearb@candelatech.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2 2/6] mac80211: Support getting sta_info stats via ethtool.
Date: Wed, 18 Apr 2012 09:27:24 -0700	[thread overview]
Message-ID: <4F8EEB6C.1030605@candelatech.com> (raw)
In-Reply-To: <1334721645.3672.2.camel@jlt3.sipsolutions.net>

On 04/17/2012 09:00 PM, Johannes Berg wrote:
> On Tue, 2012-04-17 at 20:46 -0700, Ben Greear wrote:
>
>>>> +	rcu_read_lock();
>>>> +	list_for_each_entry_rcu(sta,&local->sta_list, list) {
>>>
>>> This doesn't seem right -- shouldn't it look up the BSSID or something
>>> and only work on managed interfaces? What if there really are two
>>> stations on this interface -- then it'll just overwrite it and return a
>>> random station's data? That's useless.
>>
>> Well, its weird at least.
>>
>> But, if there are multiple stations, like for APs??, then it will
>> add the station's stats together.  Perhaps not horribly useful, but better
>> than nothing.
>
> Oh, right, it's adding, I missed that. But is that really useful?

It provides a summary for AP, and precise stats for managed station
mode (excepting TDLS where it may return sums, it seems).

I could just return all zeros for non managed
station interfaces, but I *have* to return some value, so it seems
little loss to just add the station stats together.

>> For managed interface, I *think* they don't have more than one station, right?
>
> You can't rely on it. Typically they will, but with TDLS there might be
> multiple.
>
>> And, as for the underlying driver stats and survey stats (in later patches),
>> that is only probed once.  I guess if you somehow had two
>> stations on different channels on the same network device,
>> the survey stats would be a bit dodgy, but it does return
>> the freq for the stats in question, so at least you know
>> what you are getting.
>
> A single netdev is always going to be on a single channel. Actually, I
> take that back, I think TDLS can work out of channel too, but we don't
> support that right now.

So, based on your response, a sum still seems most useful to me.  The ethtool
API gives no way to request any subset of stats, so all I can key off of
is the netdev.

I have some plans percolating to add some new ethtool API to get subsets
of stats, but that is likely a ways off, and of course it may not be
accepted regardless.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


  reply	other threads:[~2012-04-18 16:27 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-17 17:46 [PATCH v2 0/6] Add ethtool stats support for Wireless Devices greearb
2012-04-17 17:46 ` [PATCH v2 1/6] cfg80211: Add framework to support ethtool stats greearb
2012-04-17 17:46 ` [PATCH v2 2/6] mac80211: Support getting sta_info stats via ethtool greearb
2012-04-18  1:37   ` Johannes Berg
2012-04-18  3:46     ` Ben Greear
2012-04-18  4:00       ` Johannes Berg
2012-04-18 16:27         ` Ben Greear [this message]
2012-04-18 22:39           ` Johannes Berg
2012-04-18 22:59             ` Ben Greear
2012-04-19  4:38               ` Johannes Berg
2012-04-17 17:46 ` [PATCH v2 3/6] mac80211: Framework to get wifi-driver " greearb
2012-04-17 17:46 ` [PATCH v2 4/6] wireless: Add util method to get channel index from frequency greearb
2012-04-18  1:40   ` Johannes Berg
2012-04-18  3:36     ` Ben Greear
2012-04-17 17:46 ` [PATCH v2 5/6] mac80211: Add more ethtools stats: survey, rates, etc greearb
2012-04-18  1:41   ` Johannes Berg
2012-04-18  3:31     ` Ben Greear
2012-04-18  4:05       ` Johannes Berg
2012-04-18 16:19         ` Ben Greear
2012-04-18 22:40           ` Johannes Berg
2012-04-18 22:54             ` Ben Greear
2012-04-19  4:37               ` Johannes Berg
2012-04-17 17:46 ` [PATCH v2 6/6] mac80211: Add sta_state to ethtool stats greearb
2012-04-18  1:42   ` Johannes Berg
2012-04-18  1:44 ` [PATCH v2 0/6] Add ethtool stats support for Wireless Devices Johannes Berg
2012-04-18  3:56   ` Ben Greear

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F8EEB6C.1030605@candelatech.com \
    --to=greearb@candelatech.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.