Search Linux Wireless

Re: [PATCH v2 5/6] mac80211: Add more ethtools stats: survey, rates, etc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2012-04-17 at 20:31 -0700, Ben Greear wrote:
> On 04/17/2012 06:41 PM, Johannes Berg wrote:
> > On Tue, 2012-04-17 at 10:46 -0700, greearb@xxxxxxxxxxxxxxx wrote:
> >> +	/* Get survey stats for current channel */
> >> +	survey.filled = 0;
> >> +	if (drv_get_survey(local, 0,&survey) != 0) {
> >> +		survey.filled = 0;
> >> +		data[i++] = 0;
> >> +	} else {
> >> +		/* ath9k (and maybe others??) only updates internal stats
> >> +		 * when you get channel index 0, so if
> >> +		 * we are NOT on channel zero, get the real stats
> >> +		 * now.
> >> +		 */
> >> +		int ch_idx = ieee80211_get_channel_idx(local->hw.wiphy,
> >> +			local->oper_channel->center_freq);
> >> +		if (ch_idx == 0) {
> >> +			data[i++] = survey.channel->center_freq;
> >> +		} else {
> >> +			survey.filled = 0;
> >> +			if (drv_get_survey(local, ch_idx,&survey) != 0) {
> >> +				survey.filled = 0;
> >> +				data[i++] = 0;
> >> +			} else {
> >> +				data[i++] = survey.channel->center_freq;
> >> +			}
> >> +		}
> >> +	}
> >
> > This is completely incomprehensible to me. I don't think the channel
> > index is what you think it is?
> 
> It is ugly, but as far as I can tell it works.  The survey method
> wants an index, not channel number or frequency, from what I can tell
> (I just looked at and tested ath9k).

But the get_survey method's index has no relation whatsoever to the
channel. It's just a cookie identifier.

> Based on the freq returned as part of the survey results, the logic
> above appears to function as I would hope.

Well then that's because ath9k happens to add the survey results in that
order, that is certainly not guaranteed. Some drivers only give you
survey results for index 0, and the channel contained in that index is
always just the channel that you're on right now.

> That said, a new get-survey() method that took a channel object instead of an
> index would make this code cleaner.  Or, we could store the channel
> index in the channel object for fast lookup, but I was afraid storing the
> index would be too risky in case channels are ever moved around for some reason
> or another.
> 
> And, I could be missing something yet....

Yeah I think you're missing how get_survey() works :-)

For every index in get_survey() it returns the channel & the information
for that channel, the index<->channel mapping is not fixed at all.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux