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

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 15.09.25 20:54, Vincent Mailhol wrote:
> On 16/09/2025 at 03:08, Oliver Hartkopp wrote:

>> I think the interface to set the MTU lacks a clear separation of how to set the
>> MTU for real (hardware) CAN interfaces and virtual CAN interfaces.
> 
> Ack.
> 
>> 1. IMO we should be able to set the MTUs on virtual and real interfaces when the
>> interface is down (as those MTUs have no effect at this time).
> 
> Mostly agreed. It should not be possible to switch between the Classical CAN,
> CAN FD or CAN XL MTUs. But I do not yet see an issue to change the MTU to
> something in between CANXL_MIN_MTU and CANXL_MAX_MTU while a CAN XL node is running.

No, not while it is running (== up).
My point was that you can set the MTU as long as the interface is not 
"up". Together with the default initial values (see below) this makes 
perfect sense to me.

> I want to first study the other interfaces (e.g. ethernet) and the core net
> infrastructure in order to make my mind. For the moment, I am just undecided.

I'm not sure if ethernet is a good example for our use-case with 
different CAN protocols types (CC/FD/XL) which is more than having 
ethernet frames of different length.

>> 2. When a virtual interface is set to "up" the MTU is used and fixed.
> 
> Same as above, mostly agreed aside from the CAN XL on which I do not yet have my
> final opinion.
> 
>> 3. When a real interface is set to up the mtu is set to ...
>>    a. mtu = CAN_MTU when fd off and xl off
>>    b. mtu = CANFD_MTU when fd on and xl off
>>    c. mtu = the configured CAN XL MTU (*) when xl on
>>
>> (*) when the configured mtu is not in the range of CANXL_MIN_MTU and
>> CANXL_MAX_MTU the mtu is set to CANXL_MAX_MTU.
>>
>> By default the initial MTU of virtual CAN interfaces should be set to CANXL_MTU.
>>
>> By default the initial MTU of real CAN interfaces should be set to the maximum
>> value which the real CAN interface is capable too:
>>   a. CAN_CTRLMODE_XL supported -> CANXL_MTU.
>>   b. CAN_CTRLMODE_FD supported -> CANFD_MTU.
>>   c. default CAN_MTU
> 
> I was thinking of the opposite:
> 
>    a. if the device is CAN FD static it is CANFD_MTU
>    b. if the device is CAN XL static it is CANXL_MTU
>    c. otherwise, it is the CAN_MTU by default
> 
> which, if you remove point b., happens to be the current logic. I do not see a
> need to change this.

I like that approach of having the supported MTUs as default and later 
reduce the MTU based on fd=off and xl=off, because it would be similar 
with the virtual CAN configuration then.

I assume the initial MTU isn't looked at by the users anyway.

> If we set CANXL_MTU by default on XL capable devices, it means that at a moment
> in time, we have a device with the CAN_CTRLMODE_XL off but with a CAN XL MTU.

???

Maybe I was not clear enough:

You intitialize the MTU to CANXL_MTU when CAN_CTRLMODE_XL is a 
"supported mode". The interface is not "up" at this time and therefore 
the MTU is not on active service.

Then you configure the interface with bitrates and xl/fd on/off.

And then you set the interface to "up" and in this process the MTU is 
set as a valid and activated value with a MTU based on the xl/fd on/off 
settings. This was my idea.

> And this is inconsistent. For me, the MTU should always match the control mode
> flags. Because all control modes are off at the beginning, the MTU is thus the
> Classical CAN one.
> 
>> I think this should make it clearer and fix the current inconsistency.
>>
>> Setting the CANFD_MTU via the ip set mtu feature and expect "fd on" being set at
>> the same time is bad.
> 
> OK. Aside of a few details, I think we agree on the big lines. The good thing is
> that the current can_change_mtu() only targets the real interfaces. The virtual
> ones already have their own functions and so will not get impacted.

Right. The virtual CAN stuff can stay as-is.

> So I am just thinking of doing a full rewrite of can_change_mtu(). The old logic
> of being able to implicitly set the fd flag by providing an MTU will go to the
> trash can.

Yes. That was not consistent and clear in the usage.

With my suggestion the can_change_mtu() will be just a simple setting of 
values which is the same for real and virtual interfaces.
For real interfaces we might make some additional checks against the 
"supported modes" for FD and XL.

And when the real interface is set to "up" the MTU is adjusted to the 
real cc/fd/xl configuration values. I hope this makes it clear now.

> The new logic will try to follow as much as possible the intended MTU
> logic of the core net framework (which I am still studying).

Don't expect too much for our use-case there ;-)

Best regards,
Oliver