Re: [RFC PATCH v5 2/2] can: reject CAN FD content when disabled on CAN XL interfaces

[Vincent Mailhol <mailhol@kernel.org>]

On 11/09/2025 at 05:12, Oliver Hartkopp wrote:
> On 10.09.25 18:19, Vincent Mailhol wrote:
>> On 10/09/2025 at 17:48, Oliver Hartkopp wrote:
>>> On 10.09.25 09:40, Vincent Mailhol wrote:
>>>> On 10/09/2025 at 16:27, Oliver Hartkopp wrote:
>>>
>>>>>>       /* CAN XL is allowed on virtual interfaces if it fits the MTU */
>>>>>>          if (!priv)
>>>>>>              return dev->mtu == CANXL_MTU;
>>>>>
>>>>>           return can_is_canxl_dev_mtu(mtu);
>>>>>
>>>>> The MTU of CAN XL interfaces might vary.
>>>>
>>>> Maybe this is something that we discussed before, I do not remember, but how is
>>>> it that the MTU can vary?
>>>>
>>>> MTU is the *Maximum* Transmission Unit. I understand that the size of a CAN XL
>>>> frame is variable, but the MTU should be constant, right? Why can it vary?
>>>
>>> Depending on the realtime requirements the length of the CAN frames (and
>>> therefore the time the bus is blocked) the MTU can be reduced. This is (like the
>>> bitrate settings) a network architects decision which is enforced by setting the
>>> MTU accordingly.
>>
>> Is this an extension we offer in Socket CAN?
> 
> Yes.
> 
>> The standard says nothing about
>> having the MTU configurable.
>>
>> For CAN FD, we forcefully set the MTU in netlink.c
>>
>> https://elixir.bootlin.com/linux/v6.16/source/drivers/net/can/dev/netlink.c#L228
> 
> Oh. I did not realize before that we can either modify the MTU with setting fd
> on/off and via setting the MTU in can_change_mtu()
> 
> https://elixir.bootlin.com/linux/v6.16/source/drivers/net/can/dev/dev.c#L313
> 
> ?!?
> 
> The two APIs problem for changing the MTU?!?
> 
> I expected the default MTU for CAN FD capable interfaces to be CANFD_MTU which
> is obviously not the case.
> 
>> I will have to think of what are the implication for CAN XL.
> 
> I would define a default CANXL MTU (CANXL_MTU 2060) which might be changed with
> can_change_mtu().
> 
> And when ever we switch xl on this value is selected as device MTU.
> 
> Or the user can change the MTU as he needs it.
> And when xl on is selected and the MTU is a can_is_canxl_dev_mtu() this value is
> used. When can_is_canxl_dev_mtu() is not true we take CANXL_MTU.
> 
> Something like this.

Yes. I was thinking of something similar. This is what I added locally at the
moment:

	if ((priv->ctrlmode & CAN_CTRLMODE_XL) &&
	    !can_is_canxl_dev_mtu(dev->mtu)) {
  		/* Set CAN XL MTU to its max unless if already set by user */
		dev->mtu = CANXL_MAX_MTU;
	}

But I am still testing it.


Yours sincerely,
Vincent Mailhol