[RFC PATCH v5 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb()

[RFC PATCH v5 1/2] can: skb: enforce CANFD_FDF check in can_is_canfd_skb()

[Oliver Hartkopp]

The check in can_is_canfd_skb() is about a length check of skb->len and
the CAN FD data length. As a skb length of CANFD_MTU can potentially be
created with a CAN XL frame with a data length of 60, the length check of
the CAN FD data length is used to detect CAN XL frames via its CANXL_XLF
flag which exceeds valid CAN FD data length values.

To make sure the CANFD_FDF flag can be safely used as a marker for CAN FD
frame skbs the bit is set in can_send() which is used by all PF_CAN
protocols. In the RX path alloc_canfd_skb() sets the CANFD_FDF flag.

The enforced CANFD_FDF check in can_is_canfd_skb() clears up the potential
uncertainty when using the skb->len check with the CANFD_MTU.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 include/linux/can/skb.h | 19 +++++++++++++++++--
 net/can/af_can.c        |  2 +-
 net/can/raw.c           |  2 +-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h
index 1abc25a8d144..09ab4dc83199 100644
--- a/include/linux/can/skb.h
+++ b/include/linux/can/skb.h
@@ -107,18 +107,33 @@ static inline bool can_is_can_skb(const struct sk_buff *skb)
 
 	/* the CAN specific type of skb is identified by its data length */
 	return (skb->len == CAN_MTU && cf->len <= CAN_MAX_DLEN);
 }
 
-static inline bool can_is_canfd_skb(const struct sk_buff *skb)
+static inline bool can_is_canfd_skb_mtu_len(const struct sk_buff *skb)
 {
 	struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
 
-	/* the CAN specific type of skb is identified by its data length */
+	/* The CAN specific type of skb is identified by its data length.
+	 * A CAN XL frame skb might have a skb->len of CANFD_MTU but the
+	 * skb would have the CANXL_XLF bit set (0x80 = 128) in the
+	 * cfd->len field position which would intentionally break the
+	 * CAN FD length check here.
+	 */
 	return (skb->len == CANFD_MTU && cfd->len <= CANFD_MAX_DLEN);
 }
 
