CC-2174: modify CAN frame header structure to match updated struct ca… (#1851)

* CC-2174: modify CAN frame header structure to match updated struct can_frame from kernel

* allow only valid fd message lengths

* add remote message special case for dlc handling

* fix pylint issues

* verify non FD frame before assigning the len8 DLC

* Fix formatting

---------

Co-authored-by: Gal Rahamim <gal.rahamim@cymotive.com>
Co-authored-by: Eldad Sitbon <eldad.sitbon@cymotive.com>
Co-authored-by: rahagal <114643899+rahagal@users.noreply.github.com>

Author: 3 people

can/interfaces/socketcan/constants.py

@@ -53,8 +53,18 @@
 SIOCGSTAMP = 0x8906
 EXTFLG = 0x0004
 
-CANFD_BRS = 0x01
-CANFD_ESI = 0x02
+CANFD_BRS = 0x01  # bit rate switch (second bitrate for payload data)
+CANFD_ESI = 0x02  # error state indicator of the transmitting node
+CANFD_FDF = 0x04  # mark CAN FD for dual use of struct canfd_frame
+
+# CAN payload length and DLC definitions according to ISO 11898-1
+CAN_MAX_DLC = 8
+CAN_MAX_RAW_DLC = 15
+CAN_MAX_DLEN = 8
+
+# CAN FD payload length and DLC definitions according to ISO 11898-7
+CANFD_MAX_DLC = 15
+CANFD_MAX_DLEN = 64
 
 CANFD_MTU = 72
 

can/interfaces/socketcan/socketcan.py

@@ -139,23 +139,68 @@ def bcm_header_factory(
 # The 32bit can id is directly followed by the 8bit data link count
 # The data field is aligned on an 8 byte boundary, hence we add padding
 # which aligns the data field to an 8 byte boundary.
-CAN_FRAME_HEADER_STRUCT = struct.Struct("=IBB2x")
+CAN_FRAME_HEADER_STRUCT = struct.Struct("=IBB1xB")
 
 
 def build_can_frame(msg: Message) -> bytes:
     """CAN frame packing/unpacking (see 'struct can_frame' in <linux/can.h>)
     /**
-     * struct can_frame - basic CAN frame structure
-     * @can_id:  the CAN ID of the frame and CAN_*_FLAG flags, see above.
-     * @can_dlc: the data length field of the CAN frame
-     * @data:    the CAN frame payload.
-     */
+    * 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)
+    * @__pad:    padding
+    * @__res0:   reserved / padding
+    * @len8_dlc: optional DLC value (9 .. 15) at 8 byte payload length
+    *            len8_dlc contains values from 9 .. 15 when the payload length is
+    *            8 bytes but the DLC value (see ISO 11898-1) is greater then 8.
+    *            CAN_CTRLMODE_CC_LEN8_DLC flag has to be enabled in CAN driver.
+    * @data:     CAN frame payload (up to 8 byte)
+    */
     struct can_frame {
         canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
-        __u8    can_dlc; /* data length code: 0 .. 8 */
-        __u8    data[8] __attribute__((aligned(8)));
+        union {
+            /* CAN frame payload length in byte (0 .. CAN_MAX_DLEN)
+            * was previously named can_dlc so we need to carry that
+            * name for legacy support
+            */
+            __u8 len;
+            __u8 can_dlc; /* deprecated */
+        } __attribute__((packed)); /* disable padding added in some ABIs */
+        __u8 __pad; /* padding */
+        __u8 __res0; /* reserved / padding */
+        __u8 len8_dlc; /* optional DLC for 8 byte payload length (9 .. 15) */
+        __u8 data[CAN_MAX_DLEN] __attribute__((aligned(8)));
     };
 
+    /*
+    * defined bits for canfd_frame.flags
+    *
+    * The use of struct canfd_frame implies the FD Frame (FDF) bit to
+    * be set in the CAN frame bitstream on the wire. The FDF bit switch turns
+    * the CAN controllers bitstream processor into the CAN FD mode which creates
+    * two new options within the CAN FD frame specification:
+    *
+    * Bit Rate Switch - to indicate a second bitrate is/was used for the payload
+    * Error State Indicator - represents the error state of the transmitting node
+    *
+    * As the CANFD_ESI bit is internally generated by the transmitting CAN
+    * controller only the CANFD_BRS bit is relevant for real CAN controllers when
+    * building a CAN FD frame for transmission. Setting the CANFD_ESI bit can make
+    * sense for virtual CAN interfaces to test applications with echoed frames.
+    *
+    * The struct can_frame and struct canfd_frame intentionally share the same
+    * layout to be able to write CAN frame content into a CAN FD frame structure.
+    * 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).
+    * Since the introduction of CAN XL the CANFD_FDF flag is set in all CAN FD
+    * frame structures provided by the CAN subsystem of the Linux kernel.
+    */
+    #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 */
+
     /**
     * struct canfd_frame - CAN flexible data rate frame structure
     * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
@@ -175,14 +220,30 @@ def build_can_frame(msg: Message) -> bytes:
     };
     """
     can_id = _compose_arbitration_id(msg)
+
     flags = 0
+
+    # The socketcan code identify the received FD frame by the packet length.
+    # So, padding to the data length is performed according to the message type (Classic / FD)
+    if msg.is_fd:
+        flags |= constants.CANFD_FDF
+        max_len = constants.CANFD_MAX_DLEN
+    else:
+        max_len = constants.CAN_MAX_DLEN
+
     if msg.bitrate_switch:
         flags |= constants.CANFD_BRS
     if msg.error_state_indicator:
         flags |= constants.CANFD_ESI
-    max_len = 64 if msg.is_fd else 8
+
     data = bytes(msg.data).ljust(max_len, b"\x00")
-    return CAN_FRAME_HEADER_STRUCT.pack(can_id, msg.dlc, flags) + data
+
+    if msg.is_remote_frame:
+        data_len = msg.dlc
+    else:
+        data_len = min(i for i in can.util.CAN_FD_DLC if i >= len(msg.data))
+    header = CAN_FRAME_HEADER_STRUCT.pack(can_id, data_len, flags, msg.dlc)
+    return header + data
 
 
 def build_bcm_header(
@@ -259,12 +320,31 @@ def build_bcm_update_header(can_id: int, msg_flags: int, nframes: int = 1) -> by
     )
 
 
+def is_frame_fd(frame: bytes):
+    # According to the SocketCAN implementation the frame length
+    # should indicate if the message is FD or not (not the flag value)
+    return len(frame) == constants.CANFD_MTU
+
+
 def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]:
-    can_id, can_dlc, flags = CAN_FRAME_HEADER_STRUCT.unpack_from(frame)
-    if len(frame) != constants.CANFD_MTU:
+    can_id, data_len, flags, len8_dlc = CAN_FRAME_HEADER_STRUCT.unpack_from(frame)
+
+    if data_len not in can.util.CAN_FD_DLC:
+        data_len = min(i for i in can.util.CAN_FD_DLC if i >= data_len)
+
+    can_dlc = data_len
+
+    if not is_frame_fd(frame):
         # Flags not valid in non-FD frames
         flags = 0
-    return can_id, can_dlc, flags, frame[8 : 8 + can_dlc]
+
+        if (
+            data_len == constants.CAN_MAX_DLEN
+            and constants.CAN_MAX_DLEN < len8_dlc <= constants.CAN_MAX_RAW_DLC
+        ):
+            can_dlc = len8_dlc
+
+    return can_id, can_dlc, flags, frame[8 : 8 + data_len]
 
 
 def create_bcm_socket(channel: str) -> socket.socket: