On 2013-04-28 5:15 PM, Ben Greear wrote: > On 04/28/2013 08:08 AM, Felix Fietkau wrote: >> On 2013-04-28 4:54 PM, Ben Greear wrote: >>> On 04/28/2013 05:51 AM, Felix Fietkau wrote: >>>> On 2013-04-27 5:25 PM, Oleksij Rempel wrote: >>>>> Collect statistics about recived duplicate and STBC packets. >>>>> This information should help see if STBC is actually working. >>>>> >>>>> Tested on ar9285; >>>>> >>>>> Signed-off-by: Oleksij Rempel <linux@xxxxxxxxxxxxxxxx> >>>> I thought about this patch some more, and I'm wondering what's the point >>>> in doing this? These statistics are going to be completely useless for >>>> most people and they'll waste some memory/cpu cycles, especially on >>>> small-cache devices. I think it's much more useful to simply pass the >>>> information to mac80211 via rx flags and get them added to the radiotap >>>> header. >>>> I'd like to keep the number of 'poor man's debug hacks' in the driver to >>>> a minimum, and there are some other things that I think should be >>>> removed: rx_frags and rx_beacons in struct ath_rx_stats, the tx/rx MAC >>>> sampling hack, and pretty much anything else that can be just as easily >>>> accessed from mac80211 through regular interfaces. >>> >>> Does that mean we can just put the stats in mac80211, or do we have >>> to be running a sniffer to gather the stats? >> Right now you'd have to use a sniffer, but if you really care about >> getting specific stats it might make sense to write a kernel module that >> attaches to a monitor interface and gathers them (maybe even with >> support for gathering arbitrary stats by attaching bpf filters). > > I'd like ongoing stats w/out a monitor interface or sniffer, though > it would be fine to add it to the sniffer path as well. Maybe we can > have compile-time flag to enable the extra and mostly useless > stats so only the 1% that cares pays the overhead. I'd like to have proper justification for stats added to the code and kick out stuff not worth keeping. It's not just about runtime overhead, but keeping the number of lines of code down is important for maintenance as well. Adding extra compile time flags just makes the maintenance part worse. >> The problem I have with the current stats is they're just an arbitrary >> collection of random stuff that is probably useless for 99% of all >> users. In many cases the way the stats are collected also makes the data >> completely meaningless (e.g. because the source/destination address is >> not taken into account). >> >> Why care about the number of packets on the air that were sent with a >> specific rate flag? Why care about the number of beacons on the air >> (with no filter on a set of APs or anything)? Or what about the number >> of fragments received? To me it just looks like an incoherent set of >> useless facts. > > I think having lots of stats allows for the possibility of seeing a pattern > that might otherwise be missed, and when testing in an isolated environment, > you don't have to care about who is sending what being reported..you already > know. The stats I pointed out seem mostly useless for that purpose. When testing in an isolated environment you might as well run a capture on a monitor mode interface and do some more sophisticated analysis afterwards. > One thing I'd like to do, for instance, is to figure out how to get some > average MPDU lengths calculated in hopes that would correlate with decreased > performance in cases were we have lots of stations.... Please don't throw hooks for such stacks all over the ath9k or mac80211 data path. Making a module for analyzing such stuff on a monitor mode interface might be a bit more work than spraying hacks onto the code, but at least it doesn't push the maintenance/performance overhead burden onto everybody else. Something like a BPF based statistics gathering module would be useful for more people as well - I'd probably help with the code myself as well. I made a proof of concept of such a module several years ago. It was based on madwifi back then, but the code is mostly generic. You can find it here: http://git.openwrt.org/?p=packages.git;a=tree;f=net/wprobe/src When implemented properly, such am module would even make things a lot easier for you by letting you add new stats at runtime without changing kernel code or reloading modules. So let's stop wasting time on keeping lots of tiny useless hacks around and instead build something more useful. :) - Felix -- 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