+static inline bool can_is_canfd_skb(const struct sk_buff *skb)
+{
+	struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
+
+	if (!can_is_canfd_skb_mtu_len(skb))
+		return false;
+
+	return cfd->flags & CANFD_FDF;
+}
+
 static inline bool can_is_canxl_skb(const struct sk_buff *skb)
 {
 	const struct canxl_frame *cxl = (struct canxl_frame *)skb->data;
 
 	if (skb->len < CANXL_HDR_SIZE + CANXL_MIN_DLEN || skb->len > CANXL_MTU)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index b2387a46794a..7fd2ed510440 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -207,11 +207,11 @@ int can_send(struct sk_buff *skb, int loop)
 
 	if (can_is_canxl_skb(skb)) {
 		skb->protocol = htons(ETH_P_CANXL);
 	} else if (can_is_can_skb(skb)) {
 		skb->protocol = htons(ETH_P_CAN);
-	} else if (can_is_canfd_skb(skb)) {
+	} else if (can_is_canfd_skb_mtu_len(skb)) {
 		struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
 
 		skb->protocol = htons(ETH_P_CANFD);
 
 		/* set CAN FD flag for CAN FD frames by default */
diff --git a/net/can/raw.c b/net/can/raw.c
index 76b867d21def..e5e3952b0e09 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -886,11 +886,11 @@ static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
 	/* 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(skb) &&
+	if (ro->fd_frames && can_is_canfd_skb_mtu_len(skb) &&
 	    (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
 		return CANFD_MTU;
 
 	/* CAN XL -> needs to be enabled and a CAN XL device */
 	if (ro->xl_frames && can_is_canxl_skb(skb) &&
-- 
2.47.3

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

[Oliver Hartkopp]

The CAN XL devices can be configured as CAN XL only with 'xl on fd off'.
Correctly reject CAN FD frames when FD is disabled for the outgoing CAN XL
interface.

Virtual CAN interfaces (vcan, vxcan) are not affected as they have no such
CAN CC/FD/XL content configuration option.

Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 include/linux/can/dev.h | 12 ++++++++++++
 net/can/raw.c           | 20 +++++++++++++-------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 9a92cbe5b2cb..9fa139cc793e 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -183,10 +183,22 @@ struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
 void free_candev(struct net_device *dev);
 
 /* a candev safe wrapper around netdev_priv */
 struct can_priv *safe_candev_priv(struct net_device *dev);
 
+static inline bool can_dev_ctrlmode_fd_on(struct net_device *dev)
+{
+	struct can_priv *priv = safe_candev_priv(dev);
+
+	/* check ctrlmode on real CAN interfaces */
+	if (priv)
+		return (priv->ctrlmode & CAN_CTRLMODE_FD);
+
+	/* virtual CAN FD/XL interfaces always support CAN FD */
+	return true;
+}
+
 int open_candev(struct net_device *dev);
 void close_candev(struct net_device *dev);
 int can_change_mtu(struct net_device *dev, int new_mtu);
 int can_eth_ioctl_hwts(struct net_device *netdev, struct ifreq *ifr, int cmd);
 int can_ethtool_op_get_ts_info_hwts(struct net_device *dev,
diff --git a/net/can/raw.c b/net/can/raw.c
index e5e3952b0e09..5ff4ed629499 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -879,24 +879,30 @@ static void raw_put_canxl_vcid(struct raw_sock *ro, struct sk_buff *skb)
 		cxl->prio &= CANXL_PRIO_MASK;
 		cxl->prio |= ro->tx_vcid_shifted;
 	}
 }
 
-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;
+	}
 
 	/* 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;
 
 	return 0;
 }
 
@@ -948,11 +954,11 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		goto free_skb;
 
 	err = -EINVAL;
 
 	/* check for valid CAN (CC/FD/XL) frame content */
-	txmtu = raw_check_txframe(ro, skb, dev->mtu);
+	txmtu = raw_check_txframe(ro, skb, dev);
 	if (!txmtu)
 		goto free_skb;
 
 	/* only CANXL: clear/forward/set VCID value */
 	if (txmtu == CANXL_MTU)
-- 
2.47.3

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

[Oliver Hartkopp]

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.

Currently my patches are based on Linus' rc5 tree where CAN_CTRLMODE_XL 
is not defined. But I can rebase it on your b4/canxl-netlink branch if 
you like it.

Best regards,
Oliver

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

[Vincent Mailhol]

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 priv->ctrlmode & CAN_CTRLMODE_XL;
  }

(the above is not yet tested, so beware of silly errors!)

> Currently my patches are based on Linus' rc5 tree where CAN_CTRLMODE_XL is not
> defined. But I can rebase it on your b4/canxl-netlink branch if you like it.

I still want to finish the XL preparation series before moving to the actual thing.

You can do what is convinient for you:

  1/ rebase on b4/canxl-netlink branch but I take no claims if your code breaks
     after one of my force push.

  2/ add the flags you need in netlink.h. I will then chery pick your patches in
     my series when ready and take care of any conflict for you!

I am going to send the v2 of my XL preparation series. Hopefully we can finalyse
it soon so that we can focus on one sigle topic :)


Yours sincerely,
Vincent Mailhol

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

[Oliver Hartkopp]

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.

> 
>    	return priv->ctrlmode & CAN_CTRLMODE_XL;
>    }
> 
> (the above is not yet tested, so beware of silly errors!)
> 
>> Currently my patches are based on Linus' rc5 tree where CAN_CTRLMODE_XL is not
>> defined. But I can rebase it on your b4/canxl-netlink branch if you like it.
> 
> I still want to finish the XL preparation series before moving to the actual thing.
> 
> You can do what is convinient for you:
> 
>    1/ rebase on b4/canxl-netlink branch but I take no claims if your code breaks
>       after one of my force push.

I will do this with my next attempt.

>    2/ add the flags you need in netlink.h. I will then chery pick your patches in
>       my series when ready and take care of any conflict for you!
> 
> I am going to send the v2 of my XL preparation series. Hopefully we can finalyse
> it soon so that we can focus on one sigle topic :)

ACK.

Best regards,
Oliver

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

[Vincent Mailhol]

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

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

[Oliver Hartkopp]

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.

Best regards,
Oliver

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

[Vincent Mailhol]

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? 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

