Linux CAN drivers development
 help / color / mirror / Atom feed
* [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
@ 2025-09-07  8:05 Vincent Mailhol
  2025-09-07 19:03 ` Oliver Hartkopp
  0 siblings, 1 reply; 9+ messages in thread
From: Vincent Mailhol @ 2025-09-07  8:05 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: linux-can, Vincent Mailhol

Currently, the CAN FD skb validation logic is based on the MTU: the
interface is deemed FD capable if and only if its MTU is greater or
equal to CANFD_MTU.

This logic is showing its limits since the introduction of CAN XL.
Indeed, an interface which is configured to support CAN XL but not CAN
FD would have its MTU set to CANXL_MIN_MTU which is greater than
CANFD_MTU, thus fulfilling the CAN FD check condition. That is to say,
any CAN XL interface currently appears as CAN FD capable with no way
of configuring or probing this at the skb level.

Because of the limitation, the only non-UAPI-breaking workaround is to
perform the check at the device level using the CAN_CTRLMODE_FD flag.
Unfortunately, the virtual interfaces (vcan, vxcan) are left behind
with this approach.

Add a check on the CAN_CTRLMODE_FD flag in can_dev_dropped_skb() and
drop FD frames whenever the feature is turned off.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
While testing, I encountered a potential issue when:

  - CAN FD is off
  - CAN XL is on

My understanding is that this is allowed and, even more, it is
sometimes mandatory when, for example, disabling error signalling or
when using the transceiver mode switch (TMS) or both.

It seems to me that the current implementation is incapable of
handling this case at the skb level for the reasons listed in the
patch body.

I have two ideas to fix this issue:

  1. above patch which assumes that we are OK not being able to set
     CAN XL on and FD off on virtual interfaces

  2. add some information to the skb itself. I have not looked at this
     in detail. One potential idea would be to use:

       sk_buff->protocol

     as a bitfield. Whenever the MTU is greater than CANFD_MTU, below
     logic would apply:

       - if sk_buff->protocol & CAN_SKB_CC is true, classical CAN is
         supported

       - if sk_buff->protocol & CAN_SKB_FD is true, CAN FD is
         supported

       - if sk_buff->protocol & CAN_SKB_XL is true, CAN XL is
         supported

     with any of the combinations above accepted.

     Of course, because this comes as an afterthought, it is an UAPI
     breaking change for any existing CAN XL code. Also,
     sk_buff->protocol was not designed for this, making this solution
     an ugly hack.

Thanks for your comments.
---
 include/linux/can/dev.h | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 9a92cbe5b2cb7..5053d5bc20c99 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -164,12 +164,18 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s
 	if (priv->ctrlmode & CAN_CTRLMODE_LISTENONLY) {
 		netdev_info_once(dev,
 				 "interface in listen only mode, dropping skb\n");
-		kfree_skb(skb);
-		dev->stats.tx_dropped++;
-		return true;
+		goto invalid_skb;
 	}
 
+	if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb))
+		goto invalid_skb;
+
 	return can_dropped_invalid_skb(dev, skb);
+
+invalid_skb:
+	kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return true;
 }
 
 void can_setup(struct net_device *dev);
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-09-08 17:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-07  8:05 [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Vincent Mailhol
2025-09-07 19:03 ` Oliver Hartkopp
2025-09-08  4:07   ` Vincent Mailhol
2025-09-08  9:00     ` Oliver Hartkopp
2025-09-08 13:30       ` Vincent Mailhol
2025-09-08 14:59         ` Oliver Hartkopp
2025-09-08 16:31           ` Vincent Mailhol
2025-09-08 16:55             ` Oliver Hartkopp
2025-09-08 17:42               ` Vincent Mailhol

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox