On 02/18/2011 05:03 PM, Oliver Hartkopp wrote: > On 18.02.2011 11:03, Wolfgang Grandegger wrote: > >>>>>>> cf->data[5] = CAN_ERR_VERSION; >>>>>>> >>>>>>> Also candump could use it to avoid displaying rubbish with old kernels. >>>>> IMO it's not so good put the version in a 'hot path'. >>>> >>>> What do you mean with hot path. >>> 'hot path' == every CAN error frame will carry the version info. >>> Opposed to uname() where the version is transferred once. >>> >>> Or, in other words, the version isn't going to change, but is transferred >>> every time... >> >> We always send all 8 bytes and CAN_ERR_VERSION would not change that. > > Whatever people might place there in specific setups - or if we need some more > space in the future ... > > This version info transferred in the hotpath for some less relevant backward > compatibility stuff is not a very good idea. > > I think calling 'int uname(struct utsname *buf);' once at startup is the only > thing we should do to support a kernel version < 2.6.39 - which is unsafe due > to the current inconsistencies of the drivers anyway. > > >>>>>>> I tend to drop touching the network stats completely. What do you and >>>>>>> others think. >>>>>> >>>>>> Using the common network stats would have the advantage, that you can >>>>>> easily >>>>>> see problems with standard tools like 'ifconfig' or 'cat /proc/net/dev' >>>>> >>>>> After dropping network stats, some other stats mechanism will appear with >>>>> even less defined semantics, since some kind of stats are usefull (we even >>>>> have some extra stats for CAN errors!). >>>>> It is a bit messy now, I agree. >>>> >>>> I thin mantaining and incementing too similar stats is overkill. >>>> Also, we have difficulties to map the CAN errors to the netdev errors: >>>> >>>> http://lxr.linux.no/#linux+v2.6.37/include/linux/netdevice.h#L173 >>>> >>> Do you question the stats like 'bus_error', or do you question >>> having them seperate? >>> I see value in keeping such stats ... >> >> I want to drop incrementing the netdev stats errors. The CAN stats >> should be kept, of course. Currently, we increment both stats for bus >> errors, e.g.: >> >> can_stats->bus_errors++; >> stats->rx_errors++; >> > > Indeed updating two variables is not a good idea. > > Regarding the given example ... what about incrementing only > > stats->rx_errors++; /* counting bus errors with netdev stats */ > > And in the case the CAN specific statistics are requested, we do some > > can_stats->bus_errors = stats->rx_errors; > > before passing the statistics to the user?
First, not all bus errors are associated with rx: ACK errors may happen on TX CRC errors may happen on RX BIT errors may happen on RX *and* TX FORM errors may happen on RX *and* TX Mapping bus CAN errors to netdev errors is not always possible. Second, currently we increment stats->rx_errors for non-bus-errors as well, e.g.: stats->rx_over_errors++; stats->rx_errors++; Any better idea? Wolfgang. _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