I will have to think of what are the implication for CAN XL.


Yours sincerely,
Vincent Mailhol

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

[Oliver Hartkopp]

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.

Best regards,
Oliver

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

[Vincent Mailhol]

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

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

[Vincent Mailhol]

+cc: Stéphane

On 15/09/2025 at 19:55, Vincent Mailhol wrote:
> 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.

I am looking at the code of can_change_mtu() but I can not understand the intent.

Back then, commit bc05a8944a34 ("can: allow to change the device mtu for CAN FD
capable devices") stated that:

  The configuration can be done either with the 'fd { on | off }' option in the
  'ip' tool from iproute2 or by setting the CAN netdevice MTU to CAN_MTU (16) or
  to CANFD_MTU (72).

  Link: https://git.kernel.org/torvalds/c/bc05a8944a34

But if I do something like:

  ip link set can0 mtu 72 type can bitrate 500000

the command is accepted and I am then left with a device which is in an
incoherent status (FD on, but no databittiming ?!)

I tested this on a device and it is just throwing me errors.

The same goes on when setting the mtu back to 16:

  ip link set can0 type can bitrate 500000 fd on dbitrate 5000000
  ip link set can0 mtu 16

and now I have a device in Classical CAN mode but iproute2 is still reporting me
the databittiming values (although this time, the device does not send me errors).

So, whatever I try, can_change_mtu() put the device in some incoherent state
with some more or less serious impact (either device errors or bad reporting).
And the only method to remediate is to use the 'fd { on | off }' option.

Am I missing something? If the feature is just broken, then I would like to
rewrite it but this time using the net_dev infrastructure by populating
net_device->min_mtu and net_device->max_mtu. IMHO, this should give us a way
more robust interface.


Yours sincerely,
Vincent Mailhol

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

[Oliver Hartkopp]

Hi Vincent,

On 15.09.25 15:59, Vincent Mailhol wrote:
> +cc: Stéphane
> 
> On 15/09/2025 at 19:55, Vincent Mailhol wrote:
>> 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.
> 
> I am looking at the code of can_change_mtu() but I can not understand the intent.
> 
> Back then, commit bc05a8944a34 ("can: allow to change the device mtu for CAN FD
> capable devices") stated that:
> 
>    The configuration can be done either with the 'fd { on | off }' option in the
>    'ip' tool from iproute2 or by setting the CAN netdevice MTU to CAN_MTU (16) or
>    to CANFD_MTU (72).
> 
>    Link: https://git.kernel.org/torvalds/c/bc05a8944a34
> 
> But if I do something like:
> 
>    ip link set can0 mtu 72 type can bitrate 500000
> 
> the command is accepted and I am then left with a device which is in an
> incoherent status (FD on, but no databittiming ?!)
> 
> I tested this on a device and it is just throwing me errors.
> 
> The same goes on when setting the mtu back to 16:
> 
>    ip link set can0 type can bitrate 500000 fd on dbitrate 5000000
>    ip link set can0 mtu 16
> 
> and now I have a device in Classical CAN mode but iproute2 is still reporting me
> the databittiming values (although this time, the device does not send me errors).
> 
> So, whatever I try, can_change_mtu() put the device in some incoherent state
> with some more or less serious impact (either device errors or bad reporting).
> And the only method to remediate is to use the 'fd { on | off }' option.
> 
> Am I missing something? If the feature is just broken, then I would like to
> rewrite it but this time using the net_dev infrastructure by populating
> net_device->min_mtu and net_device->max_mtu. IMHO, this should give us a way
> more robust interface.

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.

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).

2. When a virtual interface is set to "up" the MTU is used and fixed.

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 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.

Best regards,
Oliver

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

[Vincent Mailhol]

On 16/09/2025 at 03:08, Oliver Hartkopp wrote:
> Hi Vincent,
> 
> On 15.09.25 15:59, Vincent Mailhol wrote:
>> +cc: Stéphane
>>
>> On 15/09/2025 at 19:55, Vincent Mailhol wrote:
>>> 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.
>>
>> I am looking at the code of can_change_mtu() but I can not understand the intent.
>>
>> Back then, commit bc05a8944a34 ("can: allow to change the device mtu for CAN FD
>> capable devices") stated that:
>>
>>    The configuration can be done either with the 'fd { on | off }' option in the
>>    'ip' tool from iproute2 or by setting the CAN netdevice MTU to CAN_MTU (16) or
>>    to CANFD_MTU (72).
>>
>>    Link: https://git.kernel.org/torvalds/c/bc05a8944a34
>>
>> But if I do something like:
>>
>>    ip link set can0 mtu 72 type can bitrate 500000
>>
>> the command is accepted and I am then left with a device which is in an
>> incoherent status (FD on, but no databittiming ?!)
>>
>> I tested this on a device and it is just throwing me errors.
>>
>> The same goes on when setting the mtu back to 16:
>>
>>    ip link set can0 type can bitrate 500000 fd on dbitrate 5000000
>>    ip link set can0 mtu 16
>>
>> and now I have a device in Classical CAN mode but iproute2 is still reporting me
>> the databittiming values (although this time, the device does not send me
>> errors).
>>
>> So, whatever I try, can_change_mtu() put the device in some incoherent state
>> with some more or less serious impact (either device errors or bad reporting).
>> And the only method to remediate is to use the 'fd { on | off }' option.
>>
>> Am I missing something? If the feature is just broken, then I would like to
>> rewrite it but this time using the net_dev infrastructure by populating
>> net_device->min_mtu and net_device->max_mtu. IMHO, this should give us a way
>> more robust interface.
> 
> 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.

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.

> 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.

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.
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.

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. 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).


Yours sincerely,
Vincent Mailhol

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

[Oliver Hartkopp]

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

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

[Vincent Mailhol]

On 16/09/2025 at 18:14, Oliver Hartkopp wrote:
> 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.

I agree that we should not switch between MTUs of different protocols while the
interface is up.

My point is just to allow switching the MTU to anything between CANXL_MIN_MTU
and CANXL_MAX_MTU on a CAN XL interface which is up. So the use case is also
just one protocol type (CAN XL). And that, I think, is comparable to Ethernet
frames.

>>> 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.

vcan's MTU is set to CANFD_MTU. You still need to manually increase it to enable
xl. But fair enough, we could modify the vcan default to CANXL_MTU so that it
works as you just described.

But I do not see the parallel. On vcan, FD is turned on by default. On real
interfaces, CAN_CTRLMODE_FD is turned off by default. Why should we be trying to
spot similarities on something which is different in the first place?

What I dislike the most is that there is an existing logic for the real
interfaces. The MTU is set by default to CAN_MTU here:

  https://elixir.bootlin.com/linux/v6.16/source/drivers/net/can/dev/dev.c#L242

and is modified only if the device is CAN FD static here:

  https://elixir.bootlin.com/linux/v6.16/source/include/linux/can/dev.h#L144

Here, we are not adding a new feature, but extending an existing one. So you
would need a strong argument to justify a change going in the opposite direction
of the current logic.

I see that can_change_mtu() is currently broken, so that's a strong enough
reason to change it. But the default MTU logic looks coherent to me so I do not
get why that should change.

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

The user may look at it. It is better not to make assumptions here.

The design must be such that the netlink always reports a coherent configuration
at any point in time.

>> 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.

Then, why do we need to set an MTU in the first place? The user must call the
netlink interface at least once.

If I follow your point, we might as well set it to zero at the beginning to just
signal that the interface is not ready and that the MTU is unknown.

Also, consider the following. If I connect a real device and do right away a:

  ip link set can0 type can xl off

then, what is supposed to happen?

This is supposed to be NOP (the CAN_CTRLMODE_XL is off at the beginning).

But then, should the MTU change? If yes, should it change to the CANFD_MTU or to
the CAN_MTU?

That's why I am saying that having a disconnection between the MTU and the
control modes is bad. Your logic creates some edge cases for no valid reason.

The simple logic would be this:

		fd off, xl off		fd on, xl off		fd any, xl on
  ---------------------------------------------------------------------------
  default mtu	CAN_MTU			CANFD_MTU		CANXL_MTU
  min mtu	CAN_MTU			CANFD_MTU		CANXL_MIN_MTU
  max mtu	CAN_MTU			CANFD_MTU		CANXL_MAX_MTU

Each time the user touches the fd and xl flags, the MTU, MTU MIN and MTU_MAX
triplet is modified accordingly in can_changelink() to the values in above table
and this way, you are always in a coherent state in which the MTU matches the
control mode flags.

>> 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 ;-)

