Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 08.09.25 18:31, Vincent Mailhol wrote:
> On 08/09/2025 at 23:59, Oliver Hartkopp wrote:
>> On 08.09.25 15:30, Vincent Mailhol wrote:
>>> On 08/09/2025 at 18:00, Oliver Hartkopp wrote:
> 
> (...)
> 
>>>> There is no way to have CAN CC with CAN XL without CAN FD right now as we can
>>>> not detect this without accessing the netlink configuration.
>>>
>>> This brings us to the next point I wanted to discuss. As you say, the only
>>> solution is to access the ctlrmode flags which, at the moment, are only exposed
>>> through the netlink interface.
>>>
>>> But using the netlink interface directly in your program is a bit troublesome,
>>> to say the least, because of all the boilerplate code needed as illustrated in
>>> the libsocketcan:
>>>
>>>     https://github.com/linux-can/libsocketcan/blob/master/src/libsocketcan.c
>>>
>>> So, my other idea would be to add a new socket option that would act as a
>>> shortcut to priv->ctrlmode.
>>>
>>>     getsockopt(s, SOL_CAN_RAW, CAN_RAW_CTRLMODE, &ctrlmode, sizeof(ctrlmode));
>>>
>>> The interface would return an error if the interface is a virtual interface.
>>> Otherwise, it would return the flags, allowing for an easier way to probe. As a
>>> bonus, all the flags become easily accessible.
>>>
>>> It means that we would have two different ways to do the same thing (netlink and
>>> getsockopt) but I do not see this as an issue.
>>>
>>> What do you think?
>>
>> You are right with having two APIs for the same thing ...
>>
>> If we would have such a getsockopt() I would suggest to provide the ctrlmode and
>> the ctrlmode_supported to the user space.
> 
> Yes, and probably also the CAN_RAW_CTRLMODE_STATIC. I do not often use that one,
> but for completeness, better to expose all three.
> 
>> And the bits are only valid when the interface is up. So additional to e.g. -
>> EOPNOTSUPP for vcan's we should also be able to return -ENETDOWN.
> 
> Ack for the vcan.
> 
> For the real hardware, I think that the CAN_RAW_CTRLMODE_SUPPORTED and
> CAN_RAW_CTRLMODE_STATIC should be always available. Only the CAN_RAW_CTRLMODE
> would return -ENETDOWN if the interface is down.
> 
>> My biggest concern is whether such a getsockopt() is really needed.
>>
>> Today you can enable CANFD and CANXL with setsockopt() and when you send frames
>> that can not be sent to the interface you get an error.
>>
>> The only thing that is "not that nice" is the CANXL-only (without FD) possibility.
> 
> The fact that vcan can not be configured with FD off and XL on is what I would
> call a "not that nice".
> 
> But the impossibility to reliably probe the FD state is worse than "not that
> nice". When I see cangen sending CAN FD frames when it shouldn't, I call this a
> bug. The impossibility to probe is a no-go to me.
> 
> An alternative would be to revive the libsocketcan to make it easier to use the
> netlink interface but this is lot of effort and it means adding one userland
> dependency. The getsockopt() seems a better alternative.

I think the best way is to properly split up the FD/XL capabilities for 
CAN XL interfaces. Right now CAN FD is implicitly enabled on CAN XL 
interfaces when CAN XL is enabled.

I would tend to change the code in raw.c in a way that you have to 
switch on FD/XL explicitly. And then the problem would directly show up 
in write() and FD frames would not be silently dropped in 
can_dev_dropped_skb().

Best regards,
Oliver