Hi Vincent,
On 21.10.20 08:23, Vincent MAILHOL wrote:
From a first thought I would see a new flag CAN_CTRLMODE_RAW_DLC in the
netlink interface of IFLA_CAN_CTRLMODE for the CAN controller driver.
This could switch the sanitizing AND the CAN controller can properly
expose its ability to support this mode.
Absolutely yes. In my first message, I mentioned the idea of managing
that through socket option, glad that we now share the same idea.
Actually, I just realized that I replied to you too quickly. I was not
exactly thinking of the same thing here so let me correct what I
previously said.
IFLA_CAN_CTRLMODE is at the netlink level. My idea is to have it, in
addition, at the socket level. Example: add CAN_RAW_RAW_DLC in
include/uapi/linux/can/raw.h.
Yes. We need at least some different handling inside the driver level
which could be switched with CAN_CTRLMODE_RAW_DLC.
The reason is that if we only manage it at the netlink level, some
application not aware of the RAW_DLC issue might run into some buffer
overflow issue. Unless an application directly requests it, the current
behaviour should be maintained (rationale: do not break userland).
So the full picture will be to have both the CAN_CTRLMODE_RAW_DLC at
netlink level and CAN_RAW_RAW_DLC at the socket level (in the exact same
way we have both CAN_CTRLMODE_FD and CAN_RAW_FD_FRAMES for
CAN-FD).
Yes. That hits the point.
Btw. the impact on all protocols seems to be pretty heavy and to me it
turned out that it would be a bad idea to use can_frame.can_dlc as
transport vehicle for the raw dlc. Especially as this will contradict
the rule that the can_dlc element is a plain length information today as
canfd_frame.len and shares the same position in the struct.
I currently tend to only have a switch on driver level with
CAN_CTRLMODE_RAW_DLC and make use of can_frame._res0 -> can_frame.raw_dlc
With CAN_CTRLMODE_RAW_DLC enabled the CAN driver would ...
- fill can_dlc and raw_dlc at reception time
- will use raw_dlc instead of can_dlc at tx time
- will use can_dlc if raw_dlc == 0 at tx time
This would have a minimal impact on the code and we only need to make
sure that the raw_dlc is not killed at some stage in the network layer.
#define CAN_MAX_RAW_DLC 15
IMO the raw dlc use-case is a really special case for testing purposes.
Touching the current can_frame.can_dlc handling could lead to more
complexity and the fear to run into overflows as already stated by Joakim.
What do you think about the above approach?
Regards,
Oliver