There is one feature that I would really like to use:

  net_device->min_mtu

and

  net_device->max_mtu

Once you set those, you automatically get:

  - validation by the core infrastructure that the user's inputs are in range
  - reporting of those min and max MTU values through the netlink interface

The first point is a nice to have. This way, the devices are not forced anymore
to populate net_device_ops->ndo_change_mtu.

The second point is what looks the most important to me. I am incapable of
remembering what are the actual values of CANXL_MIN_MTU and CANXL_MAX_MTU. If we
advertise those through netlink, then I can easily confirm the range.


Yours sincerely,
Vincent Mailhol

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

[Oliver Hartkopp]

On 16.09.25 15:17, Vincent Mailhol wrote:
> On 16/09/2025 at 18:14, Oliver Hartkopp wrote:
>> 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.
> 
> I agree that we should not switch between MTUs of different protocols while the
> interface is up.
> 
> My point is just to allow switching the MTU to anything between CANXL_MIN_MTU
> and CANXL_MAX_MTU on a CAN XL interface which is up. So the use case is also
> just one protocol type (CAN XL). And that, I think, is comparable to Ethernet
> frames.

Ok. I did not know that it is possible to change the ethernet MTU on the 
fly when the interface is up. But this would indeed match the CAN XL 
MIN/MAX MTU pattern then. I'm fine with it.

