Fwd: Re: Request for feedback for CAN message structure
I am forwarding Oliver Hartkopp's founded reply to the list for archival and rest of the RTEMS community -- Forwarded Message -- Subject: Re: Request for feedback for CAN message structure Date: Tuesday 21 of June 2022, 08:55:03 From: Oliver Hartkopp To: Pavel Pisa , devel@rtems.org CC: Prashanth S Hello all, I'm definitely with Pavel's suggestions here! I added some small remarks regarding the current CAN XL discussion in-line: On 20.06.22 22:57, Pavel Pisa wrote: > Hello Prashanth S and others, > > excuse me for lower activity last weeks. We have exams period > and I have lot of other duties and was even ill. I am at Embedded > World in Nuremberg now, so may it be there is some chance to meet > somebody other from RTEMS community. > > As for the e-mail it is better to add one of my personal e-mails > (p...@cmp.felk.cvut.cz or pp...@pikron.com) > to notice me for something important, I do not check every > message comming through my list subscription. I go through > RTEMS, NuttX, CAN, etc. forums only when I have spare time. > > On Monday 20 of June 2022 18:37:38 Prashanth S wrote: >> This is a request for suggestions and feedback for the CAN message >> structure. >> >> *CAN message structure that represents the can messages from application to >> driver:* >> >> struct can_message { >> uint32_t timestamp; >> uint32_t id; // 32 bits to support extended id (29 bits) >> uint16_t flags;// RTR | BRS | EXT ... >> uint8_t dlc; // (0 - 8) | 12 | 16 | 20 | 24 | 32 | 48 | 64 >> uint8_t res; // reserved for alignment. >> uint8_t data[64]; // For supporting data transfer up to 64 bytes. >> }; >> >> This is the CAN messages structure created based on the suggestions in the >> mail chain and looking through other CAN solutions (Nuttx, GRCAN, LinCAN). > > Yes I like solution with explicit and aligned fields. > I confirm that I think that length representing real data length is better > and it has been chosen for CAN FD by SocketCAN. Yes. Handling the DLC/Length conversion in every user program is a mess. That's why we actually changed the dlc to len even for Classical CAN frames in the struct can_frame: struct can_frame { canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */ union { /* CAN frame payload length in byte (0 .. CAN_MAX_DLEN) * was previously named can_dlc so we need to carry that * name for legacy support */ __u8 len; __u8 can_dlc; /* deprecated */ } __attribute__((packed)); /* disable padding added in some ABIs */ __u8 __pad; /* padding */ __u8 __res0; /* reserved / padding */ __u8 len8_dlc; /* optional DLC for 8 byte payload length (9 .. 15) */ __u8 data[CAN_MAX_DLEN] __attribute__((aligned(8))); }; You might also think about a union to carry the Classical CAN length and the CAN FD length in one 8 bit value and add the extra Classical CAN raw DLC in another 8 bit value - and make a union with a 16 bit length value which is needed for CAN XL. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/can.h?id=ea7800565a128c1adafa1791ce80afd6016fe21c But in fact the raw DLC value has been introduced only because of some people doing security testing. The Classical CAN raw DLC value has no value for normal CAN applications and therefore using a simple 16 bit length value would be the best choice IMHO (see below). > I confirm that flags implemented with explicit sized field seem to be more > safe to me. Bitfields are tricky due to endianing and have problem with > initialization and copying when new field is added. > > I am not sure if the field name "dlc" (data length code) is right > when the field represents real length, may be "len" is a better > match. Yes, definitely! > I think that fields order makes sense as well (alignment, purpose etc..). > Only think to consider is possibility to swap > >uint32_t timestamp; >uint32_t id; > > because id is the first and the most important and accessed field, > so when the pointer to structure is give it can be small win > on some targets that it can be accessed without offset addition. > But that is probably neglectable and there can be arguments against. The 'id' really defines the CAN frame so it can be seen as a 'key' - like in a data base. It makes sense to put it first. Btw. adding RTR and EXT to the flags field is something I would do today too. Good choice! > Another think to consider is a size of "dlc" and or "len" code. > There is ongoing standardization process for CAN XL an
Re: Request for feedback for CAN message structure
Hello Pavel and Prashanth, Am 20.06.22 um 22:57 schrieb Pavel Pisa: Hello Prashanth S and others, excuse me for lower activity last weeks. We have exams period and I have lot of other duties and was even ill. I am at Embedded World in Nuremberg now, so may it be there is some chance to meet somebody other from RTEMS community. As for the e-mail it is better to add one of my personal e-mails (p...@cmp.felk.cvut.cz or pp...@pikron.com) to notice me for something important, I do not check every message comming through my list subscription. I go through RTEMS, NuttX, CAN, etc. forums only when I have spare time. On Monday 20 of June 2022 18:37:38 Prashanth S wrote: This is a request for suggestions and feedback for the CAN message structure. *CAN message structure that represents the can messages from application to driver:* struct can_message { uint32_t timestamp; uint32_t id; // 32 bits to support extended id (29 bits) uint16_t flags;// RTR | BRS | EXT ... uint8_t dlc; // (0 - 8) | 12 | 16 | 20 | 24 | 32 | 48 | 64 uint8_t res; // reserved for alignment. uint8_t data[64]; // For supporting data transfer up to 64 bytes. }; This is the CAN messages structure created based on the suggestions in the mail chain and looking through other CAN solutions (Nuttx, GRCAN, LinCAN). Yes I like solution with explicit and aligned fields. I confirm that I think that length representing real data length is better and it has been chosen for CAN FD by SocketCAN. I confirm that flags implemented with explicit sized field seem to be more safe to me. Bitfields are tricky due to endianing and have problem with initialization and copying when new field is added. I am not sure if the field name "dlc" (data length code) is right when the field represents real length, may be "len" is a better match. I think that fields order makes sense as well (alignment, purpose etc..). Only think to consider is possibility to swap uint32_t timestamp; uint32_t id; because id is the first and the most important and accessed field, so when the pointer to structure is give it can be small win on some targets that it can be accessed without offset addition. But that is probably neglectable and there can be arguments against. Another think to consider is a size of "dlc" and or "len" code. There is ongoing standardization process for CAN XL and it would allow CAN frames up to 2kB and the length will be with byte granularity. But CAN XL adds datatype filed and much more options so it is probably out of the actual RTEMS scope and would require variable length read and write systemcalls because 2kB overhead for some 1 or 2 byte message is too high. So even 8 bit length field is OK for now and may be res field can solve CAN XL compatibility one day. Not sure where the field will be used but wouldn't it be better if length is 16 bit already if there is an upcoming standard that will support lengths that need it. Otherwise some drivers might use the "res" for something else or make some assumptions about the length of the field that make it harder to change it later. Best regards Christian So generally I agree with the message structure, I declare minor weight vote to switch from "dlc" to "len". I think that switch to common message structure based API on RTEMS would be big win and that (at least for now) keeping character device like API with IOCTLs is simpler and matches better actual RTEMS CAN implementations. Best wishes, Pavel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: Request for feedback for CAN message structure
Hello Prashanth S and others, excuse me for lower activity last weeks. We have exams period and I have lot of other duties and was even ill. I am at Embedded World in Nuremberg now, so may it be there is some chance to meet somebody other from RTEMS community. As for the e-mail it is better to add one of my personal e-mails (p...@cmp.felk.cvut.cz or pp...@pikron.com) to notice me for something important, I do not check every message comming through my list subscription. I go through RTEMS, NuttX, CAN, etc. forums only when I have spare time. On Monday 20 of June 2022 18:37:38 Prashanth S wrote: > This is a request for suggestions and feedback for the CAN message > structure. > > *CAN message structure that represents the can messages from application to > driver:* > > struct can_message { > uint32_t timestamp; > uint32_t id; // 32 bits to support extended id (29 bits) > uint16_t flags;// RTR | BRS | EXT ... > uint8_t dlc; // (0 - 8) | 12 | 16 | 20 | 24 | 32 | 48 | 64 > uint8_t res; // reserved for alignment. > uint8_t data[64]; // For supporting data transfer up to 64 bytes. > }; > > This is the CAN messages structure created based on the suggestions in the > mail chain and looking through other CAN solutions (Nuttx, GRCAN, LinCAN). Yes I like solution with explicit and aligned fields. I confirm that I think that length representing real data length is better and it has been chosen for CAN FD by SocketCAN. I confirm that flags implemented with explicit sized field seem to be more safe to me. Bitfields are tricky due to endianing and have problem with initialization and copying when new field is added. I am not sure if the field name "dlc" (data length code) is right when the field represents real length, may be "len" is a better match. I think that fields order makes sense as well (alignment, purpose etc..). Only think to consider is possibility to swap uint32_t timestamp; uint32_t id; because id is the first and the most important and accessed field, so when the pointer to structure is give it can be small win on some targets that it can be accessed without offset addition. But that is probably neglectable and there can be arguments against. Another think to consider is a size of "dlc" and or "len" code. There is ongoing standardization process for CAN XL and it would allow CAN frames up to 2kB and the length will be with byte granularity. But CAN XL adds datatype filed and much more options so it is probably out of the actual RTEMS scope and would require variable length read and write systemcalls because 2kB overhead for some 1 or 2 byte message is too high. So even 8 bit length field is OK for now and may be res field can solve CAN XL compatibility one day. So generally I agree with the message structure, I declare minor weight vote to switch from "dlc" to "len". I think that switch to common message structure based API on RTEMS would be big win and that (at least for now) keeping character device like API with IOCTLs is simpler and matches better actual RTEMS CAN implementations. Best wishes, Pavel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: Request for feedback for CAN message structure
IIRC Pavel asked to always being on cc to get CAN related email right to his inbox instead of email list boxes... On 6/20/22 18:37, Prashanth S wrote: Hi All, This is a request for suggestions and feedback for the CAN message structure. *CAN message structure that represents the can messages from application to driver:* struct can_message { uint32_t timestamp; uint32_t id; // 32 bits to support extended id (29 bits) uint16_t flags; // RTR | BRS | EXT ... uint8_t dlc; // (0 - 8) | 12 | 16 | 20 | 24 | 32 | 48 | 64 uint8_t res; // reserved for alignment. uint8_t data[64]; // For supporting data transfer up to 64 bytes. }; This is the CAN messages structure created based on the suggestions in the mail chain and looking through other CAN solutions (Nuttx, GRCAN, LinCAN). Regards Prashanth S ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Request for feedback for CAN message structure
Hi All, This is a request for suggestions and feedback for the CAN message structure. *CAN message structure that represents the can messages from application to driver:* struct can_message { uint32_t timestamp; uint32_t id; // 32 bits to support extended id (29 bits) uint16_t flags;// RTR | BRS | EXT ... uint8_t dlc; // (0 - 8) | 12 | 16 | 20 | 24 | 32 | 48 | 64 uint8_t res; // reserved for alignment. uint8_t data[64]; // For supporting data transfer up to 64 bytes. }; This is the CAN messages structure created based on the suggestions in the mail chain and looking through other CAN solutions (Nuttx, GRCAN, LinCAN). Regards Prashanth S ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel