| 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
| ||
|
Message-ID: <fc5e764d-3ef8-455e-9bae-bd50ea206ce2@kernel.org>
Date: Sun, 9 Nov 2025 15:54:48 +0100
From: Vincent Mailhol <mailhol@...nel.org>
To: Oliver Hartkopp <socketcan@...tkopp.net>,
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 06/11/2025 at 09:50, Oliver Hartkopp wrote:
> 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.
Ack!
>> 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@...tkopp.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");
I changed that error message to:
NL_SET_ERR_MSG(extack, "Mixed mode requires error signalling");
> This implicitly tells us that mixed-mode is CC/FD/XL ;-)
I was under the assumption that Classical CAN was always allowed, even under
TMS. The arbitration still uses the nominal bittiming anyway, so I still have
some issue understanding why an XL nodes operating under TMS wouldn't be able to
send a classical CAN frame.
The restriction seems rather arbitrary to me. I would be curious to understand
what the issue would be to allow Classical CAN under TMS.
> > + 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>
Thanks!
Yours sincerely,
Vincent Mailhol
Powered by blists - more mailing lists