>>>> 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.
> 
> vcan's MTU is set to CANFD_MTU. You still need to manually increase it to enable
> xl. But fair enough, we could modify the vcan default to CANXL_MTU so that it
> works as you just described.

Yes. I changed to initial value from CC to FD here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=97edec3a11cf6f73f2e45c3035b5ff8e4c3543dd

Therefore it makes sense to do this for CAN XL again.

> But I do not see the parallel. On vcan, FD is turned on by default. On real
> interfaces, CAN_CTRLMODE_FD is turned off by default. Why should we be trying to
> spot similarities on something which is different in the first place?

We should generally separate the configuration of interfaces with 
netlink ctrlmode support (real CAN interfaces) from virtual CAN 
interfaces that don't know about  CAN_CTRLMODE_FD. See my personal 
summary below.

> What I dislike the most is that there is an existing logic for the real
> interfaces. The MTU is set by default to CAN_MTU here:
> 
>    https://elixir.bootlin.com/linux/v6.16/source/drivers/net/can/dev/dev.c#L242
> 
> and is modified only if the device is CAN FD static here:
> 
>    https://elixir.bootlin.com/linux/v6.16/source/include/linux/can/dev.h#L144
> 
> Here, we are not adding a new feature, but extending an existing one. So you
> would need a strong argument to justify a change going in the opposite direction
> of the current logic.

Ok. You convinced me. Please follow the current approach.
Every CAN interface starts with CAN_MTU unless they are static FD/XL 
interfaces.

> I see that can_change_mtu() is currently broken, so that's a strong enough
> reason to change it. But the default MTU logic looks coherent to me so I do not
> get why that should change.
> 
>> I assume the initial MTU isn't looked at by the users anyway.
> 
> The user may look at it. It is better not to make assumptions here.
> 
> The design must be such that the netlink always reports a coherent configuration
> at any point in time.
> 
>>> 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.
> 
> Then, why do we need to set an MTU in the first place? The user must call the
> netlink interface at least once.
> 
> If I follow your point, we might as well set it to zero at the beginning to just
> signal that the interface is not ready and that the MTU is unknown.
> 
> Also, consider the following. If I connect a real device and do right away a:
> 
>    ip link set can0 type can xl off
> 
> then, what is supposed to happen?

Nothing. There is no bitrate configured.

