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? Regards, Oliver _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
