On 12.07.22 12:19, Vincent Mailhol wrote:
On Tue. 12 Jul. 2022 at 18:31, Oliver Hartkopp <socketcan@xxxxxxxxxxxx> wrote:
+/* truncated CAN XL structs must contain at least 64 data bytes */
+#define CANXL_MINTU (CANXL_MTU - CANXL_MAX_DLEN + CANFD_MAX_DLEN)
I did not get the concept of the "truncated CAN XL structs". The valid
data field lengths are 1 to 2048, right? I did not get where this 64
comes from.
Your formula is equivalent to
#define CANXL_MINTU (sizeof(struct canxl_frame) + CANFD_MAX_DLEN)
No. CANXL_MINTU becomes sizeof(struct canfd_frame) + sizeof(af)
So I wanted some size value that is 'more than' CANFD_MTU so that you
know that you have read a CANXL frame.
Even if the cxf->len would be 14 you would at least copy a struct
canxl_frame with data[64].
OK, I finally got your point. Your concern is that if skb->len could
be equal or less than CANFD_MTU, then there would be a collision.
My approach here would be to stop using the MTU correlation to
differentiate between CAN(-FD) and CANXL. Instead, I suggest using
can{fd,xl}_frame::flags. If can{fd,xl}_frame has a CANXL flag set,
then it is a CANXL frame regardless of the value of skb->len. If the
CANXL flag is not set, then skb->len is used to differentiate between
Classic CAN and CAN FD (so that we remain compatible with the
existing). That way, no need to impose a minimum length of
CANFD_MAX_DLEN.
Hm, that sounds interesting! I like that as it looks clear and simple.
Need to pick a coffee to think about potential (security) effects ... ;-)
E.g. we would need to keep skb->len and canxl_frame::len in sync now.
I would have just expected:
#define CANXL_MINTU (sizeof(struct canxl_frame))
That is CANXL_MTU (max transfer unit).
I was writing while thinking that canxl_frame::data was a flexible
array member as suggested in this thread.
In that case canxl_frame::data counts as zero when doing sizeof(struct
canxl_frame). And so sizeof(struct canxl_frame) == sizeof(struct
canfd_frame) + sizeof(af).
Actually, thinking twice, the Minimum transfer unit would be:
#define CANXL_MINLEN 1
#define CANXL_MINTU (sizeof(struct canxl_frame) + CANXL_MINLEN)
(I forgot that the length can not be zero anymore in CANXL...)
I still would suggest to have the struct canxl_frame contain the 2048
byte of data (data[CANXL_MAXDLEN]) - as the entire CAN XL frame is
defined like this in the CAN XL spec. This would be also in common with
CAN/CANFD.
E.g. when reading into the struct canxl_frame you always have a defined
data structure which can contain a complete CAN XL frame.
But if you get or send less than that size (when reading/writing) this
would be ok now (with your idea with CANXL_XLF set).
E.g.
#define CANXL_MINDLEN 1
#define CANXL_MAXDLEN 2048
#define CANXL_MTU sizeof(struct canxl_frame)
#define CANXL_HEAD_SZ (CANXL_MTU - CANXL_MAXDLEN)
#define CANXL_MINTU (CANXL_HEAD_SZ + CANXL_MINDLEN)
(..)
Here, you lost me. The only reference document I have is the Bosch
presentation you linked in the cover letter. I would need to get a
copy of the specification first to follow up on this level of details.
There is a Special Interest Group for CAN XL at CAN in Automation
(can-cia.org) and these doscuments are currently under development.
I wonder how hard it is to join the group. Right now, I was thinking
of waiting for the ISO Final Draft International Standard (FDIS) to
deep dive in CANXL.
I'm not sure if the SDT definitions will be part of an ISO draft as
defining the content inside a CAN XL frame is probably not ISO standard
relevant.
The content definitions are industrial recommendations to increase
interoperability. You are always free to put into the CAN XL frame
whatever you want.
E.g. CAN-CiA defined the pinout of the SUB-D9 connectors for CAN (Pin
2/7) which is not defines in ISO too.
Best regards,
Oliver