> This is supposed to be NOP (the CAN_CTRLMODE_XL is off at the beginning).
> 
> But then, should the MTU change? If yes, should it change to the CANFD_MTU or to
> the CAN_MTU?
> 
> That's why I am saying that having a disconnection between the MTU and the
> control modes is bad. Your logic creates some edge cases for no valid reason.
> 
> The simple logic would be this:
> 
> 		fd off, xl off		fd on, xl off		fd any, xl on
>    ---------------------------------------------------------------------------
>    default mtu	CAN_MTU			CANFD_MTU		CANXL_MTU
>    min mtu	CAN_MTU			CANFD_MTU		CANXL_MIN_MTU
>    max mtu	CAN_MTU			CANFD_MTU		CANXL_MAX_MTU
> 
> Each time the user touches the fd and xl flags, the MTU, MTU MIN and MTU_MAX
> triplet is modified accordingly in can_changelink() to the values in above table
> and this way, you are always in a coherent state in which the MTU matches the
> control mode flags.
> 

Yes, that's fine.

Will the user defined MTU for CAN XL survive the settings when xl is set 
to off and then set to on again?

>>> 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 ;-)
> 
> There is one feature that I would really like to use:
> 
>    net_device->min_mtu
> 
> and
> 
>    net_device->max_mtu
> 
> Once you set those, you automatically get:
> 
>    - validation by the core infrastructure that the user's inputs are in range
>    - reporting of those min and max MTU values through the netlink interface
> 
> The first point is a nice to have. This way, the devices are not forced anymore
> to populate net_device_ops->ndo_change_mtu.
> 
> The second point is what looks the most important to me. I am incapable of
> remembering what are the actual values of CANXL_MIN_MTU and CANXL_MAX_MTU. If we
> advertise those through netlink, then I can easily confirm the range.

Ok. Nice.

To sum it up for myself:

1. Setting the MTU from user space is only relevant for virtual CAN 
interfaces and CAN XL interfaces for values between CANXL_MIN_MTU and 
CANXL_MAX_MTU.

2. Usually the MTU is set automatically by the netlink configuration 
process when fd/xl on/off are set.

Right?

Best regards,
Oliver

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

[Vincent Mailhol]

On 18/09/2025 at 06:29, Oliver Hartkopp wrote:
> On 16.09.25 15:17, Vincent Mailhol wrote:
>> On 16/09/2025 at 18:14, Oliver Hartkopp wrote:
>>> 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.
>>
>> I agree that we should not switch between MTUs of different protocols while the
>> interface is up.
>>
>> My point is just to allow switching the MTU to anything between CANXL_MIN_MTU
>> and CANXL_MAX_MTU on a CAN XL interface which is up. So the use case is also
>> just one protocol type (CAN XL). And that, I think, is comparable to Ethernet
>> frames.
> 
> Ok. I did not know that it is possible to change the ethernet MTU on the fly
> when the interface is up. But this would indeed match the CAN XL MIN/MAX MTU
> pattern then. I'm fine with it.

Ack. Note that I am still testing here, so I reserve the right to back out on
this idea if I discover any conflict.

>>>>> 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.
>>
>> vcan's MTU is set to CANFD_MTU. You still need to manually increase it to enable
>> xl. But fair enough, we could modify the vcan default to CANXL_MTU so that it
>> works as you just described.
> 
> Yes. I changed to initial value from CC to FD here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?
> id=97edec3a11cf6f73f2e45c3035b5ff8e4c3543dd
> 
> Therefore it makes sense to do this for CAN XL again.

Agreed. I will add one patch to change this as well (otherwise, I will not touch
the vcan logic, as discussed).

>> But I do not see the parallel. On vcan, FD is turned on by default. On real
>> interfaces, CAN_CTRLMODE_FD is turned off by default. Why should we be trying to
>> spot similarities on something which is different in the first place?
> 
> We should generally separate the configuration of interfaces with netlink
> ctrlmode support (real CAN interfaces) from virtual CAN interfaces that don't
> know about  CAN_CTRLMODE_FD. See my personal summary below.
> 
>> What I dislike the most is that there is an existing logic for the real
>> interfaces. The MTU is set by default to CAN_MTU here:
>>
>>    https://elixir.bootlin.com/linux/v6.16/source/drivers/net/can/dev/dev.c#L242
>>
>> and is modified only if the device is CAN FD static here:
>>
>>    https://elixir.bootlin.com/linux/v6.16/source/include/linux/can/dev.h#L144
>>
>> Here, we are not adding a new feature, but extending an existing one. So you
>> would need a strong argument to justify a change going in the opposite direction
>> of the current logic.
> 
> Ok. You convinced me. Please follow the current approach.
> Every CAN interface starts with CAN_MTU unless they are static FD/XL interfaces.

