Re: [PATCH] flexcan: Fix CAN_RAW_RECV_OWN_MSGS and CAN_RAW_LOOPBACK

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 02.11.2011 22:31, Reuben Dowle wrote:

>>>
>>> If we change the behaviour of flexcan, I think at91 and slcan should
>> be changed too, because this should be handled consistently across all
>> drivers I would think.
>>
>>
>> At least the slcan driver does not support the correct echo of CAN
>> frames on
>> driver level at all. The slcan driver adapts CAN frames to a serial
>> line CAN
>> adapter where the feedback of successful transmission is not
>> guaranteed/checked within the serial data stream. Therefore the
>> interface flag
>> IFF_ECHO is not set in the slcan driver and the local echo is performed
>> in
>> af_can.c (as a workaround).
>>
>> Regards,
>> Oliver
> 
> Thats fine for the echo behaviour, but does not really answer why the slcan
> (and flexcan) driver is setting the stats for tx_packets and tx_bytes at 
> different points in the transmission logic. To my mind, either we think the
> packet has transmitted (along with its bytes) so we should increment both,
> or we should increment neither.


In slcan.c the points of incrementing tx_bytes and tx_packets have been
adopted from slip.c where slcan.c heavily bases on.

Indeed one could argument that tx_packets could be incremented in slc_encaps
together with tx_bytes. This means that the packet is sent.

But the packet is surely sent in slcan_write_wakeup(), when tx_packets is
incremented. Unfortunately we do not have the can_dlc available at this point
anymore.

If you think it's worth the effort tx_packets++ could be integrated into
slc_encaps() to have a consistent statistic update (but you don't know whether
the packet hit the serial line entirely).

Additionally slip.c does it the same (inconsistent) way for the reasons above
and therefore i tend not to change it to stay on the current serial netdevices
behaviour (for slcan as only CAN netdevice).

Regards,
Oliver