netdev - Re: [PATCH] flexcan: Fix CAN_RAW_RECV_OWN_MSGS and CAN_RAW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 03 Nov 2011 15:54:50 +0100
From:	Oliver Hartkopp <socketcan@...tkopp.net>
To:	Reuben Dowle <Reuben.Dowle@...ico.com>
CC:	netdev@...r.kernel.org, linux-can@...r.kernel.org
Subject: Re: [PATCH] flexcan: Fix CAN_RAW_RECV_OWN_MSGS and CAN_RAW_LOOPBACK

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html