Skip to contentSkip to conversation

canxl: add virtual CAN network identifier support

PATCH Superseded Oliver Hartkopp 2 participants 3 messages Jan 8, 2024 – Jan 28, 2024

Subsystems (2)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Oliver Hartkopp

author maintainer
#0 lore raw

PATCH v4 1/1 canxl: add virtual CAN network identifier support

CAN XL data frames contain an 8-bit virtual CAN network identifier (VCID). A VCID value of zero represents an 'untagged' CAN XL frame.

To receive and send these optional VCIDs via CAN_RAW sockets a new socket option CAN_RAW_XL_VCID_OPTS is introduced to define/access VCID content:

- tx: set the outgoing VCID value by the kernel (one fixed 8-bit value)
- tx: pass through VCID values from the user space (e.g. for traffic replay)
- rx: apply VCID receive filter (value/mask) to be passed to the user space

With the 'tx pass through' option CAN_RAW_XL_VCID_TX_PASS all valid VCID values can be sent, e.g. to replay full qualified CAN XL traffic.

The VCID value provided for the CAN_RAW_XL_VCID_TX_SET option will override the VCID value in the struct canxl_frame.vcid element defined for CAN_RAW_XL_VCID_TX_PASS when both flags are set.

With a rx_vcid_mask of zero all possible VCID values (0x00 - 0xFF) are passed to the user space when the CAN_RAW_XL_VCID_RX_FILTER flag is set. Without this flag only untagged CAN XL frames (VCID = 0x00) are delivered to the user space (default).

---
 include/uapi/linux/can.h     | 41 ++++++++++++++--------
 include/uapi/linux/can/raw.h | 14 ++++++++
 net/can/af_can.c             |  2 ++
 net/can/raw.c                | 66 ++++++++++++++++++++++++++++++++++--
 4 files changed, 106 insertions(+), 17 deletions(-)
include/uapi/linux/can.h +27 -14
index 939db2388208..8dcd8a9b5676 100644
@@ -44,10 +44,11 @@
  */
 
 #ifndef _UAPI_CAN_H
 #define _UAPI_CAN_H
 
+#include <asm/byteorder.h>
 #include <linux/types.h>
 #include <linux/socket.h>
 #include <linux/stddef.h> /* for offsetof */
 
 /* controller area network (CAN) kernel definitions */
