canxl: add virtual CAN network identifier support
Subsystems (2)
Oliver Hartkopp
maintainerPATCH 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 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.
--- 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(-)
include/uapi/linux/can.h +10 -4
@@ -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 ];
include/uapi/linux/can/raw.h +14 -0
@@ -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 */
net/can/raw.c +76 -3
132 lines not on this page · raw message
Signature · 1 line
2.34.1
Oliver Hartkopp
maintainerHi 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
Quoted 250 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_RAWsockets a new socket optionCAN_RAW_XL_VCID_OPTSis 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 spaceWith the 'tx pass through' option
CAN_RAW_XL_VCID_TX_PASSall 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_SEToption will override the VCID value in the struct canxl_frame.prio defined forCAN_RAW_XL_VCID_TX_PASSwhen both flags are set.With a
rx_vcid_maskof zero all possible VCID values (0x00-0xFF) are passed to the user space when theCAN_RAW_XL_VCID_RX_FILTERflag is set. Without this flag only untagged CAN XL frames (VCID =0x00,CANXL_VCIDflag 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_RAWsockets and kernel infrastruture.Signed-off-by Oliver Hartkopp <[email protected]>--- 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(-)include/uapi/linux/can.h +10
-4@@ -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 ];include/uapi/linux/can/raw.h +14
-0@@ -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 */ net/can/raw.c +76
-3@@ -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)
Quoted 7 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_RAWsockets a new socket optionCAN_RAW_XL_VCID_OPTSis introduced to define/access VCID content:
Quoted 6 lines
- 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 spaceWith the 'tx pass through' option
CAN_RAW_XL_VCID_TX_PASSall valid VCID values can be send, e.g. to replay full qualified CAN XL traffic.
^^^^
sent
Quoted 5 lines
The VCID value provided for the
CAN_RAW_XL_VCID_TX_SEToption will override the VCID value in the struct canxl_frame.prio defined forCAN_RAW_XL_VCID_TX_PASSwhen both flags are set.
Quoted 8 lines
With a
rx_vcid_maskof zero all possible VCID values (0x00-0xFF) are passed to the user space when theCAN_RAW_XL_VCID_RX_FILTERflag is set. Without this flag only untagged CAN XL frames (VCID =0x00,CANXL_VCIDflag 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_RAWsockets and kernel infrastruture.
^^^^^^^^^^^^^
infrastructure
Quoted 42 lines
Signed-off-by Oliver Hartkopp <[email protected]>--- 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(-)include/uapi/linux/can.h +9
-3@@ -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
include/uapi/linux/can.h struct canfd_frame { · line 214
Quoted 10 lines
include/uapi/linux/can.h +1
-1* @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 */
Isn't this a UAPI breaking change? Prior to this patch, the applications may do:
canxl_frame.prio
to get the prio, but after this patch, applications are required to do:
canxl_frame.prio & CANXL_PRIO_MASK
in order to mask out the VCID (currently, there are no requirements that canxl_frame.prio must be masked before use).
In the past, I was reluctant to acknowledge the introduction of CANXL in the kernel prior to reading the ISO standard because I was afraid of such UAPI stability issues. Now we have to deal with it.
What do you think of:
struct canxl_frame {
#if defined(__LITTLE_ENDIAN)
__u16 prio; /* 11 bit priority for arbitration */
__u8 vcid; /* 8 bit VCID */
__u8 __reserved; /* must be 0 */
/* ... */
#elif defined(__BIG_ENDIAN)
__u8 __reserved; /* must be 0 */
__u8 vcid; /* 8 bit VCID */
__u16 prio; /* 11 bit priority for arbitration */
#else
#error "Unknown endianness"
#endif
}Here, canxl_frame.prio always gives a correct value without need for CANXL_PRIO_MASK masking. The big/little endianness checks are needed to maintain the ABI compatibility. Not yet tested, so forgive if there is a mistake. Getting the endianness logic correct on a first try is not easy.
Also, the VCID can now be accessed through canxl_frame.vcid instead of relying on some mask and shift logic.
The drawback is that you lose the can_id type. For what I understand, this is only used for filtering. If we absolutely need to maintain the canid_t, then maybe:
struct canxl_frame {
union {
canid_t filter;
struct {
#if defined(__LITTLE_ENDIAN)
__u16 prio; /* 11 bit priority for arbitration */
__u8 vcid; /* 8 bit VCID */
__u8 __reserved; /* must be 0 */
#elif defined(__BIG_ENDIAN)
__u8 __reserved; /* must be 0 */
__u8 vcid; /* 8 bit VCID */
__u16 prio; /* 11 bit priority for arbitration */
#else
#error "Unknown endianness"
#endif
};
};
/* ... */
}But I think it is better to drop it. If someone wants a canid_t, then he or she can just cast the XL frame to either struct can_frame or struct canfd_frame.
Though?
include/uapi/linux/can.h struct canfd_frame { · line 215
Quoted 1 line
include/uapi/linux/can.h +0
-0__u8 flags;/* additional flags for CAN XL */
If CANXL_VCID is set, can vcid be zero? If not, no need for a flag. Just need to check if canxl_frame.vcid is not zero.
Quoted 176 lines
include/uapi/linux/can.h +0
-0__u8 sdt;/* SDU (service data unit) type */ __u16 len;/* frame payload length in byte */ __u32 af;/* acceptance field */ __u8 data[CANXL_MAX_DLEN ];include/uapi/linux/can/raw.h +14
-0@@ -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 */ net/can/raw.c +76
-4@@ -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) --2.34.1
Oliver Hartkopp
maintainerHi Vincent,
thanks for your review!
Quoted 5 lines · Vincent MAILHOL
Quoted 1 line · Oliver Hartkopp
values can be send, e.g. to replay full qualified CAN XL traffic.
^^^^sent
ACK
Quoted 3 lines
Quoted 1 line
provided by the
CAN_RAWsockets and kernel infrastruture.^^^^^^^^^^^^^infrastructure
ACK
Quoted 4 lines
include/uapi/linux/can.h struct canfd_frame { · line 214
Quoted 3 lines
struct canxl_frame {include/uapi/linux/can.h +1
-1-canid_t prio;/* 11 bit priority for arbitration (canid_t) */ +canid_t prio;/* 11 bit priority for arbitration / 8 bit VCID */
Quoted 8 lines
Isn't this a UAPI breaking change? Prior to this patch, the applications may do:
canxl_frame.prio
to get the prio, but after this patch, applications are required to do:
canxl_frame.prio &
CANXL_PRIO_MASK
Not really. I also thought about it but you *only* need to take care about the VCID content when you have enabled it explicitly with the new sockopt. Otherwise you will never see anything beyond the 32 bit prio.
Quoted 5 lines
in order to mask out the VCID (currently, there are no requirements that canxl_frame.prio must be masked before use).
In the past, I was reluctant to acknowledge the introduction of CANXL in the kernel prior to reading the ISO standard because I was afraid of such UAPI stability issues. Now we have to deal with it.
Yes, but that kind of extension would be backwards compatible.
Quoted 17 lines
What do you think of:
struct canxl_frame { #if defined(__LITTLE_ENDIAN) __u16 prio; /* 11 bit priority for arbitration */ __u8 vcid; /* 8 bit VCID */ __u8 __reserved; /* must be 0 */ /* ... */ #elif defined(__BIG_ENDIAN) __u8 __reserved; /* must be 0 */ __u8 vcid; /* 8 bit VCID */ __u16 prio; /* 11 bit priority for arbitration */ #else #error "Unknown endianness" #endif }
Quoted 5 lines
Here, canxl_frame.prio always gives a correct value without need for
CANXL_PRIO_MASKmasking. The big/little endianness checks are needed to maintain the ABI compatibility. Not yet tested, so forgive if there is a mistake. Getting the endianness logic correct on a first try is not easy.
Yes, I tested such approach too (with little endian only) and it worked great - and of course looked better in the code.
Quoted 2 lines
Also, the VCID can now be accessed through canxl_frame.vcid instead of relying on some mask and shift logic.
Right. That looked nice.
Quoted 24 lines
The drawback is that you lose the
can_idtype. For what I understand, this is only used for filtering. If we absolutely need to maintain thecanid_t, then maybe:struct canxl_frame { union { canid_t filter; struct { #if defined(__LITTLE_ENDIAN) __u16 prio; /* 11 bit priority for arbitration */ __u8 vcid; /* 8 bit VCID */ __u8 __reserved; /* must be 0 */ #elif defined(__BIG_ENDIAN) __u8 __reserved; /* must be 0 */ __u8 vcid; /* 8 bit VCID */ __u16 prio; /* 11 bit priority for arbitration */ #else #error "Unknown endianness" #endif }; }; /* ... */ }
Quoted 5 lines
But I think it is better to drop it. If someone wants a
canid_t, then he or she can just cast the XL frame to either structcan_frameor structcanfd_frame.Though?
My only concern is that it looks really ugly :-/
The change of the prio element from u32 to u16 will also not harm anyone as I assume to be the only person who's currently working with CAN XL frames on virtual CAN interfaces:
https://github.com/hartkopp?tab=repositories&q=can-cia
I'll prepare a patch that picks up this suggestion of an __u16 prio etc.
Maybe we can add some compile time checks to ensure the correct struct layout for this case.
Quoted 6 lines
Quoted 2 lines
__u8flags; /* additional flags for CAN XL */If
CANXL_VCIDis set, can vcid be zero? If not, no need for a flag. Just need to check if canxl_frame.vcid is not zero.
This is probably indeed a leftover which can be removed with my latest implementation. Will recheck.
Many thanks,
Oliver