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

[Vincent Mailhol <mailhol@kernel.org>]

On 10/09/2025 at 16:27, Oliver Hartkopp wrote:
> On 10.09.25 07:13, Vincent Mailhol wrote:
>> On 10/09/2025 at 01:36, Oliver Hartkopp wrote:
>>> Hi Vincent,
>>>
>>> On 09.09.25 11:24, Oliver Hartkopp wrote:
>>>
>>>
>>>> -static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff
>>>> *skb, int mtu)
>>>> +static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff
>>>> *skb,
>>>> +                      struct net_device *dev)
>>>>    {
>>>>        /* Classical CAN -> no checks for flags and device capabilities */
>>>>        if (can_is_can_skb(skb))
>>>>            return CAN_MTU;
>>>>    -    /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
>>>> -    if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb) &&
>>>> -        (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
>>>> -        return CANFD_MTU;
>>>> +    /* CAN FD -> needs to be enabled in a CAN FD or CAN XL device */
>>>> +    if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb)) {
>>>> +        /* real/virtual CAN FD interface */
>>>> +        if (dev->mtu == CANFD_MTU)
>>>> +            return CANFD_MTU;
>>>> +        if (can_is_canxl_dev_mtu(dev->mtu) &&
>>>> +            can_dev_ctrlmode_fd_on(dev))
>>>> +            return CANFD_MTU;
>>>> +    }
>>>
>>> I've simplified the above code and rewrote the commit message in v6
>>>
>>>>        /* CAN XL -> needs to be enabled and a CAN XL device */
>>>>        if (ro->xl_frames && can_is_canxl_skb(skb) &&
>>>> -        can_is_canxl_dev_mtu(mtu))
>>>> +        can_is_canxl_dev_mtu(dev->mtu))
>>>>            return CANXL_MTU;
>>>
>>> We might also discuss if we create a can_dev_ctrlmode_xl_on(dev) function to
>>> check if the CAN XL interface has CAN_CTRLMODE_XL enabled.
>>
>> I checked ISO 11898-1:2024 again and the relevant wording I can found is in
>> §6.6.21.4.1:
>>
>>    When error signalling is disabled, the node shall transmit and
>>    receive only XLFF frames. It shall not transmit EF, OF and RF, nor
>>    DFs in CBFF, CEFF, FBFF and FEFF.
>>
>> TLDR; Classical CAN is deactivated when error signaling is off.
>>
>> So, I think that we also need the same logic for the Classical CAN. The nuance
>> is that instead of using CAN_CTRLMODE_CC (which does not exist), we can check
>> CAN_CTRLMODE_XL_ERR_SIGNAL. Note that CAN_CTRLMODE_XL_TMS implies that error
>> signalling is off, so no need for extra checks on TMS. This is what I have in
>> mind at the moment.
>>
>>    static inline bool can_dev_cc_on(struct net_device *dev)
>>    {
>>        struct can_priv *priv = safe_candev_priv(dev);
>>
>>        /* Classical CAN frames are always allowed on virtual interfaces */
>>        if (!priv)
>>            return true;
>>
>>        /* When error signalling is off only CAN XL frames are allowed */
>>        return !(priv->ctrlmode & CAN_CTRLMODE_XL) ||
>>            (priv->ctrlmode & CAN_CTRLMODE_XL_ERR_SIGNAL);
>>    }
>>
>>    static inline bool can_dev_fd_on(struct net_device *dev)
>>    {
>>        struct can_priv *priv = safe_candev_priv(dev);
>>
>>     /* CAN FD is allowed on virtual interfaces if it fits the MTU */
>>        if (!priv)
>>            return dev->mtu == CANFD_MTU;
>>
>>        return can_dev_cc_on(dev) && (priv->ctrlmode & CAN_CTRLMODE_FD);
>>    }
>>
>>    static inline bool can_dev_xl_on(struct net_device *dev)
>>    {
>>        struct can_priv *priv = safe_candev_priv(dev);
>>
>>     /* 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?


Yours sincerely,
Vincent Mailhol