@@ -193,26 +194,38 @@ struct canfd_frame {
 #define CANXL_XLF 0x80 /* mandatory CAN XL frame flag (must always be set!) */
 #define CANXL_SEC 0x01 /* Simple Extended Content (security/segmentation) */
 
 /**
  * struct canxl_frame - CAN with e'X'tended frame 'L'ength frame structure
- * @prio:  11 bit arbitration priority with zero'ed CAN_*_FLAG flags
- * @flags: additional flags for CAN XL
- * @sdt:   SDU (service data unit) type
- * @len:   frame payload length in byte (CANXL_MIN_DLEN .. CANXL_MAX_DLEN)
- * @af:    acceptance field
- * @data:  CAN XL frame payload (CANXL_MIN_DLEN .. CANXL_MAX_DLEN byte)
- *
- * @prio shares the same position as @can_id from struct can[fd]_frame.
+ * @prio:   11 bit arbitration priority (unused 5 bits must be set to zero)
+ *          Shares the lower 16 bits of @can_id from struct can[fd]_frame
+ * @vcid:   CAN XL virtual CAN network identifier
+ * @__res0: reserved / padding must be set to zero to maintain the CAN filter
+ *          functionality as is holds the @can_id flags CAN_xxx_FLAG
+ * @flags:  additional flags for CAN XL
+ * @sdt:    SDU (service data unit) type
+ * @len:    frame payload length in byte (CANXL_MIN_DLEN .. CANXL_MAX_DLEN)
+ * @af:     acceptance field
+ * @data:   CAN XL frame payload (CANXL_MIN_DLEN .. CANXL_MAX_DLEN byte)
  */
 struct canxl_frame {
-	canid_t prio;  /* 11 bit priority for arbitration (canid_t) */
-	__u8    flags; /* additional flags for CAN XL */
-	__u8    sdt;   /* SDU (service data unit) type */
-	__u16   len;   /* frame payload length in byte */
-	__u32   af;    /* acceptance field */
-	__u8    data[CANXL_MAX_DLEN];
+#if defined(__LITTLE_ENDIAN)
+	__u16 prio;   /* 11 bit priority for arbitration */
+	__u8  vcid;   /* virtual CAN network identifier */
+	__u8  __res0; /* reserved / padding must be set to zero */
+#elif defined(__BIG_ENDIAN)
+	__u8  __res0; /* reserved / padding must be set to zero */
+	__u8  vcid;   /* virtual CAN network identifier */
+	__u16 prio;   /* 11 bit priority for arbitration */
+#else
+#error "Unknown endianness"
+#endif
+	__u8  flags;  /* additional flags for CAN XL */
+	__u8  sdt;    /* SDU (service data unit) type */
+	__u16 len;    /* frame payload length in byte */
+	__u32 af;     /* acceptance field */
+	__u8  data[CANXL_MAX_DLEN];
 };
 
 #define CAN_MTU		(sizeof(struct can_frame))
 #define CANFD_MTU	(sizeof(struct canfd_frame))
 #define CANXL_MTU	(sizeof(struct canxl_frame))
include/uapi/linux/can/raw.h +14 -0
index 31622c9b7988..88e4e8176b89 100644
@@ -63,8 +63,22 @@ enum {
 	CAN_RAW_LOOPBACK,	/* local loopback (default:on)       */
 	CAN_RAW_RECV_OWN_MSGS,	/* receive my own msgs (default:off) */
 	CAN_RAW_FD_FRAMES,	/* allow CAN FD frames (default:off) */
 	CAN_RAW_JOIN_FILTERS,	/* all filters must match to trigger */
 	CAN_RAW_XL_FRAMES,	/* allow CAN XL frames (default:off) */
+	CAN_RAW_XL_VCID_OPTS,	/* CAN XL VCID configuration options */
 };
 
+/* configuration for CAN XL virtual CAN identifier (VCID) handling */
+struct can_raw_vcid_options {
+	__u8 flags;		/* flags for vcid (filter) behaviour */
+	__u8 tx_vcid;		/* VCID value set into canxl_frame.vcid */
+	__u8 rx_vcid;		/* VCID value for VCID filter */
+	__u8 rx_vcid_mask;	/* VCID mask for VCID filter */
+};
+
+/* can_raw_vcid_options.flags for CAN XL virtual CAN identifier handling */
+#define CAN_RAW_XL_VCID_TX_SET		0x01
+#define CAN_RAW_XL_VCID_TX_PASS		0x02
+#define CAN_RAW_XL_VCID_RX_FILTER	0x04
+
 #endif /* !_UAPI_CAN_RAW_H */
net/can/af_can.c +2 -0
index 7343fd487dbe..707576eeeb58 100644
@@ -863,10 +863,12 @@ static __init int can_init(void)
 	int err;
 
 	/* check for correct padding to be able to use the structs similarly */
 	BUILD_BUG_ON(offsetof(struct can_frame, len) !=
 		     offsetof(struct canfd_frame, len) ||
+		     offsetof(struct can_frame, len) !=
+		     offsetof(struct canxl_frame, flags) ||
 		     offsetof(struct can_frame, data) !=
 		     offsetof(struct canfd_frame, data));
 
 	pr_info("can: controller area network core\n");
 
net/can/raw.c +63 -3

119 lines not on this page · raw message

Signature · 1 line
2.39.2
Reviewed-by on v4 1/1
#1 lore raw

Hi Oliver,

Sorry for the delay, last weeks were busier than average.

Quoted 17 lines · Oliver Hartkopp

CAN XL data frames contain an 8-bit virtual CAN network identifier (VCID). A VCID value of zero represents an 'untagged' CAN XL frame.

To receive and send these optional VCIDs via CAN_RAW sockets a new socket option CAN_RAW_XL_VCID_OPTS is introduced to define/access VCID content:

- tx: set the outgoing VCID value by the kernel (one fixed 8-bit value)
- tx: pass through VCID values from the user space (e.g. for traffic replay)
- rx: apply VCID receive filter (value/mask) to be passed to the user space

With the 'tx pass through' option CAN_RAW_XL_VCID_TX_PASS all valid VCID values can be sent, e.g. to replay full qualified CAN XL traffic.

The VCID value provided for the CAN_RAW_XL_VCID_TX_SET option will override the VCID value in the struct canxl_frame.vcid element defined for CAN_RAW_XL_VCID_TX_PASS when both flags are set.

Quoted 6 lines

With a rx_vcid_mask of zero all possible VCID values (0x00 - 0xFF) are passed to the user space when the CAN_RAW_XL_VCID_RX_FILTER flag is set. Without this flag only untagged CAN XL frames (VCID = 0x00) are delivered to the user space (default).

I am not yet aware of all the details of CAN XL and its VCID, but to the extent of my knowledge, the patch looks good.

I left two nitpicks. Notwithstanding: you can add my review tag to v5.

Quoted 32 lines
---
 include/uapi/linux/can.h     | 41 ++++++++++++++--------
 include/uapi/linux/can/raw.h | 14 ++++++++
 net/can/af_can.c             |  2 ++
 net/can/raw.c                | 66 ++++++++++++++++++++++++++++++++++--
 4 files changed, 106 insertions(+), 17 deletions(-)
include/uapi/linux/can.h +1 -3
index 939db2388208..8dcd8a9b5676 100644
@@ -44,10 +44,11 @@
  */
 
 #ifndef _UAPI_CAN_H
 #define _UAPI_CAN_H
 
+#include <asm/byteorder.h>
 #include <linux/types.h>
 #include <linux/socket.h>
 #include <linux/stddef.h> /* for offsetof */
 
 /* controller area network (CAN) kernel definitions */
@@ -193,26 +194,38 @@ struct canfd_frame {
 #define CANXL_XLF 0x80 /* mandatory CAN XL frame flag (must always be set!) */
 #define CANXL_SEC 0x01 /* Simple Extended Content (security/segmentation) */
 
 /**
  * struct canxl_frame - CAN with e'X'tended frame 'L'ength frame structure
- * @prio:  11 bit arbitration priority with zero'ed CAN_*_FLAG flags
- * @flags: additional flags for CAN XL
- * @sdt:   SDU (service data unit) type

include/uapi/linux/can.h struct canfd_frame { · line 203

Quoted 10 lines
include/uapi/linux/can.h +5 -5
- * @len:   frame payload length in byte (CANXL_MIN_DLEN .. CANXL_MAX_DLEN)
- * @af:    acceptance field
- * @data:  CAN XL frame payload (CANXL_MIN_DLEN .. CANXL_MAX_DLEN byte)
- *
- * @prio shares the same position as @can_id from struct can[fd]_frame.
+ * @prio:   11 bit arbitration priority (unused 5 bits must be set to zero)
+ *          Shares the lower 16 bits of @can_id from struct can[fd]_frame
+ * @vcid:   CAN XL virtual CAN network identifier
+ * @__res0: reserved / padding must be set to zero to maintain the CAN filter
+ *          functionality as is holds the @can_id flags CAN_xxx_FLAG
                                ^^

as it holds

Quoted 188 lines
include/uapi/linux/can.h +21 -6
+ * @flags:  additional flags for CAN XL
+ * @sdt:    SDU (service data unit) type
+ * @len:    frame payload length in byte (CANXL_MIN_DLEN .. CANXL_MAX_DLEN)
+ * @af:     acceptance field
+ * @data:   CAN XL frame payload (CANXL_MIN_DLEN .. CANXL_MAX_DLEN byte)
  */
 struct canxl_frame {
-       canid_t prio;  /* 11 bit priority for arbitration (canid_t) */
-       __u8    flags; /* additional flags for CAN XL */
-       __u8    sdt;   /* SDU (service data unit) type */
-       __u16   len;   /* frame payload length in byte */
-       __u32   af;    /* acceptance field */
-       __u8    data[CANXL_MAX_DLEN];
+#if defined(__LITTLE_ENDIAN)
+       __u16 prio;   /* 11 bit priority for arbitration */
+       __u8  vcid;   /* virtual CAN network identifier */
+       __u8  __res0; /* reserved / padding must be set to zero */
+#elif defined(__BIG_ENDIAN)
+       __u8  __res0; /* reserved / padding must be set to zero */
+       __u8  vcid;   /* virtual CAN network identifier */
+       __u16 prio;   /* 11 bit priority for arbitration */
+#else
+#error "Unknown endianness"
+#endif
+       __u8  flags;  /* additional flags for CAN XL */
+       __u8  sdt;    /* SDU (service data unit) type */
+       __u16 len;    /* frame payload length in byte */
+       __u32 af;     /* acceptance field */
+       __u8  data[CANXL_MAX_DLEN];
 };
 
 #define CAN_MTU                (sizeof(struct can_frame))
 #define CANFD_MTU      (sizeof(struct canfd_frame))
 #define CANXL_MTU      (sizeof(struct canxl_frame))
include/uapi/linux/can/raw.h +14 -0
index 31622c9b7988..88e4e8176b89 100644
@@ -63,8 +63,22 @@ enum {
        CAN_RAW_LOOPBACK,       /* local loopback (default:on)       */
        CAN_RAW_RECV_OWN_MSGS,  /* receive my own msgs (default:off) */
        CAN_RAW_FD_FRAMES,      /* allow CAN FD frames (default:off) */
        CAN_RAW_JOIN_FILTERS,   /* all filters must match to trigger */
        CAN_RAW_XL_FRAMES,      /* allow CAN XL frames (default:off) */
+       CAN_RAW_XL_VCID_OPTS,   /* CAN XL VCID configuration options */
 };
 
+/* configuration for CAN XL virtual CAN identifier (VCID) handling */
+struct can_raw_vcid_options {
+       __u8 flags;             /* flags for vcid (filter) behaviour */
+       __u8 tx_vcid;           /* VCID value set into canxl_frame.vcid */
+       __u8 rx_vcid;           /* VCID value for VCID filter */
+       __u8 rx_vcid_mask;      /* VCID mask for VCID filter */
+};
+
+/* can_raw_vcid_options.flags for CAN XL virtual CAN identifier handling */
+#define CAN_RAW_XL_VCID_TX_SET         0x01
+#define CAN_RAW_XL_VCID_TX_PASS                0x02
+#define CAN_RAW_XL_VCID_RX_FILTER      0x04
+
 #endif /* !_UAPI_CAN_RAW_H */
net/can/af_can.c +2 -0
index 7343fd487dbe..707576eeeb58 100644
@@ -863,10 +863,12 @@ static __init int can_init(void)
        int err;
 
        /* check for correct padding to be able to use the structs similarly */
        BUILD_BUG_ON(offsetof(struct can_frame, len) !=
                     offsetof(struct canfd_frame, len) ||
+                    offsetof(struct can_frame, len) !=
+                    offsetof(struct canxl_frame, flags) ||
                     offsetof(struct can_frame, data) !=
                     offsetof(struct canfd_frame, data));
 
        pr_info("can: controller area network core\n");
 
net/can/raw.c +51 -3
index e6b822624ba2..3083df64723c 100644
@@ -89,10 +89,11 @@ struct raw_sock {
        struct list_head notifier;
        int loopback;
        int recv_own_msgs;
        int fd_frames;
        int xl_frames;
+       struct can_raw_vcid_options raw_vcid_opts;
        int join_filters;
        int count;                 /* number of active filters */
        struct can_filter dfilter; /* default/single filter */
        struct can_filter *filter; /* pointer to filter(s) */
        can_err_mask_t err_mask;
@@ -132,14 +133,34 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
        /* check the received tx sock reference */
        if (!ro->recv_own_msgs && oskb->sk == sk)
                return;
 
        /* make sure to not pass oversized frames to the socket */
-       if ((!ro->fd_frames && can_is_canfd_skb(oskb)) ||
-           (!ro->xl_frames && can_is_canxl_skb(oskb)))
+       if (!ro->fd_frames && can_is_canfd_skb(oskb))
                return;
 
+       if (can_is_canxl_skb(oskb)) {
+               struct canxl_frame *cxl = (struct canxl_frame *)oskb->data;
+               struct can_raw_vcid_options *ropts = &ro->raw_vcid_opts;
+
+               /* make sure to not pass oversized frames to the socket */
+               if (!ro->xl_frames)
+                       return;
+
+               /* filter CAN XL VCID content */
+               if (ropts->flags & CAN_RAW_XL_VCID_RX_FILTER) {
+                       /* apply VCID filter if user enabled the filter */
+                       if ((cxl->vcid & ropts->rx_vcid_mask) !=
+                           (ropts->rx_vcid & ropts->rx_vcid_mask))
+                               return;
+               } else {
+                       /* no filter => do not forward VCID tagged frames */
+                       if (cxl->vcid)
+                               return;
+               }
+       }
+
        /* eliminate multiple filter matches for the same skb */
        if (this_cpu_ptr(ro->uniq)->skb == oskb &&
            this_cpu_ptr(ro->uniq)->skbcnt == can_skb_prv(oskb)->skbcnt) {
                if (!ro->join_filters)
                        return;
@@ -696,10 +717,19 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
                /* Enabling CAN XL includes CAN FD */
                if (ro->xl_frames)
                        ro->fd_frames = ro->xl_frames;
                break;
 
+       case CAN_RAW_XL_VCID_OPTS:
+               if (optlen != sizeof(ro->raw_vcid_opts))
+                       return -EINVAL;
+
+               if (copy_from_sockptr(&ro->raw_vcid_opts, optval, optlen))
+                       return -EFAULT;
+
+               break;
+
        case CAN_RAW_JOIN_FILTERS:
                if (optlen != sizeof(ro->join_filters))
                        return -EINVAL;
 
                if (copy_from_sockptr(&ro->join_filters, optval, optlen))
@@ -784,10 +814,25 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
                if (len > sizeof(int))
                        len = sizeof(int);
                val = &ro->xl_frames;
                break;
 
+       case CAN_RAW_XL_VCID_OPTS:
+               /* user space buffer to small for VCID opts? */
+               if (len < sizeof(ro->raw_vcid_opts)) {
+                       /* return -ERANGE and needed space in optlen */
+                       err = -ERANGE;
+                       if (put_user(sizeof(ro->raw_vcid_opts), optlen))
+                               err = -EFAULT;
+               } else {
+                       if (len > sizeof(ro->raw_vcid_opts))
+                               len = sizeof(ro->raw_vcid_opts);
+                       if (copy_to_user(optval, &ro->raw_vcid_opts, len))
+                               err = -EFAULT;
+               }
+               break;
+
        case CAN_RAW_JOIN_FILTERS:
                if (len > sizeof(int))
                        len = sizeof(int);
                val = &ro->join_filters;
                break;
@@ -814,12 +859,27 @@ static bool raw_bad_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
            (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
                return false;
 
        /* 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(mtu)) {
+               struct canxl_frame *cxl = (struct canxl_frame *)skb->data;
+               struct can_raw_vcid_options *ropts = &ro->raw_vcid_opts;
+

net/can/raw.c static bool raw_bad_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu) · line 877

Quoted 10 lines
net/can/raw.c +10 -0
+               /* sanitize non CAN XL bits */
+               cxl->__res0 = 0;
+
+               /* clear VCID in CAN XL frame if pass through is disabled */
+               if (!(ropts->flags & CAN_RAW_XL_VCID_TX_PASS))
+                       cxl->vcid = 0;
+
+               /* set VCID in CAN XL frame if enabled */
+               if (ropts->flags & CAN_RAW_XL_VCID_TX_SET)
+                       cxl->vcid = ropts->tx_vcid;

Just in terms of software design, I am not sure why this sanitization is part of raw_bad_txframe(). The purpose of this function is, so far, to just return a boolean telling whether or not the frame is valid. It is surprising to repurpose this to also do the sanitization. I would rather see the above code go in a new dedicated function (maybe raw_sanitize_tx_frame()) and call that new function from raw_sendmsg().

Quoted 11 lines
net/can/raw.c +1 -1
                return false;
+       }
 
        return true;
 }
 
 static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
--

2.39.2

author maintainer
#2 lore raw

Hi Vincent,

Quoted 1 line · Vincent MAILHOL

Sorry for the delay, last weeks were busier than average.

No problem. I just wanted to finalize the data structure for further work on the CAN XL hardware driver and some CAN CiA protocol implementations.

Quoted 2 lines

I am not yet aware of all the details of CAN XL and its VCID, but to the extent of my knowledge, the patch looks good.

Thanks.

Quoted 3 lines

I left two nitpicks. Notwithstanding: you can add my review tag to v5.

I've sent out a v5 that addresses both of your good remarks:
https://lore.kernel.org/linux-can/[email protected]/

Quoted 3 lines
Quoted 1 line

+ * functionality as is holds the @can_id flags CAN_xxx_FLAG

                                 ^^

as it holds

Fixed.

Quoted 3 lines
Quoted 3 lines
(quoted diff) +0 -0
@@ -814,12 +859,27 @@ static bool raw_bad_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
             (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
                 return false;

(..)

Quoted 7 lines

Just in terms of software design, I am not sure why this sanitization is part of raw_bad_txframe(). The purpose of this function is, so far, to just return a boolean telling whether or not the frame is valid. It is surprising to repurpose this to also do the sanitization. I would rather see the above code go in a new dedicated function (maybe raw_sanitize_tx_frame()) and call that new function from raw_sendmsg().

Right. I felt similar that the naming does not fit that good anymore when adding the VCID handling :-/

I splitted up the raw_bad_txframe() function and created two new functions:

raw_check_txframe() (is raw_bad_txframe() with a non-bool return value) raw_put_canxl_vcid() (which handles the VCID content for CAN XL frames)

Best regards,
Oliver