On Thu, Feb 17, 2011 at 08:17:41PM +0100, Wolfgang Grandegger wrote:
> On 02/17/2011 12:05 AM, Kurt Van Dijck wrote:
> > On Wed, Feb 16, 2011 at 09:31:30PM +0100, Oliver Hartkopp wrote:
> >> On 16.02.2011 21:04, Wolfgang Grandegger wrote:
> >>> On 02/16/2011 08:00 PM, Oliver Hartkopp wrote:
> >>>> On 16.02.2011 09:10, Wolfgang Grandegger wrote:
> >>
> >>
[...]
> >>>>> We then could or even should add a version information into the error
> >>>>> message. I think data[5] could be used for that purpose.
> >>>>
> >>>> It's like having the CAN core version information where Dave was
> >>>> complaining
> >>>> when it gone Mainline. Who should check version numbers and maintain all
> >>>> this?
> >>>
> >>> This would allow users already using the error flags to easily migrate
> >>> to the new and changed error types. My idea here is:
> >>>
> >>> 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...
> It just adds a version dependency to the
> error message. So it's always clear what formatting has been used.
getsockopt() ?
>
> >> Hm - maybe then it's better to retrieve the Kernel version with uname(2)
> >> ...
> >>
> >>> I started writing the patch and realized that we need some more
> >>> clarification and cleanup. Next point is how we increment the
> >>> stats->[r|t]x_error counters? It's not done consistently and it's not
> >>> even possible to assign the direction of bus errors properly. Have a
> >>> look to the drivers to realizes the mess. Should we do that at all? We
> >>> have our own stats of CAN errors:
> >>>
> >>> struct can_device_stats {
> >>> __u32 bus_error; /* Bus errors */
> >>> __u32 error_warning; /* Changes to error warning state */
> >>> __u32 error_passive; /* Changes to error passive state */
> >>> __u32 bus_off; /* Changes to bus off state */
> >>> __u32 arbitration_lost; /* Arbitration lost errors */
> >>> __u32 restarts; /* CAN controller re-starts */
> >>> };
> >>>
> >>> 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 ...
Kurt
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
