Re: [RFC 2/2] mac80211: Support getting sta_info stats via ethtool.

[Ben Greear <greearb@candelatech.com>]

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 <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com