Re: [PATCH] canxl: add virtual CAN network identifier support

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hi all,

I've been working some time how to integrate the CAN XL VCID stuff into 
the Linux CAN infrastructure.

Going for a complete IEEE 802.1Q VLAN-style was not possible due to 
several reasons. Getting the VCID from the system via cmsg could have 
been done with a new extended cmsg analogue to the packet socket. But 
'sending' different VCIDs is not intended and would really lead to a 
complete mess.

To be able to read/write complete CAN XL frames and to store and replay 
the entire CAN traffic (with candump/canplayer) the VCID is now stored 
in an unused space in the prio element. Looks a bit ugly but is handy to 
use and does not interfere the filter infrastructure and CAN/CANFD frames.

Defining VLAN-like CAN network devices like canxl0.123 could still be 
done later. Same applies to Ethernet-like software interfaces to carry 
IP over CAN XL.

Some PoC tools and a WireShark plugin to 'decode' CAN XL traffic can be 
found here:

https://github.com/hartkopp/canxl-utils

Thanks for review and feedback.

Best regards,
Oliver

On 06.01.24 20:28, Oliver Hartkopp wrote:
> 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 send, 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.prio 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, CANXL_VCID flag unset) are
> delivered to the user space.
> 
> The 8-bit VCID is stored inside the CAN XL prio element (only in CAN XL
> frames!) to not interfere with other CAN content or the CAN filters
> provided by the CAN_RAW sockets and kernel infrastruture.
> 
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
>   include/uapi/linux/can.h     | 14 +++++--
>   include/uapi/linux/can/raw.h | 14 +++++++
>   net/can/raw.c                | 79 ++++++++++++++++++++++++++++++++++--
>   3 files changed, 100 insertions(+), 7 deletions(-)
> 
> diff --git a/include/uapi/linux/can.h b/include/uapi/linux/can.h
> index 939db2388208..30453905fbd2 100644
> --- a/include/uapi/linux/can.h
> +++ b/include/uapi/linux/can.h
> @@ -180,34 +180,40 @@ struct canfd_frame {
>   };
>   
>   /*
>    * defined bits for canxl_frame.flags
>    *
> - * The canxl_frame.flags element contains two bits CANXL_XLF and CANXL_SEC
> - * and shares the relative position of the struct can[fd]_frame.len element.
> + * The canxl_frame.flags element contains CAN XL frame specific bits and
> + * shares the relative position of the struct can[fd]_frame.len element.
>    * The CANXL_XLF bit ALWAYS needs to be set to indicate a valid CAN XL frame.
>    * As a side effect setting this bit intentionally breaks the length checks
>    * for Classical CAN and CAN FD frames.
>    *
>    * Undefined bits in canxl_frame.flags are reserved and shall be set to zero.
>    */
>   #define CANXL_XLF 0x80 /* mandatory CAN XL frame flag (must always be set!) */
>   #define CANXL_SEC 0x01 /* Simple Extended Content (security/segmentation) */
> +#define CANXL_VCID 0x02 /* prio contains the virtual CAN network identifier */
> +
> +/* the 8-bit VCID is optionally placed in the canxl_frame.prio element */
> +#define CANXL_VCID_OFFSET 16 /* bit offset of VCID in prio element */
> +#define CANXL_VCID_VAL_MASK 0xFFUL /* VCID is an 8-bit value */
> +#define CANXL_VCID_MASK (CANXL_VCID_VAL_MASK << CANXL_VCID_OFFSET)
>   
>   /**
>    * struct canxl_frame - CAN with e'X'tended frame 'L'ength frame structure
> - * @prio:  11 bit arbitration priority with zero'ed CAN_*_FLAG flags
> + * @prio:  11 bit arbitration priority with zero'ed CAN_*_FLAG flags / VCID
>    * @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.
>    */
>   struct canxl_frame {
> -	canid_t prio;  /* 11 bit priority for arbitration (canid_t) */
> +	canid_t prio;  /* 11 bit priority for arbitration / 8 bit VCID */
>   	__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];
> diff --git a/include/uapi/linux/can/raw.h b/include/uapi/linux/can/raw.h
> index 31622c9b7988..8890b0d2fd48 100644
> --- a/include/uapi/linux/can/raw.h
> +++ b/include/uapi/linux/can/raw.h
> @@ -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 */
>   };
>   
> +struct can_raw_vcid_options {
> +
> +	__u8 flags;		/* flags for vcid (filter) behaviour */
> +	__u8 tx_vcid;		/* VCID value set into canxl_frame.prio */
> +	__u8 rx_vcid;		/* VCID value for VCID filter */
> +	__u8 rx_vcid_mask;	/* VCID mask for VCID filter */
> +
> +};
> +
> +#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 */
> diff --git a/net/can/raw.c b/net/can/raw.c
> index e6b822624ba2..a6f0b6a0a934 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -89,10 +89,14 @@ 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;
> +	canid_t tx_vcid_shifted;
> +	canid_t rx_vcid_shifted;
> +	canid_t rx_vcid_mask_shifted;
>   	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 +136,33 @@ 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;
> +
> +		/* make sure to not pass oversized frames to the socket */
> +		if (!ro->xl_frames)
> +			return;
> +
> +		/* filter CAN XL VCID content */
> +		if (ro->raw_vcid_opts.flags & CAN_RAW_XL_VCID_RX_FILTER) {
> +			/* apply VCID filter if user enabled the filter */
> +			if ((cxl->prio & ro->rx_vcid_mask_shifted) !=
> +			    (ro->rx_vcid_shifted & ro->rx_vcid_mask_shifted))
> +				return;
> +		} else {
> +			/* no filter => do not forward VCID tagged frames */
> +			if (cxl->prio & CANXL_VCID_MASK)
> +				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 +719,23 @@ 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;
> +
> +		/* prepare 32 bit values for handling in hot path */
> +		ro->tx_vcid_shifted = ro->raw_vcid_opts.tx_vcid << CANXL_VCID_OFFSET;
> +		ro->rx_vcid_shifted = ro->raw_vcid_opts.rx_vcid << CANXL_VCID_OFFSET;
> +		ro->rx_vcid_mask_shifted = ro->raw_vcid_opts.rx_vcid_mask << CANXL_VCID_OFFSET;
> +		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 +820,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 +865,34 @@ 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;
> +
> +		/* sanitize non CAN XL bits */
> +		cxl->prio &= (CANXL_PRIO_MASK | CANXL_VCID_MASK);
> +
> +		/* clear VCID in CAN XL frame if pass through is disabled */
> +		if (!(ro->raw_vcid_opts.flags & CAN_RAW_XL_VCID_TX_PASS))
> +			cxl->prio &= CANXL_PRIO_MASK;
> +
> +		/* set VCID in CAN XL frame if enabled */
> +		if (ro->raw_vcid_opts.flags & CAN_RAW_XL_VCID_TX_SET) {
> +			cxl->prio &= CANXL_PRIO_MASK;
> +			cxl->prio |= ro->tx_vcid_shifted;
> +		}
> +
> +		/* set flag only for 'tagged' VCID values */
> +		if (cxl->prio & CANXL_VCID_MASK)
> +			cxl->flags |= CANXL_VCID;
> +		else
> +			cxl->flags &= ~CANXL_VCID;
> +
>   		return false;
> +	}
>   
>   	return true;
>   }
>   
>   static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)