Hello Pavel,
On 18.07.22 13:03, Pavel Pisa wrote:
Hello Oliver,
On Sunday 17 of July 2022 15:27:25 Oliver Hartkopp wrote:
V2: Major rework after discussion and feedback on Linux-CAN ML
- rework of struct canxl_frame
- CANXL_XLF flag is now the switch between CAN XL and CAN/CANFD
- variable length in r/w operations for CAN XL frames
- write CAN XL frame to raw socket enforces size <-> canxl_frame.len sync
I generally like the idea but I would like even to extend it to process
all CAN messages types through same API.
+struct canxl_frame {
+ canid_t prio; /* 11 bit priority for arbitration (canid_t) */
+ __u8 flags; /* additional flags for CAN XL */
+ __u8 sdt; /* SDU (service data unit) type */
+ __u16 len; /* frame payload length in byte */
+ __u32 af; /* acceptance field */
+ __u8 data[CANXL_MAX_DLEN];
+};
I would suggest to think about possibility to have single structure type
for processing of all CAN frames types in usersace.
Yes, I would love to do this too.
But the length information in the CAN/CANFD frame is a 8 bit (u8)
element while we need 16 bit in CAN XL frames which can cover the 2048
bytes available in CAN XL.
So we can not place canxl_frame.len at the same place as can[fd]_frame.len.
And this leads to 'the trick' that canxl_frame.flags shares its position
with can[fd]_frame.len.
With CANXL_XLF = 0x80 we switch between CAN XL and CAN/CANFD which would
lead to can[fd]_frame.len = 128 which is way beyond a valid CAN/CANFD
length value.
When you define field
prio as anonyous union of prio and can_id
union {
canid_t prio;
canid_t can_id;
}
The CAN XL specification has no can_id anymore - but canxl_frame.prio
and can[fd]_frame.can_id share the same position, length and type (canid_t).
This allows us to re-use the CAN-ID filters in af_can.c for the
canxl_frame.prio.
then it is possible to define flags such way, that canxl_frame is alternative
for all other formats
Ah, now I got your point.
But e.g. you don't need 'sdt' and 'af' elements in CAN[FD] frames and
you would need raw-DLC fields for Classical CAN.
This will break all in-kernel users (bcm, gw, isotp, j1939).
+#define CANXL_XLF 0x80 /* mandatory CAN XL frame flag (must always be set!) */
So CANXL_XLF will be changed to CANXLS_SELECT frame structure selected,
then rest of bits can be used for CANXLS_XLF, CANXLS_FD, in ideal case even
RTR, BRS etc.. or RTR can be left as part of ID if that is easier.
This way only single structure can be used to receive and send all
frames over single interface when XL option is selected
by software.
Yes, there would be redundancy in the kernel handling which has to accept
two types of encoding of CAN FD and standard messages but actual complexity
in usespace when you want to support all variants and for example
forward frames between interfaces or process them inside QEMU etc.
is really quite high.
IMHO it is very easy for CAN interfaces (as you can see in the vcan
driver (patch 4/5).
CAN XL is quite different to CAN[FD]. I would name it "Ethernet with CAN
arbitration" and for that reason I'm currently not thinking of adapting
the CAN modification features in gw.c for CAN XL.
If you go through the patches 2/5 and 3/5 you can see the current
infrastructure effort to handle CAN, CAN FD and CAN XL frames in parallel.
When working with a mix of CAN[|FD|XL] frames in user space I would
simply read into a struct canxl_frame and cast it to CAN[FD] when
CANXL_XLF is not set (and the sizeof(struct can[fd]_frame) and the len
value fits.
Don't you think, that is easy enough?
Best regards,
Oliver