Thanks!

>> I see that can_change_mtu() is currently broken, so that's a strong enough
>> reason to change it. But the default MTU logic looks coherent to me so I do not
>> get why that should change.
>>
>>> I assume the initial MTU isn't looked at by the users anyway.
>>
>> The user may look at it. It is better not to make assumptions here.
>>
>> The design must be such that the netlink always reports a coherent configuration
>> at any point in time.
>>
>>>> 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.
>>
>> Then, why do we need to set an MTU in the first place? The user must call the
>> netlink interface at least once.
>>
>> If I follow your point, we might as well set it to zero at the beginning to just
>> signal that the interface is not ready and that the MTU is unknown.
>>
>> Also, consider the following. If I connect a real device and do right away a:
>>
>>    ip link set can0 type can xl off
>>
>> then, what is supposed to happen?
> 
> Nothing. There is no bitrate configured.
> 
>> This is supposed to be NOP (the CAN_CTRLMODE_XL is off at the beginning).
>>
>> But then, should the MTU change? If yes, should it change to the CANFD_MTU or to
>> the CAN_MTU?
>>
>> That's why I am saying that having a disconnection between the MTU and the
>> control modes is bad. Your logic creates some edge cases for no valid reason.
>>
>> The simple logic would be this:
>>
>>         fd off, xl off        fd on, xl off        fd any, xl on
>>    ---------------------------------------------------------------------------
>>    default mtu    CAN_MTU            CANFD_MTU        CANXL_MTU
>>    min mtu    CAN_MTU            CANFD_MTU        CANXL_MIN_MTU
>>    max mtu    CAN_MTU            CANFD_MTU        CANXL_MAX_MTU
>>
>> Each time the user touches the fd and xl flags, the MTU, MTU MIN and MTU_MAX
>> triplet is modified accordingly in can_changelink() to the values in above table
>> and this way, you are always in a coherent state in which the MTU matches the
>> control mode flags.
>>
> 
> Yes, that's fine.
> 
> Will the user defined MTU for CAN XL survive the settings when xl is set to off
> and then set to on again?

Unfortunately no. Or at least not without adding one additional field to save
the old value.

But after turning FD or XL off, none on the bittiming parameters would survive
either. So I think it is coherent to say that the user has to set everything
again each time XL is switched on.

>>>> 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 ;-)
>>
>> There is one feature that I would really like to use:
>>
>>    net_device->min_mtu
>>
>> and
>>
>>    net_device->max_mtu
>>
>> Once you set those, you automatically get:
>>
>>    - validation by the core infrastructure that the user's inputs are in range
>>    - reporting of those min and max MTU values through the netlink interface
>>
>> The first point is a nice to have. This way, the devices are not forced anymore
>> to populate net_device_ops->ndo_change_mtu.
>>
>> The second point is what looks the most important to me. I am incapable of
>> remembering what are the actual values of CANXL_MIN_MTU and CANXL_MAX_MTU. If we
>> advertise those through netlink, then I can easily confirm the range.
> 
> Ok. Nice.
> 
> To sum it up for myself:
> 
> 1. Setting the MTU from user space is only relevant for virtual CAN interfaces
> and CAN XL interfaces for values between CANXL_MIN_MTU and CANXL_MAX_MTU.

Ack.

> 2. Usually the MTU is set automatically by the netlink configuration process
> when fd/xl on/off are set.

Ack.

As you will see, I found some bug because a few drivers forgot to set their
can_change_mtu() and addressed the issue here:

https://lore.kernel.org/linux-can/20250918-can-fix-mtu-v1-0-0d1cada9393b@kernel.org/

That series is just to fix things and is meant to be back ported to stable.

I will send a couple more patches as an RFC which will implement the actual
logic which we discussed here.


Yours sincerely,
Vincent Mailhol

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

[Oliver Hartkopp]

On 18.09.25 11:18, Vincent Mailhol wrote:
> On 18/09/2025 at 06:29, Oliver Hartkopp wrote:


