Re: [RFC PATCH 1/5] can: canxl: introduce CAN XL data structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue. 12 Jul. 2022 at 03:44, Oliver Hartkopp <socketcan@xxxxxxxxxxxx> wrote:
>
> This patch adds defines for data structures and length information for
> CAN XL (CAN with eXtended data Length) which can transfer up to 2048
> byte insinde a single frame.
>
> Notable changes from CAN FD:
>
> - the 11 bit arbitration field is now named 'priority' instead of 'can_id'
>   (there are no 29 bit identifiers nor RTR frames anymore)
> - the data length needs a uint16 value to cover up to 2048 byte
>   (the length element position is different to struct can[fd]_frame)
> - new fields (SDT, AF) and a SEC bit have been introduced
> - the virtual CAN interface identifier is not part if the CAN XL frame
>   struct as this VCID value is stored in struct skbuff (analog to vlan id)
>
> Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
> ---
>  include/uapi/linux/can.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/include/uapi/linux/can.h b/include/uapi/linux/can.h
> index 90801ada2bbe..9f97a5d06f3b 100644
> --- a/include/uapi/linux/can.h
> +++ b/include/uapi/linux/can.h
> @@ -58,10 +58,11 @@
>
>  /* valid bits in CAN ID for frame formats */
>  #define CAN_SFF_MASK 0x000007FFU /* standard frame format (SFF) */
>  #define CAN_EFF_MASK 0x1FFFFFFFU /* extended frame format (EFF) */
>  #define CAN_ERR_MASK 0x1FFFFFFFU /* omit EFF, RTR, ERR flags */
> +#define CANXL_PRIO_MASK CAN_SFF_MASK /* 11 bit priority mask */
>
>  /*
>   * Controller Area Network Identifier structure
>   *
>   * bit 0-28    : CAN identifier (11/29 bit)
> @@ -71,10 +72,11 @@
>   */
>  typedef __u32 canid_t;
>
>  #define CAN_SFF_ID_BITS                11
>  #define CAN_EFF_ID_BITS                29
> +#define CANXL_PRIO_BITS                CAN_SFF_ID_BITS
>
>  /*
>   * Controller Area Network Error Message Frame Mask structure
>   *
>   * bit 0-28    : error class mask (see include/uapi/linux/can/error.h)
> @@ -89,10 +91,18 @@ typedef __u32 can_err_mask_t;
>
>  /* CAN FD payload length and DLC definitions according to ISO 11898-7 */
>  #define CANFD_MAX_DLC 15
>  #define CANFD_MAX_DLEN 64
>
> +/*
> + * CAN XL payload length and DLC definitions according to ISO 11898-1
> + * CAN XL DLC ranges from 0 .. 2047 => data length from 1 .. 2048 byte
> + */
> +#define CANXL_MAX_DLC 2047
> +#define CANXL_MAX_DLC_MASK 0x07FF
> +#define CANXL_MAX_DLEN 2048
> +
>  /**
>   * struct can_frame - Classical CAN frame structure (aka CAN 2.0B)
>   * @can_id:   CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
>   * @len:      CAN frame payload length in byte (0 .. 8)
>   * @can_dlc:  deprecated name for CAN frame payload length in byte (0 .. 8)
> @@ -141,14 +151,20 @@ struct can_frame {
>   * When this is done the former differentiation via CAN_MTU / CANFD_MTU gets
>   * lost. CANFD_FDF allows programmers to mark CAN FD frames in the case of
>   * using struct canfd_frame for mixed CAN / CAN FD content (dual use).
>   * N.B. the Kernel APIs do NOT provide mixed CAN / CAN FD content inside of
>   * struct canfd_frame therefore the CANFD_FDF flag is disregarded by Linux.
> + * Same applies to the CANXL_XLF bit.
> + *
> + * For CAN XL the SEC bit has been added to the flags field which shares the
> + * same position in struct can[fd|xl]_frame.
>   */
>  #define CANFD_BRS 0x01 /* bit rate switch (second bitrate for payload data) */
>  #define CANFD_ESI 0x02 /* error state indicator of the transmitting node */
>  #define CANFD_FDF 0x04 /* mark CAN FD for dual use of struct canfd_frame */
> +#define CANXL_XLF 0x08 /* mark CAN XL for dual use of struct canfd_frame */
> +#define CANXL_SEC 0x10 /* Simple Extended Content (security/segmentation) */
>
>  /**
>   * struct canfd_frame - CAN flexible data rate frame structure
>   * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
>   * @len:    frame payload length in byte (0 .. CANFD_MAX_DLEN)
> @@ -164,12 +180,34 @@ struct canfd_frame {
>         __u8    __res0;  /* reserved / padding */
>         __u8    __res1;  /* reserved / padding */
>         __u8    data[CANFD_MAX_DLEN] __attribute__((aligned(8)));
>  };
>
> +/**
> + * struct canxl_frame - CAN with e'X'tended frame 'L'ength frame structure
> + * @prio:  11 bit arbitration priority with zero'ed CAN_*_FLAG flags
> + * @sdt:   SDU (service data unit) type
> + * @flags: additional flags for CAN XL
> + * @len:   frame payload length in byte (1 .. CANXL_MAX_DLEN)
> + * @af:    acceptance field
> + * @data:  CAN XL frame payload (up to CANXL_MAX_DLEN byte)
> + *
> + * @prio shares the same position as @can_id from struct canfd_frame.
> + * Same applies to the relative position and length of @flags.
> + */
> +struct canxl_frame {
> +       canid_t prio;  /* 11 bit priority for arbitration (canid_t) */
> +       __u8    sdt;   /* SDU (service data unit) type */
> +       __u8    flags; /* additional flags for CAN XL */
> +       __u16   len;   /* frame payload length in byte */
> +       __u32   af;    /* acceptance field */
> +       __u8    data[CANXL_MAX_DLEN];

__u8 data[];

2 kilobytes start to be a significant size. Would it make sense to use
a flexible array member to minimize the copies? A bit like the TCP/IP
stack where you do not have to allocate the MTU but just what is
actually needed for the headers and your payload.

Of course this is a tradeoff. It will add some complexity. The first
impact is that the skb's data length might be smaller than the MTU and
thus any logic using the MTU to differentiate between Classic CAN,
CAN-FD or CAN XL would have to be adjusted.

Also, are we fine to drop the __attribute__((aligned(8)))? If I
understand correctly, we moved from a 8 bytes alignment in struct
can(fd)_frame to a 4 bytes alignment in struct canxl_frame.

>
> +};
> +
>  #define CAN_MTU                (sizeof(struct can_frame))
>  #define CANFD_MTU      (sizeof(struct canfd_frame))
> +#define CANXL_MTU      (sizeof(struct canxl_frame))

#define CANXL_MTU      (sizeof(struct canxl_frame) + CANXL_MAX_DLEN)

>  /* particular protocols of the protocol family PF_CAN */
>  #define CAN_RAW                1 /* RAW sockets */
>  #define CAN_BCM                2 /* Broadcast Manager */
>  #define CAN_TP16       3 /* VAG Transport Protocol v1.6 */


Yours sincerely,
Vincent Mailhol



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux