can: propagate CAN device capabilities via ml_priv
Subsystems (2)
Oliver Hartkopp
maintainerPATCH v2 1/1 can: propagate CAN device capabilities via ml_priv
Commit 1a620a723853 ("can: raw: instantly reject unsupported CAN frames") caused a sequence of dependency and linker fixes starting with commit cb2dc6d2869a ("can: Kconfig: select CAN driver infrastructure by default").
The entire problem was caused by the requirement that a new network layer feature needed to know about the protocol capabilities of the CAN devices. Instead of accessing CAN device internal data structures which caused the dependency problems this patch introduces capabilty information into the CAN specific ml_priv data which is accessible from both sides.
With this change the CAN network layer can check the required features and the decoupling of the driver layer and network layer is restored. Therefore the Kconfig and Makefile changes/fixes are reverted too.
--- drivers/net/can/Kconfig | 7 ++++-- drivers/net/can/Makefile | 2 +- drivers/net/can/dev/Makefile | 5 ++-- drivers/net/can/dev/dev.c | 19 +++++++++++++++ drivers/net/can/dev/netlink.c | 1 + drivers/net/can/vcan.c | 15 ++++++++++++ drivers/net/can/vxcan.c | 15 ++++++++++++ include/linux/can/can-ml.h | 24 +++++++++++++++++++ include/linux/can/dev.h | 8 +------ net/can/raw.c | 45 ++++------------------------------- 10 files changed, 88 insertions(+), 53 deletions(-)
drivers/net/can/Kconfig +5 -2
@@ -1,9 +1,9 @@ # SPDX-License-Identifier: GPL-2.0-only menuconfig CAN_DEV - bool "CAN Device Drivers" + tristate "CAN Device Drivers" default y depends on CAN help Controller Area Network (CAN) is serial communications protocol up to 1Mbit/s for its original release (now known as Classical CAN) and up
@@ -15,11 +15,14 @@ menuconfig CAN_DEV This section contains all the CAN(-FD) device drivers including the virtual ones. If you own such devices or plan to use the virtual CAN interfaces to develop applications, say Y here. -if CAN_DEV && CAN + To compile as a module, choose M here: the module will be called + can-dev. + +if CAN_DEV config CAN_VCAN tristate "Virtual Local CAN Interface (vcan)" help Similar to the network loopback devices, vcan offers a
drivers/net/can/Makefile +1 -1
@@ -5,11 +5,11 @@ obj-$(CONFIG_CAN_VCAN) += vcan.o obj-$(CONFIG_CAN_VXCAN) += vxcan.o obj-$(CONFIG_CAN_SLCAN) += slcan/ -obj-$(CONFIG_CAN_DEV) += dev/ +obj-y += dev/ obj-y += esd/ obj-y += rcar/ obj-y += rockchip/ obj-y += spi/ obj-y += usb/
drivers/net/can/dev/Makefile +3 -2
@@ -1,10 +1,11 @@ # SPDX-License-Identifier: GPL-2.0 -obj-$(CONFIG_CAN) += can-dev.o +obj-$(CONFIG_CAN_DEV) += can-dev.o + +can-dev-y += skb.o -can-dev-$(CONFIG_CAN_DEV) += skb.o can-dev-$(CONFIG_CAN_CALC_BITTIMING) += calc_bittiming.o can-dev-$(CONFIG_CAN_NETLINK) += bittiming.o can-dev-$(CONFIG_CAN_NETLINK) += dev.o can-dev-$(CONFIG_CAN_NETLINK) += length.o can-dev-$(CONFIG_CAN_NETLINK) += netlink.o
drivers/net/can/dev/dev.c +19 -0
@@ -373,10 +373,28 @@ void can_set_default_mtu(struct net_device *dev) dev->min_mtu =CAN_MTU ; dev->max_mtu =CAN_MTU ; } } +void can_set_cap_info (struct net_device *dev) +{ +struct can_priv *priv =netdev_priv (dev); +u32 can_cap =CAN_CAP_CC ; + +if (can_dev_in_xl_only_mode (priv)) { +/* XL only mode => no CC/FD capability */ + can_cap =CAN_CAP_XL ; + }else { +if (priv->ctrlmode &CAN_CTRLMODE_FD ) + can_cap |=CAN_CAP_FD ; + +if (priv->ctrlmode &CAN_CTRLMODE_XL ) + can_cap |=CAN_CAP_XL ; + } +can_set_cap (dev, can_cap); +} +/* helper to define static CAN controller features at device creation time */ int can_set_static_ctrlmode (struct net_device *dev,u32 static_mode) {struct can_priv *priv =netdev_priv (dev);
@@ -388,10 +406,11 @@ int can_set_static_ctrlmode(struct net_device *dev, u32 static_mode) } priv->ctrlmode = static_mode;/* override MTU which was set by default in can_setup()? */ can_set_default_mtu (dev); +can_set_cap_info (dev);return 0 ; }EXPORT_SYMBOL_GPL (can_set_static_ctrlmode);
drivers/net/can/dev/netlink.c +1 -0
@@ -375,10 +375,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,memset (&priv->xl.tdc,0 ,sizeof (priv->xl.tdc));memset (&priv->xl.pwm,0 ,sizeof (priv->xl.pwm)); }can_set_default_mtu (dev); +can_set_cap_info (dev);return 0 ; } static int can_tdc_changelink(struct data_bittiming_params *dbt_params,
drivers/net/can/vcan.c +15 -0
@@ -128,10 +128,23 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)consume_skb (skb); }return NETDEV_TX_OK ; } +static void vcan_set_cap_info (struct net_device *dev) +{ +u32 can_cap =CAN_CAP_CC ; + +if (dev->mtu >CAN_MTU ) + can_cap |=CAN_CAP_FD ; + +if (dev->mtu >=CANXL_MIN_MTU ) + can_cap |=CAN_CAP_XL ; + +can_set_cap (dev, can_cap); +} +static int vcan_change_mtu (struct net_device *dev,int new_mtu) {/* Do not allow changing the MTU while running */ if (dev->flags &IFF_UP )return -EBUSY ;
@@ -139,10 +152,11 @@ static int vcan_change_mtu(struct net_device *dev, int new_mtu)if (new_mtu !=CAN_MTU && new_mtu !=CANFD_MTU && !can_is_canxl_dev_mtu (new_mtu))return -EINVAL ;WRITE_ONCE (dev->mtu, new_mtu); +vcan_set_cap_info (dev);return 0 ; }static const struct net_device_ops vcan_netdev_ops = { .ndo_start_xmit = vcan_tx,
@@ -160,10 +174,11 @@ static void vcan_setup(struct net_device *dev) dev->hard_header_len =0 ; dev->addr_len =0 ; dev->tx_queue_len =0 ; dev->flags =IFF_NOARP ;can_set_ml_priv (dev,netdev_priv (dev)); +vcan_set_cap_info (dev);/* set flags according to driver capabilities */ if (echo) dev->flags |=IFF_ECHO ;
drivers/net/can/vxcan.c +15 -0
@@ -123,10 +123,23 @@ static int vxcan_get_iflink(const struct net_device *dev)rcu_read_unlock ();return iflink; } +static void vxcan_set_cap_info (struct net_device *dev) +{ +u32 can_cap =CAN_CAP_CC ; + +if (dev->mtu >CAN_MTU ) + can_cap |=CAN_CAP_FD ; + +if (dev->mtu >=CANXL_MIN_MTU ) + can_cap |=CAN_CAP_XL ; + +can_set_cap (dev, can_cap); +} +static int vxcan_change_mtu (struct net_device *dev,int new_mtu) {/* Do not allow changing the MTU while running */ if (dev->flags &IFF_UP )return -EBUSY ;
@@ -134,10 +147,11 @@ static int vxcan_change_mtu(struct net_device *dev, int new_mtu)if (new_mtu !=CAN_MTU && new_mtu !=CANFD_MTU && !can_is_canxl_dev_mtu (new_mtu))return -EINVAL ;WRITE_ONCE (dev->mtu, new_mtu); +vxcan_set_cap_info (dev);return 0 ; }static const struct net_device_ops vxcan_netdev_ops = { .ndo_open = vxcan_open,
@@ -165,10 +179,11 @@ static void vxcan_setup(struct net_device *dev) dev->ethtool_ops = &vxcan_ethtool_ops; dev->needs_free_netdev = true; can_ml =netdev_priv (dev) +ALIGN (sizeof (struct vxcan_priv ),NETDEV_ALIGN );can_set_ml_priv (dev, can_ml); +vxcan_set_cap_info (dev); }/* forward declaration for rtnl_create_link() */ static struct rtnl_link_ops vxcan_link_ops;
include/linux/can/can-ml.h +24 -0
@@ -44,10 +44,15 @@#include <linux/can.h> #include <linux/list.h> #include <linux/netdevice.h> +/* exposed CAN device capabilities for network layer */ +#define CAN_CAP_CC 0x1U +#define CAN_CAP_FD 0x2U +#define CAN_CAP_XL 0x4U +#define CAN_SFF_RCV_ARRAY_SZ (1 << CAN_SFF_ID_BITS)#define CAN_EFF_RCV_HASH_BITS 10#define CAN_EFF_RCV_ARRAY_SZ (1 << CAN_EFF_RCV_HASH_BITS)enum {RX_ERR ,RX_ALL ,RX_FIL ,RX_INV ,RX_MAX };
@@ -62,10 +67,11 @@ struct can_dev_rcv_lists {struct can_ml_priv {struct can_dev_rcv_lists dev_rcv_lists;#ifdef CAN_J1939 struct j1939_priv *j1939_priv;#endif +u32 can_cap; };static inline struct can_ml_priv *can_get_ml_priv (struct net_device *dev) {return netdev_get_ml_priv (dev,ML_PRIV_CAN );
@@ -75,6 +81,24 @@ static inline void can_set_ml_priv(struct net_device *dev, struct can_ml_priv *ml_priv) {netdev_set_ml_priv (dev, ml_priv,ML_PRIV_CAN ); } +static inline bool can_cap_enabled (struct net_device *dev,u32 cap) +{ +struct can_ml_priv *can_ml =can_get_ml_priv (dev); + +if (!can_ml) +return true; + +return (can_ml->can_cap & cap); +} + +static inline void can_set_cap (struct net_device *dev,u32 cap) +{ +struct can_ml_priv *can_ml =can_get_ml_priv (dev); + +if (can_ml) + can_ml->can_cap = cap; +} +#endif /* CAN_ML_H */
include/linux/can/dev.h +1 -7
@@ -109,22 +109,16 @@ struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,#define alloc_candev_mq (sizeof_priv, echo_skb_max, count) \ alloc_candev_mqs(sizeof_priv, echo_skb_max, count, count)void free_candev (struct net_device *dev);/* a candev safe wrapper around netdev_priv */ -#if IS_ENABLED (CONFIG_CAN_NETLINK )struct can_priv *safe_candev_priv (struct net_device *dev); -#else -static inline struct can_priv *safe_candev_priv (struct net_device *dev) -{ -return NULL ; -} -#endif int open_candev (struct net_device *dev);void close_candev (struct net_device *dev);void can_set_default_mtu (struct net_device *dev); +void can_set_cap_info (struct net_device *dev);int __must_check can_set_static_ctrlmode (struct net_device *dev,u32 static_mode);int can_hwtstamp_get (struct net_device *netdev,struct kernel_hwtstamp_config *cfg); int can_hwtstamp_set(struct net_device *netdev,
net/can/raw.c +4 -41
77 lines not on this page · raw message
Signature · 1 line
2.47.3
Vincent Mailhol
maintainerHi Oliver,
Happy new year to you and to all the members of the CAN mailing list!(and also to Arnd who is listed in CC ;))
Quoted 17 lines · Oliver Hartkopp
Commit
1a620a723853("can: raw: instantly reject unsupported CAN frames") caused a sequence of dependency and linker fixes starting with commitcb2dc6d2869a("can:Kconfig: select CAN driver infrastructure by default").The entire problem was caused by the requirement that a new network layer feature needed to know about the protocol capabilities of the CAN devices. Instead of accessing CAN device internal data structures which caused the dependency problems this patch introduces capabilty information into the CAN specific
ml_privdata which is accessible from both sides.With this change the CAN network layer can check the required features and the decoupling of the driver layer and network layer is restored. Therefore the
KconfigandMakefilechanges/fixes are reverted too.Fixes 1a620a723853 ("can: raw: instantly reject unsupported CAN frames")Fixes cb2dc6d2869a ("can: Kconfig: select CAN driver infrastructure by default")Signed-off-by Oliver Hartkopp <[email protected]>
Quoted 11 lines
--- drivers/net/can/Kconfig | 7 ++++-- drivers/net/can/Makefile | 2 +- drivers/net/can/dev/Makefile | 5 ++-- drivers/net/can/dev/dev.c | 19 +++++++++++++++ drivers/net/can/dev/netlink.c | 1 + drivers/net/can/vcan.c | 15 ++++++++++++ drivers/net/can/vxcan.c | 15 ++++++++++++ include/linux/can/can-ml.h | 24 +++++++++++++++++++ include/linux/can/dev.h | 8 +------ net/can/raw.c | 45 ++++-------------------------------
This is a huge patch. I think it should be split in smaller pieces with the reverts and the new code in different patches. What I would suggest is to split it in 3 like this:
Patch #1: revert 1a620a723853 ("can: raw: instantly reject unsupported
CAN frames") and all the Kconfig subsequent fix attempts.
Patch #2: Introduce the new logic.
Patch #3: reintroduce 1a620a723853.
Reverting 1a620a723853 and reintroducing it later is kind of necessary to have each patch of the series in a working state.
Yours sincerely,
Vincent Mailhol
Oliver Hartkopp
maintainerHi Vincent,
Quoted 2 lines · Vincent Mailhol
Happy new year to you and to all the members of the CAN mailing list!(and also to Arnd who is listed in CC ;))
Thanks! Happy new year to you too!
This year CAN XL will lift off with a first native CAN XL netdevice driver and likely also with a new ISO-TP variant for CAN XL \o/
Looking forward to both of it!
(..)
Quoted 15 lines
Quoted 14 lines
Fixes 1a620a723853 ("can: raw: instantly reject unsupported CAN frames")Fixes cb2dc6d2869a ("can: Kconfig: select CAN driver infrastructure by default")Signed-off-by Oliver Hartkopp <[email protected]>--- drivers/net/can/Kconfig | 7 ++++-- drivers/net/can/Makefile | 2 +- drivers/net/can/dev/Makefile | 5 ++-- drivers/net/can/dev/dev.c | 19 +++++++++++++++ drivers/net/can/dev/netlink.c | 1 + drivers/net/can/vcan.c | 15 ++++++++++++ drivers/net/can/vxcan.c | 15 ++++++++++++ include/linux/can/can-ml.h | 24 +++++++++++++++++++ include/linux/can/dev.h | 8 +------ net/can/raw.c | 45 ++++-------------------------------
Quoted 1 line
This is a huge patch.
88 insertions(+), 53 deletions(-) => adding 35 lines. Not *that* huge ;-)
Quoted 10 lines
I think it should be split in smaller pieces with
the reverts and the new code in different patches. What I would suggest is to split it in 3 like this:Patch #1: revert
1a620a723853("can: raw: instantly reject unsupported
CAN frames") and all theKconfigsubsequent fix attempts.Patch #2: Introduce the new logic.
Patch #3: reintroduce 1a620a723853.
Right, that's at least easier to review.
Quoted 2 lines
Reverting
1a620a723853and reintroducing it later is kind of necessary to have each patch of the series in a working state.
Hopefully patch #2/#3 will still be accepted to restore the functionality of the 6.19-rc1 kernel then ...
I will send a set of three patches as suggested.
Best regards,
Oliver
Quoted 14 lines · Oliver Hartkopp
Commit
1a620a723853("can: raw: instantly reject unsupported CAN frames") caused a sequence of dependency and linker fixes starting with commitcb2dc6d2869a("can:Kconfig: select CAN driver infrastructure by default").The entire problem was caused by the requirement that a new network layer feature needed to know about the protocol capabilities of the CAN devices. Instead of accessing CAN device internal data structures which caused the dependency problems this patch introduces capabilty information into the CAN specific
ml_privdata which is accessible from both sides.With this change the CAN network layer can check the required features and the decoupling of the driver layer and network layer is restored. Therefore the
KconfigandMakefilechanges/fixes are reverted too.
Quoted 5 lines
Fixes 1a620a723853 ("can: raw: instantly reject unsupported CANframes")
Fixes cb2dc6d2869a ("can: Kconfig: select CAN driver infrastructure bydefault")
Signed-off-by Oliver Hartkopp <[email protected]>
No objections to the patch, but I don't understand from your description here if there is a remaining regression in the current upstream version after the 5a5aff6338c0 ("can: fix build dependency") fixup, or if this is meant as a cleanup that can wait until the next merge window.
Arnd
Oliver Hartkopp
maintainerHello Arnd,
Quoted 1 line · Arnd Bergmann
On Wed, Dec 31, 2025, at 17:51, Oliver Hartkopp wrote:
(..)
Quoted 10 lines
Quoted 9 lines
With this change the CAN network layer can check the required features and the decoupling of the driver layer and network layer is restored. Therefore the
KconfigandMakefilechanges/fixes are reverted too.Fixes 1a620a723853 ("can: raw: instantly reject unsupported CANframes")
Fixes cb2dc6d2869a ("can: Kconfig: select CAN driver infrastructure bydefault")
Signed-off-by Oliver Hartkopp <[email protected]>
Quoted 5 lines
No objections to the patch, but I don't understand from your description here if there is a remaining regression in the current upstream version after the
5a5aff6338c0("can: fix build dependency") fixup, or if this is meant as a cleanup that can wait until the next merge window.
To me it is a regression for code which was introduced in the 6.19 merge window, as we bundled new dependencies of the CAN network layer and the CAN driver layer that where not needed before.
E.g. you were able to build the network layer without configuring any CAN driver - and building the netlayer stuff as built-in code and having the CAN drivers/infrastructure as modules was also possible.
So the entire sequence of needed fixes for my can-raw patch generated the regression IMO.
Best regards,
Oliver