Re: More flags for logging

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 16.05.21 01:46, Ayoub Kaanich wrote:
> Hi,
> 
> Sorry for taking so long to reply, I wanted to check the FDF topic 
> thoroughly first.
> 

No problem ;-)

> 
> The documentation of the canfd_frame states that "The use of struct 
> canfd_frame implies the Extended Data Length (EDL) bit to be set in the 
> CAN frame bitstream on the wire."

Thanks for the hint. This comment in include/uapi/linux/can.h has been 
created before the renaming that bit to FDF and needs an update.

> There have been editorial changes when the FD Frame Format was 
> introduced into ISO 11898-1,the bit EDL is now named FDF (FD Format).
> 
> See 
> https://www.can-cia.org/fileadmin/resources/documents/proceedings/2013_hartwich_v2.pdf
> 
> I believe that the fact that "EDL" and "FDF" are the same thing should 
> be clarified to avoid future confusion.

I was personally involved in CAN FD standardization and suggested the 
renaming, as "extended data length" (EDL) did not completely catch the 
CAN FD features. This renaming EDL -> FDF is a replacement for 
clarification. No confusion here :-)

> According to 
> https://www.kernel.org/doc/html/latest/networking/can.html#raw-socket-option-can-raw-fd-frames
> 
> When performing a read operation on a socket that had CAN_RAW_FD_FRAMES 
> enabled, both CAN_MTU and CANFD_MTU are allowed.
> 
> I originally though that only one of them is allowed at a time, that was 
> a mistake from my side.

You might want to take a look into this slide deck (slide 48ff):

https://wiki.automotivelinux.org/_media/agl-distro/agl2017-socketcan-print.pdf

> So using MTU will work for FD, but will definitely be a problem in the 
> future with CAN XL due to its large size (more than 2KB)

Yes. But we should then thing about CAN_RAW_XL_FRAMES in the same manner.

> The last question would be, would libpcap keep the extra padding bytes 
> in, or remove them.
> 
> Since the document in 
> https://www.tcpdump.org/linktypes/LINKTYPE_CAN_SOCKETCAN.html is not 
> conclusive about the topic (the diagram makes it look like it's truncated)
> 
> I will have to check on an actual system during the work days, and push 
> a PR to tcpdump if it turns out it's just the diagram that was misleading.

In fact this documentation is outdated as we updated the struct 
can_frame for the 'Classical CAN' (not CAN FD) to contain a raw data 
length code (DLC) value in the formerly reserved space:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=ea7800565a128c1adafa1791ce80afd6016fe21c

Btw. I would still be interested to get the CANFD_FDF reservation for 
user space application programmers into Mainline:

https://lore.kernel.org/linux-can/20170411134343.3089-1-socketcan@hartkopp.net/

So let's see whether Marc can be excited on this patch ;-)

Best regards,
Oliver


> 
> 
> Best Regards.
> 
> 
> On 2021-05-04 10:49 AM, Oliver Hartkopp wrote:
>>
>>
>> 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/packet-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
>>>