On 14.07.22 03:23, Vincent Mailhol wrote:
On Thu. 14 Jul. 2022 at 05:02, Oliver Hartkopp <socketcan@xxxxxxxxxxxx> wrote:
If we follow your idea, use __u8 xlsec in order to avoid undefined behaviours.
Where we define
#define CANXL_TAG 0x7F
Here, you "burn" all the flags for future usage.
FYI, this doesn't have to be 0x7F. It could be any of the length
values not allowed by CAN-FD, namely (in decimal): 9-11, 13-15, 17-19,
21-23, 25-31, 33-47, 49-63
Yes, I detected this issue too when waking up this morning ...
#define CANXL_SEC 0x80
I did not get why CANXL_XLF isn't needed anymore.
which has to be set in the xlsec element then.
With this move we get rid of any flags problems (we only need the SEC
bit anyway) and define a clear 'escape value' in the former length element.
If I try to bounce on your idea, I will propose:
/*********** begin **********/
struct canxl_frame {
canid_t prio; /* 11 bit priority for arbitration (canid_t) */
__u8 flags; /* additional flags for CAN XL. MSB must be set */
__u8 sdt; /* SDU (service data unit) type */
__u16 len; /* frame payload length in bytes */
__u32 af; /* acceptance field */
__u8 data[];
};
ACK.
#define CANXL_XLF 0x01 /* mark CAN XL for dual use of struct canfd_frame */
IMO the dual use stuff between CAN FD & CAN XL is not working anymore
and became obsolete here.
CAN_XLF needs to be a hard switch for CAN XL - no optional thing.
#define CANXL_SEC 0x02 /* Simple Extended Content (security/segmentation) */
#define CANXL_DISCRIMINATOR 0x80; /* Mandatory to distinguish between
CAN(-FD) and XL frames */
/*********** end ************/
This has no undefined behaviour and we still have five flags (0x04 to
0x40) for future use.
I would suggest the following:
#define CANXL_XLF 0x80 /* mark CAN XL frame (must be set) */
#define CANXL_SEC 0x40 /* Simple Extended Content (security/segmentation) */
And the rest of the bits are reserved (shall be set to zero).
This way the CAN_XLF value would make the former 'len' element 128 -
which is a proper distinction for CAN XL frames as such length value
surely bounces on CAN/CANFD frames.
Best regards,
Oliver