>> Will the user defined MTU for CAN XL survive the settings when xl is set to off
>> and then set to on again?
> 
> Unfortunately no. Or at least not without adding one additional field to save
> the old value.
> 
> But after turning FD or XL off, none on the bittiming parameters would survive
> either. So I think it is coherent to say that the user has to set everything
> again each time XL is switched on.
> 

Ok. IMO setting the CAN XL MTU lower than CANXL_MTU_MAX is a power user 
feature anyway. So setting the bitrates and the MTU in one sequence 
should be no problem.


>> To sum it up for myself:
>>
>> 1. Setting the MTU from user space is only relevant for virtual CAN interfaces
>> and CAN XL interfaces for values between CANXL_MIN_MTU and CANXL_MAX_MTU.
> 
> Ack.
> 
>> 2. Usually the MTU is set automatically by the netlink configuration process
>> when fd/xl on/off are set.
> 
> Ack.
> 
> As you will see, I found some bug because a few drivers forgot to set their
> can_change_mtu() and addressed the issue here:
> 
> https://lore.kernel.org/linux-can/20250918-can-fix-mtu-v1-0-0d1cada9393b@kernel.org/
> 
> That series is just to fix things and is meant to be back ported to stable.

Nice cleanup!

> I will send a couple more patches as an RFC which will implement the actual
> logic which we discussed here.

Fine!

I assume we will miss this merge window for the CAN XL support then.
With all the things that need to be looked at carefully the next merge 
window is probably the better choice.

Best regards,
Oliver

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

[Vincent Mailhol]

On 21/09/2025 at 02:38, Oliver Hartkopp wrote:
> On 18.09.25 11:18, Vincent Mailhol wrote:
>> On 18/09/2025 at 06:29, Oliver Hartkopp wrote:
> 
> 
>>> Will the user defined MTU for CAN XL survive the settings when xl is set to off
>>> and then set to on again?
>>
>> Unfortunately no. Or at least not without adding one additional field to save
>> the old value.
>>
>> But after turning FD or XL off, none on the bittiming parameters would survive
>> either. So I think it is coherent to say that the user has to set everything
>> again each time XL is switched on.
>>
> 
> Ok. IMO setting the CAN XL MTU lower than CANXL_MTU_MAX is a power user feature
> anyway. So setting the bitrates and the MTU in one sequence should be no problem.
> 
> 
>>> To sum it up for myself:
>>>
>>> 1. Setting the MTU from user space is only relevant for virtual CAN interfaces
>>> and CAN XL interfaces for values between CANXL_MIN_MTU and CANXL_MAX_MTU.
>>
>> Ack.
>>
>>> 2. Usually the MTU is set automatically by the netlink configuration process
>>> when fd/xl on/off are set.
>>
>> Ack.
>>
>> As you will see, I found some bug because a few drivers forgot to set their
>> can_change_mtu() and addressed the issue here:
>>
>> https://lore.kernel.org/linux-can/20250918-can-fix-mtu-
>> v1-0-0d1cada9393b@kernel.org/
>>
>> That series is just to fix things and is meant to be back ported to stable.
> 
> Nice cleanup!
> 
>> I will send a couple more patches as an RFC which will implement the actual
>> logic which we discussed here.
> 
> Fine!
> 
> I assume we will miss this merge window for the CAN XL support then.

Yes, but I hope that to have the preparation series merged. Marc already picked
the MTU fix series:

https://lore.kernel.org/linux-can/20250918-can-fix-mtu-v1-0-0d1cada9393b@kernel.org/

Next week, I would like to merge the MTU rework:

https://lore.kernel.org/linux-can/20250918-can-fix-mtu-v1-0-471edb942295@kernel.org/

as well as the XL preparation:

https://lore.kernel.org/linux-can/20250910-canxl-netlink-prep-v2-0-f128d4083721@kernel.org/

Both are ready locally, but I first need to wait for the MTU fix to appear in
net-next before re-sending.

> With all the things that need to be looked at carefully the next merge window is
> probably the better choice.

By the way, my latest test are all green.

I updated my WIP

https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/log/?h=b4/canxl-netlink

I may still refactor a few things and I still need to write the patch comments,
but feature wise, I now implemented all what I had in mind.

But for next week, let's focus on the preparation series first, the discussion
on the actual XL code will be for the next merge window ;)


Yours sincerely,
Vincent Mailhol