linux-kernel - Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2c75aca3-a19a-4144-8be5-8fb7524e581e@hartkopp.net>
Date: Thu, 6 Nov 2025 09:50:36 +0100
From: Oliver Hartkopp <socketcan@...tkopp.net>
To: Vincent Mailhol <mailhol@...nel.org>,
 Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Stéphane Grosjean <stephane.grosjean@...-networks.com>,
 Robert Nawrath <mbro1689@...il.com>, Minh Le <minh.le.aj@...esas.com>,
 Duy Nguyen <duy.nguyen.rh@...esas.com>, linux-can@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL



On 21.10.25 17:47, Vincent Mailhol wrote:
> Classical CAN and CAN FD must generate error frames on the CAN bus
> when detecting a protocol violation.
> 
> CAN XL's error signaling is different and works as follows:
> 
>    - In interoperability mode (both FD and XL), error signaling must be
>      on.
> 
>    - When operating a CAN controller in CAN XL only mode but with TMS
>      off, the user can decide whether the error signalling is enabled
>      or disabled.
> 
>    - On the contrary, when using TMS, error signalling must be off.
> 
> Introduce the new CAN_CTRLMODE_XL_ERR_SIGNAL control mode. This new
> option is only made available for CAN XL, so despite the error
> signalling being always on for Classical CAN and CAN FD, forbid the
> use of this flag when CAN XL is off.
> 
> If the user provides the error signalling flag, check its validity. If
> the flag is omitted, activate error signalling by default whenever
> possible. This is summarized in below table:
> 
> 			CAN_CTRLMODE_XL_ERR_SIGNAL
> 	-------------------------------------------
> 	CC/FD		option not available
> 	CC/FD/XL	on

Yes. This is the 'mixed-mode'
I would propose to use the 'mixed-mode' expression in the patch description.

> 	XL TMS off	configurable (default on)

Good default.

> 	XL TMS on	off
> 
> Suggested-by: Oliver Hartkopp <socketcan@...tkopp.net>
> Link: https://lore.kernel.org/linux-can/20250527195625.65252-9-socketcan@hartkopp.net/
> Signed-off-by: Vincent Mailhol <mailhol@...nel.org>
> ---
>   drivers/net/can/dev/dev.c        |  2 ++
>   drivers/net/can/dev/netlink.c    | 29 +++++++++++++++++++++++++++--
>   include/uapi/linux/can/netlink.h |  1 +
>   3 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 1de5babcc4f3..0c16d0174f7f 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -125,6 +125,8 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>   		return "xl-tdc-manual";
>   	case CAN_CTRLMODE_XL_TMS:
>   		return "xl-tms";
> +	case CAN_CTRLMODE_XL_ERR_SIGNAL:
> +		return "xl-error-signalling";
>   	default:
>   		return "<unknown>";
>   	}
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 8afd2baa03cf..6126b191fea0 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -191,7 +191,8 @@ static int can_validate_xl_flags(struct netlink_ext_ack *extack,
>   		}
>   		if (masked_flags & CAN_CTRLMODE_XL_TMS) {
>   			const u32 tms_conflicts_mask = CAN_CTRLMODE_FD |
> -				CAN_CTRLMODE_XL_TDC_MASK;
> +				CAN_CTRLMODE_XL_TDC_MASK |
> +				CAN_CTRLMODE_XL_ERR_SIGNAL;
>   			u32 tms_conflicts = masked_flags & tms_conflicts_mask;
>   
>   			if (tms_conflicts) {
> @@ -201,11 +202,23 @@ static int can_validate_xl_flags(struct netlink_ext_ack *extack,
>   				return -EOPNOTSUPP;
>   			}
>   		}
> +		if ((masked_flags & CAN_CTRLMODE_FD) &&
> +		    (mask & CAN_CTRLMODE_XL_ERR_SIGNAL) &&
> +		    !(masked_flags & CAN_CTRLMODE_XL_ERR_SIGNAL)) {
> +			NL_SET_ERR_MSG(extack,
> +				       "When using both CAN FD and XL, error signalling must be on");

This implicitly tells us that mixed-mode is CC/FD/XL ;-)
  > +			return -EOPNOTSUPP;
> +		}
>   	} else {
>   		if (mask & CAN_CTRLMODE_XL_TMS) {
>   			NL_SET_ERR_MSG(extack, "TMS requires CAN XL");
>   			return -EOPNOTSUPP;
>   		}
> +		if (mask & CAN_CTRLMODE_XL_ERR_SIGNAL) {
> +			NL_SET_ERR_MSG(extack,
> +				       "Error signalling is only configurable with CAN XL");
> +			return -EOPNOTSUPP;
> +		}
>   	}
>   
>   	return 0;
> @@ -310,6 +323,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   				       "TMS can not be activated while CAN FD is on");
>   			return -EOPNOTSUPP;
>   		}
> +		if (deactivated & CAN_CTRLMODE_XL_ERR_SIGNAL) {
> +			NL_SET_ERR_MSG(extack,
> +				       "Error signalling can not be deactivated while CAN FD is on");
> +			return -EOPNOTSUPP;
> +		}
>   	}
>   
>   	/* If a top dependency flag is provided, reset all its dependencies */
> @@ -317,12 +335,19 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   		priv->ctrlmode &= ~CAN_CTRLMODE_FD_TDC_MASK;
>   	if (cm->mask & CAN_CTRLMODE_XL)
>   		priv->ctrlmode &= ~(CAN_CTRLMODE_XL_TDC_MASK |
> -				    CAN_CTRLMODE_XL_TMS);
> +				    CAN_CTRLMODE_XL_TMS |
> +				    CAN_CTRLMODE_XL_ERR_SIGNAL);
>   
>   	/* clear bits to be modified and copy the flag values */
>   	priv->ctrlmode &= ~cm->mask;
>   	priv->ctrlmode |= maskedflags;
>   
> +	/* If omitted, set error signalling on if possible */
> +	if ((maskedflags & CAN_CTRLMODE_XL) &&
> +	    !(cm->mask & CAN_CTRLMODE_XL_ERR_SIGNAL) &&
> +	    !(priv->ctrlmode & CAN_CTRLMODE_XL_TMS))
> +		priv->ctrlmode |= CAN_CTRLMODE_XL_ERR_SIGNAL;
> +
>   	/* Wipe potential leftovers from previous CAN FD/XL config */
>   	if (!(priv->ctrlmode & CAN_CTRLMODE_FD)) {
>   		memset(&priv->fd.data_bittiming, 0,
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index ebafb091d80f..30d446921dc4 100644
> --- a/include/uapi/linux/can/netlink.h
> +++ b/include/uapi/linux/can/netlink.h
> @@ -108,6 +108,7 @@ struct can_ctrlmode {
>   #define CAN_CTRLMODE_XL_TDC_AUTO	0x2000	/* XL transceiver automatically calculates TDCV */
>   #define CAN_CTRLMODE_XL_TDC_MANUAL	0x4000	/* XL TDCV is manually set up by user */
>   #define CAN_CTRLMODE_XL_TMS		0x8000	/* Transceiver Mode Switching */
> +#define CAN_CTRLMODE_XL_ERR_SIGNAL	0x10000	/* XL error signalling */
>   
>   /*
>    * CAN device statistics
> 
Reviewed-by: Oliver Hartkopp <socketcan@...tkopp.net>
Best regards,
Oliver