Re: More flags for logging

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hi Ayoub,

On 04.05.21 11:49, Ayoub Kaanich wrote:

> There seems to be a mis-conception about how WireShark/Libpcap handle SocketCAN:
> 
> There is no ethertype involved, the data of the frame will directly start with the CAN ID.
> 
> The socket CAN in PCAP format pre-date CAN-FD (Developed in 2011), so at the time, there was only "CAN" (Introduced in Oct, 2009).
> See https://github.com/the-tcpdump-group/libpcap/commit/86ed15cc8b4bd7dc1458559108e66a07ec6721ec
> The introduction of CAN-FD later on did not change that.

Ah, now I got the problem.

I was heavily involved in introducing CAN FD into Wireshark here:

https://github.com/wireshark/wireshark/commit/7fad354a3e379382368cd1ef67b841315c29e050#diff-f0fd5f515da65110cd5c1231b43180693e1d46779765d65c997e32da11ae70d1

But this obviously had NO impact on libpcap :-/

> Libpcap library maps both LINUX_SLL_P_CAN and LINUX_SLL_P_CANFD (they are treated identically the same way) to DLT_CAN_SOCKETCAN and copy bytes as is (the only change is changing the endianness if ID if needed).

So this would mean, we need a separate handler for LINUX_SLL_P_CANFD ??

Best regards,
Oliver

> 
> The MTU solution still not does provide a fix for the case of a normal CAN message being transported over a CAN-FD bus.
> In this case, application cannot tell if frame was physically transported as CAN or CAN-FD.
> See https://noelscher.com/blog/posts/compatibility-of-can-and-can-fd/
> 
> Best Regards.
> 
> Ayoub Kaanich.
> 
> -----Original Message-----
> 
> From: mailto:socketcan@hartkopp.net
> Sent: Tuesday, May 4, 2021 10:49 AM
> To: mailto:mkl@pengutronix.de; mailto:linux-can@vger.kernel.org
> Cc: mailto:kayoub5@live.com
> Subject: Re: More flags for logging
> 
> 
> 
> On 03.05.21 12:08, Marc Kleine-Budde wrote:
>> On 03.05.2021 12:02:46, Marc Kleine-Budde wrote:
>>> The SocketCAN API is a great initiative for standardizing the CAN
>>> interface API. This request tries to extend this initiative for more
>>> use cases.
>>>
>>> Context:
>>>
>>> The SocketCAN was adopted by libpcap and tcpdump as the standard
>>> format for logging CAN Frames in PCAP and PCAP-NG. See:
>>>
>>> https://www.tcpdump.org/linktypes/LINKTYPE_CAN_SOCKETCAN.html
>>> https://github.com/wireshark/wireshark/blob/master/epan/dissectors/pa
>>> cket-socketcan.c https://www.wireshark.org/docs/dfref/c/can.html
>>>
>>> Problem:
>>> Applications that perform logging, usually need more details that
>>> normal applications, for the sake of debugging later on. Flags needs
>>> to be reserved/allocated in the SocketCAN API, so that logging
>>> applications can make use of them in the PCAP format. The flags does
>>> not need necessary need to be implemented by SocketCAN, they just
>>> need to be marked as reserved for such use case.
>>>
>>> Needed Flags:
>>> FDF Flag
>>> - Since CAN Frames and CAN-FD frames can co-exist in the same bus,
>>>      logging application needs to know if a normal CAN Frame was
>>>      transmitted on a CAN-FD bus, namely was the FDF bit set or not.
>>
>> I think someone asked for that some time ago. But that was never
>> mainlined. I'll look for that old mail.
>>
> 
> When you display CAN and CAN FD frames in Wireshark they are displayed as different "protocols" - as they also have different ethtypes.
> 
> So the difference is provided by the 'protocol' field. Or did I miss something?
> 
> Regards,
> Oliver
> 
>>> ACK bit in data frame
>>> - Some logging hardware can act as a "spy", meaning that it records
>>> CAN
>>>      Frames, but does not set the ACK bit
>>> - A way to know for a given frame (FD or not), was the ACK bit set or
>>>      not.
>>> - Current API allow detecting missing ACK, but it does not report
>>> what
>>>      Frame had a missing ACK.
>>
>> This means the driver has to set a flag if it's configured in
>> listen-only mode. That should be possible.
>>
>> I think we can make use of flags in the struct canfd_frame for this:
>>
>> | struct canfd_frame {
>> |      canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>> |      __u8    len;     /* frame payload length in byte */
>> |      __u8    flags;   /* additional flags for CAN FD */
>> |      __u8    __res0;  /* reserved / padding */
>> |      __u8    __res1;  /* reserved / padding */
>> |      __u8    data[CANFD_MAX_DLEN] __attribute__((aligned(8)));  };
>>
>> The struct can_frame doesn't have the flags member yet, but we can add
>> it there.
>>
>> regards,
>> Marc
>>
>