diff mbox series

[v2,3/3] can: length: refactor frame lengths definition to add size in bits

Message ID 20230523065218.51227-4-mailhol.vincent@wanadoo.fr (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series can: length: fix definitions and add bit length calculation | expand

Checks

Context Check Description
netdev/tree_selection success Series ignored based on subject

Commit Message

Vincent Mailhol May 23, 2023, 6:52 a.m. UTC
Introduce a method to calculate the exact size in bits of a CAN(-FD)
frame with or without dynamic bitsuffing.

These are all the possible combinations taken into account:

  - Classical CAN or CAN-FD
  - Standard or Extended frame format
  - CAN-FD CRC17 or CRC21
  - Include or not intermission

Instead of doing several macro definitions, declare the
can_frame_bits() static inline function. To this extend, do a full
refactoring of the length definitions.

If given integer constant expressions as argument, can_frame_bits()
folds into a compile time constant expression, giving no penalty over
the use of macros.

Also add the can_frame_bytes(). This function replaces the existing
macro:

  - CAN_FRAME_OVERHEAD_SFF: can_frame_bytes(false, false, 0)
  - CAN_FRAME_OVERHEAD_EFF: can_frame_bytes(false, true, 0)
  - CANFD_FRAME_OVERHEAD_SFF: can_frame_bytes(true, false, 0)
  - CANFD_FRAME_OVERHEAD_EFF: can_frame_bytes(true, true, 0)

The different frame lengths (including intermission) are as follow:

   Frame type				bits	bytes
  ----------------------------------------------------------
   Classic CAN SFF no-bitstuffing	111	14
   Classic CAN EFF no-bitstuffing	131	17
   Classic CAN SFF bitstuffing		135	17
   Classic CAN EFF bitstuffing		160	20
   CAN-FD SFF no-bitstuffing		579	73
   CAN-FD EFF no-bitstuffing		598	75
   CAN-FD SFF bitstuffing		712	89
   CAN-FD EFF bitstuffing		736	92

The macro CAN_FRAME_LEN_MAX and CANFD_FRAME_LEN_MAX are kept to be
used in const struct declarations (folding of can_frame_bytes() occurs
too late in the compilation to be used in struct declarations).

In addition to the above:

 - Use ISO 11898-1:2015 definitions for the name of the CAN frame
   fields.
 - Include linux/bits.h for use of BITS_PER_BYTE.
 - Include linux/math.h for use of mult_frac() and
   DIV_ROUND_UP(). N.B: the use of DIV_ROUND_UP() is not new to this
   patch, but the include was previously omitted.
 - Add copyright 2023 for myself.

[1] commit 85d99c3e2a13 ("can: length: can_skb_get_frame_len(): introduce
    function to get data length of frame in data link layer")
Link: https://git.kernel.org/torvalds/c/85d99c3e2a13

[2] RE: [PATCH] can: mcp251xfd: Increase poll timeout
Link: https://lore.kernel.org/linux-can/BL3PR11MB64846C83ACD04E9330B0FE66FB729@BL3PR11MB6484.namprd11.prod.outlook.com/

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
In case you ask, here is the proof that can_frame_bits() folds into
a compile time constant expression.

This file:

  #include <linux/can/length.h>

  unsigned int canfd_max_len_bitsuffing(void)
  {
	return can_frame_bytes(true, true, true, CANFD_MAX_DLEN);
  }

Produces this assembly code:

  0000000000000010 <canfd_max_len_bitsuffing>:
    10:	f3 0f 1e fa          	endbr64
    14:	b8 5c 00 00 00       	mov    $0x5c,%eax
    19:	e9 00 00 00 00       	jmpq   1e <canfd_max_len_bitsuffing+0xe>

where 0x5c corresponds to the expected value of 92 bytes.
---
 drivers/net/can/dev/length.c |  15 +-
 include/linux/can/length.h   | 326 +++++++++++++++++++++++++----------
 2 files changed, 238 insertions(+), 103 deletions(-)

Comments

Vincent Mailhol May 23, 2023, 7:13 a.m. UTC | #1
Sorry for the late reply, I wanted to have this completed earlier but
other imperatives and the time needed to debug decided differently.

On Tue. 23 May 2023 at 15:52, Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
> Introduce a method to calculate the exact size in bits of a CAN(-FD)
> frame with or without dynamic bitsuffing.
>
> These are all the possible combinations taken into account:
>
>   - Classical CAN or CAN-FD
>   - Standard or Extended frame format
>   - CAN-FD CRC17 or CRC21
>   - Include or not intermission
>
> Instead of doing several macro definitions, declare the
> can_frame_bits() static inline function. To this extend, do a full
                                                   ^^^^^^
Typo: extent
(I will not send a v3 just for that).

> refactoring of the length definitions.
>
> If given integer constant expressions as argument, can_frame_bits()
> folds into a compile time constant expression, giving no penalty over
> the use of macros.
>
> Also add the can_frame_bytes(). This function replaces the existing
> macro:
>
>   - CAN_FRAME_OVERHEAD_SFF: can_frame_bytes(false, false, 0)
>   - CAN_FRAME_OVERHEAD_EFF: can_frame_bytes(false, true, 0)
>   - CANFD_FRAME_OVERHEAD_SFF: can_frame_bytes(true, false, 0)
>   - CANFD_FRAME_OVERHEAD_EFF: can_frame_bytes(true, true, 0)
>
> The different frame lengths (including intermission) are as follow:
>
>    Frame type                           bits    bytes
>   ----------------------------------------------------------
>    Classic CAN SFF no-bitstuffing       111     14
>    Classic CAN EFF no-bitstuffing       131     17
>    Classic CAN SFF bitstuffing          135     17
>    Classic CAN EFF bitstuffing          160     20
>    CAN-FD SFF no-bitstuffing            579     73
>    CAN-FD EFF no-bitstuffing            598     75
>    CAN-FD SFF bitstuffing               712     89
>    CAN-FD EFF bitstuffing               736     92
>
> The macro CAN_FRAME_LEN_MAX and CANFD_FRAME_LEN_MAX are kept to be
> used in const struct declarations (folding of can_frame_bytes() occurs
> too late in the compilation to be used in struct declarations).

To be fair, I am not fully happy with that part. The fact that
can_frame_bits() and can_frame_bytes() can not be used in structure
declaration even if these are compile time constants is unfortunate.
But after reflection, I still see those inline functions as a better
compromise than a collection of macro definitions. Let me know your
thoughts.

(...)

Yours sincerely,
Vincent Mailhol
Simon Horman May 23, 2023, 11:14 a.m. UTC | #2
On Tue, May 23, 2023 at 03:52:18PM +0900, Vincent Mailhol wrote:
> Introduce a method to calculate the exact size in bits of a CAN(-FD)
> frame with or without dynamic bitsuffing.

...

> +/**
> + * can_frame_bits() - Calculate the number of bits in on the wire in a
> + *	CAN frame

nit: @is_fd should be documented here.

> + * @is_eff: true: Extended Frame; false: Standard Frame.
> + * @bitstuffing: if true, calculate the bitsuffing worst case, if
> + *	false, calculated the bitsuffing best case (no dynamic
> + *	bitsuffing). Fixed stuff bits always get included.
> + * @intermission: if and only if true, include the inter frame space
> + *	assuming no bus idle (i.e. only the intermission gets added).
> + * @data_len: length of the data field in bytes. Correspond to
> + *	can(fd)_frame->len. Should be zero for remote frames. No
> + *	sanitization is done on @data_len.
> + *
> + * Return: the numbers of bits on the wire of a CAN frame.
> + */
> +static inline
> +unsigned int can_frame_bits(bool is_fd, bool is_eff,
> +			    bool bitstuffing, bool intermission,
> +			    unsigned int data_len)
> +{

...
diff mbox series

Patch

diff --git a/drivers/net/can/dev/length.c b/drivers/net/can/dev/length.c
index b48140b1102e..b7f4d76dd444 100644
--- a/drivers/net/can/dev/length.c
+++ b/drivers/net/can/dev/length.c
@@ -78,18 +78,7 @@  unsigned int can_skb_get_frame_len(const struct sk_buff *skb)
 	else
 		len = cf->len;
 
-	if (can_is_canfd_skb(skb)) {
-		if (cf->can_id & CAN_EFF_FLAG)
-			len += CANFD_FRAME_OVERHEAD_EFF;
-		else
-			len += CANFD_FRAME_OVERHEAD_SFF;
-	} else {
-		if (cf->can_id & CAN_EFF_FLAG)
-			len += CAN_FRAME_OVERHEAD_EFF;
-		else
-			len += CAN_FRAME_OVERHEAD_SFF;
-	}
-
-	return len;
+	return can_frame_bytes(can_is_canfd_skb(skb), cf->can_id & CAN_EFF_FLAG,
+			       false, len);
 }
 EXPORT_SYMBOL_GPL(can_skb_get_frame_len);
diff --git a/include/linux/can/length.h b/include/linux/can/length.h
index 521fdbce2d69..7518117ee5fc 100644
--- a/include/linux/can/length.h
+++ b/include/linux/can/length.h
@@ -1,132 +1,278 @@ 
 /* SPDX-License-Identifier: GPL-2.0 */
 /* Copyright (C) 2020 Oliver Hartkopp <socketcan@hartkopp.net>
  * Copyright (C) 2020 Marc Kleine-Budde <kernel@pengutronix.de>
- * Copyright (C) 2020 Vincent Mailhol <mailhol.vincent@wanadoo.fr>
+ * Copyright (C) 2020, 2023 Vincent Mailhol <mailhol.vincent@wanadoo.fr>
  */
 
 #ifndef _CAN_LENGTH_H
 #define _CAN_LENGTH_H
 
+#include <linux/bits.h>
 #include <linux/can.h>
 #include <linux/can/netlink.h>
+#include <linux/math.h>
 
 /*
- * Size of a Classical CAN Standard Frame
+ * Size of a Classical CAN Standard Frame header in bits
+ *
+ * Name of Field				Bits
+ * ---------------------------------------------------------
+ * Start Of Frame (SOF)				1
+ * Arbitration field:
+ *	base ID					11
+ *	Remote Transmission Request (RTR)	1
+ * Control field:
+ *	IDentifier Extension bit (IDE)		1
+ *	FD Format indicatior (FDF)		1
+ *	Data Length Code (DLC)			4
+ *
+ * including all fields preceding the data field, ignoring bitstuffing
+ */
+#define CAN_FRAME_HEADER_SFF_BITS 19
+
+/*
+ * Size of a Classical CAN Extended Frame header in bits
+ *
+ * Name of Field				Bits
+ * ---------------------------------------------------------
+ * Start Of Frame (SOF)				1
+ * Arbitration field:
+ *	base ID					11
+ *	Substitute Remote Request (SRR)		1
+ *	IDentifier Extension bit (IDE)		1
+ *	ID extension				18
+ *	Remote Transmission Request (RTR)	1
+ * Control field:
+ *	FD Format indicatior (FDF)		1
+ *	Reserved bit (r0)			1
+ *	Data length code (DLC)			4
+ *
+ * including all fields preceding the data field, ignoring bitstuffing
+ */
+#define CAN_FRAME_HEADER_EFF_BITS 39
+
+/*
+ * Size of a CAN-FD Standard Frame in bits
+ *
+ * Name of Field				Bits
+ * ---------------------------------------------------------
+ * Start Of Frame (SOF)				1
+ * Arbitration field:
+ *	base ID					11
+ *	Remote Request Substitution (RRS)	1
+ * Control field:
+ *	IDentifier Extension bit (IDE)		1
+ *	FD Format indicator (FDF)		1
+ *	Reserved bit (res)			1
+ *	Bit Rate Switch (BRS)			1
+ *	Error Status Indicator (ESI)		1
+ *	Data length code (DLC)			4
+ *
+ * including all fields preceding the data field, ignoring bitstuffing
+ */
+#define CANFD_FRAME_HEADER_SFF_BITS 22
+
+/*
+ * Size of a CAN-FD Extended Frame in bits
+ *
+ * Name of Field				Bits
+ * ---------------------------------------------------------
+ * Start Of Frame (SOF)				1
+ * Arbitration field:
+ *	base ID					11
+ *	Substitute Remote Request (SRR)		1
+ *	IDentifier Extension bit (IDE)		1
+ *	ID extension				18
+ *	Remote Request Substitution (RRS)	1
+ * Control field:
+ *	FD Format indicator (FDF)		1
+ *	Reserved bit (res)			1
+ *	Bit Rate Switch (BRS)			1
+ *	Error Status Indicator (ESI)		1
+ *	Data length code (DLC)			4
+ *
+ * including all fields preceding the data field, ignoring bitstuffing
+ */
+#define CANFD_FRAME_HEADER_EFF_BITS 41
+
+/*
+ * Size of a CAN CRC Field in bits
+ *
+ * Name of Field			Bits
+ * ---------------------------------------------------------
+ * CRC sequence (CRC15)			15
+ * CRC Delimiter			1
+ *
+ * ignoring bitstuffing
+ */
+#define CAN_FRAME_CRC_FIELD_BITS 16
+
+/*
+ * Size of a CAN-FD CRC17 Field in bits (length: 0..16)
+ *
+ * Name of Field			Bits
+ * ---------------------------------------------------------
+ * Stuff Count				4
+ * CRC Sequence (CRC17)			17
+ * CRC Delimiter			1
+ * Fixed stuff bits			6
+ */
+#define CANFD_FRAME_CRC17_FIELD_BITS 28
+
+/*
+ * Size of a CAN-FD CRC21 Field in bits (length: 20..64)
+ *
+ * Name of Field			Bits
+ * ---------------------------------------------------------
+ * Stuff Count				4
+ * CRC sequence (CRC21)			21
+ * CRC Delimiter			1
+ * Fixed stuff bits			7
+ */
+#define CANFD_FRAME_CRC21_FIELD_BITS 33
+
+/*
+ * Size of a CAN(-FD) Frame footer in bits
  *
  * Name of Field			Bits
  * ---------------------------------------------------------
- * Start-of-frame			1
- * Identifier				11
- * Remote transmission request (RTR)	1
- * Identifier extension bit (IDE)	1
- * Reserved bit (r0)			1
- * Data length code (DLC)		4
- * Data field				0...64
- * CRC					15
- * CRC delimiter			1
  * ACK slot				1
  * ACK delimiter			1
- * End-of-frame (EOF)			7
- * Inter frame spacing			3
+ * End Of Frame (EOF)			7
  *
- * rounded up and ignoring bitstuffing
+ * including all fields following the CRC field
  */
-#define CAN_FRAME_OVERHEAD_SFF DIV_ROUND_UP(47, 8)
+#define CAN_FRAME_FOOTER_BITS 9
 
 /*
- * Size of a Classical CAN Extended Frame
- *
- * Name of Field			Bits
- * ---------------------------------------------------------
- * Start-of-frame			1
- * Identifier A				11
- * Substitute remote request (SRR)	1
- * Identifier extension bit (IDE)	1
- * Identifier B				18
- * Remote transmission request (RTR)	1
- * Reserved bits (r1, r0)		2
- * Data length code (DLC)		4
- * Data field				0...64
- * CRC					15
- * CRC delimiter			1
- * ACK slot				1
- * ACK delimiter			1
- * End-of-frame (EOF)			7
- * Inter frame spacing			3
- *
- * rounded up and ignoring bitstuffing
+ * First part of the Inter Frame Space
+ * (a.k.a. IMF - intermission field)
  */
-#define CAN_FRAME_OVERHEAD_EFF DIV_ROUND_UP(67, 8)
+#define CAN_INTERMISSION_BITS 3
 
 /*
- * Size of a CAN-FD Standard Frame
+ * CAN bit stuffing overhead multiplication factor
  *
- * Name of Field			Bits
- * ---------------------------------------------------------
- * Start-of-frame			1
- * Identifier				11
- * Remote Request Substitution (RRS)	1
- * Identifier extension bit (IDE)	1
- * Flexible data rate format (FDF)	1
- * Reserved bit (r0)			1
- * Bit Rate Switch (BRS)		1
- * Error Status Indicator (ESI)		1
- * Data length code (DLC)		4
- * Data field				0...512
- * Stuff Bit Count (SBC)		4
- * CRC					0...16: 17 20...64:21
- * CRC delimiter (CD)			1
- * Fixed Stuff bits (FSB)		0...16: 6 20...64:7
- * ACK slot (AS)			1
- * ACK delimiter (AD)			1
- * End-of-frame (EOF)			7
- * Inter frame spacing			3
+ * The worst bit stuffing case is a sequence in which dominant and
+ * recessive bits alternate every four bits:
  *
- * assuming CRC21, rounded up and ignoring dynamic bitstuffing
+ *   Destuffed: 1 1111  0000  1111  0000  1111
+ *   Stuffed:   1 1111o 0000i 1111o 0000i 1111o
+ *
+ * Nomenclature:
+ *
+ *  - "0": dominant bit
+ *  - "o": dominant stuff bit
+ *  - "1": recessive bit
+ *  - "i": recessive stuff bit
+ *
+ * Ignoring the first bit, one stuff bit is added every four bits
+ * giving us an overhead of 1/4 = 0.25.
  */
-#define CANFD_FRAME_OVERHEAD_SFF DIV_ROUND_UP(67, 8)
+#define can_bit_stuffing_overhead(stream_len) mult_frac(stream_len, 1, 4)
+
+/**
+ * can_frame_bits() - Calculate the number of bits in on the wire in a
+ *	CAN frame
+ * @is_eff: true: Extended Frame; false: Standard Frame.
+ * @bitstuffing: if true, calculate the bitsuffing worst case, if
+ *	false, calculated the bitsuffing best case (no dynamic
+ *	bitsuffing). Fixed stuff bits always get included.
+ * @intermission: if and only if true, include the inter frame space
+ *	assuming no bus idle (i.e. only the intermission gets added).
+ * @data_len: length of the data field in bytes. Correspond to
+ *	can(fd)_frame->len. Should be zero for remote frames. No
+ *	sanitization is done on @data_len.
+ *
+ * Return: the numbers of bits on the wire of a CAN frame.
+ */
+static inline
+unsigned int can_frame_bits(bool is_fd, bool is_eff,
+			    bool bitstuffing, bool intermission,
+			    unsigned int data_len)
+{
+	unsigned int bits;
+
+	if (is_fd) {
+		if (is_eff)
+			bits = CANFD_FRAME_HEADER_EFF_BITS;
+		else
+			bits = CANFD_FRAME_HEADER_SFF_BITS;
+	} else {
+		if (is_eff)
+			bits = CAN_FRAME_HEADER_EFF_BITS;
+		else
+			bits = CAN_FRAME_HEADER_SFF_BITS;
+	}
+
+	bits += data_len * BITS_PER_BYTE;
+
+	if (!is_fd) {
+		bits += CAN_FRAME_CRC_FIELD_BITS;
+		/*
+		 * In Classical CAN, bit stuffing applies to all field
+		 * from SOF to CRC delimiter
+		 */
+		if (bitstuffing)
+			bits += can_bit_stuffing_overhead(bits);
+	} else {
+		/*
+		 * In CAN-FD, the fields preceding the CRC field have
+		 * dynamic bit stuffing; but the CRC field has fixed
+		 * bitstuffing
+		 */
+		if (bitstuffing)
+			bits += can_bit_stuffing_overhead(bits - 1);
+		if (data_len <= 16)
+			bits += CANFD_FRAME_CRC17_FIELD_BITS;
+		else
+			bits += CANFD_FRAME_CRC21_FIELD_BITS;
+	}
+
+	bits += CAN_FRAME_FOOTER_BITS;
+
+	if (intermission)
+		bits += CAN_INTERMISSION_BITS;
+
+	return bits;
+}
 
 /*
- * Size of a CAN-FD Extended Frame
- *
- * Name of Field			Bits
- * ---------------------------------------------------------
- * Start-of-frame			1
- * Identifier A				11
- * Substitute remote request (SRR)	1
- * Identifier extension bit (IDE)	1
- * Identifier B				18
- * Remote Request Substitution (RRS)	1
- * Flexible data rate format (FDF)	1
- * Reserved bit (r0)			1
- * Bit Rate Switch (BRS)		1
- * Error Status Indicator (ESI)		1
- * Data length code (DLC)		4
- * Data field				0...512
- * Stuff Bit Count (SBC)		4
- * CRC					0...16: 17 20...64:21
- * CRC delimiter (CD)			1
- * Fixed Stuff bits (FSB)		0...16: 6 20...64:7
- * ACK slot (AS)			1
- * ACK delimiter (AD)			1
- * End-of-frame (EOF)			7
- * Inter frame spacing			3
- *
- * assuming CRC21, rounded up and ignoring dynamic bitstuffing
+ * Number of bytes in a CAN frame
+ * (rounded up, including intermission)
  */
-#define CANFD_FRAME_OVERHEAD_EFF DIV_ROUND_UP(86, 8)
+static inline
+unsigned int can_frame_bytes(bool is_fd, bool is_eff, bool bitstuffing,
+			     unsigned int data_len)
+{
+	return DIV_ROUND_UP(can_frame_bits(is_fd, is_eff, bitstuffing, true,
+					   data_len),
+			    BITS_PER_BYTE);
+}
 
 /*
  * Maximum size of a Classical CAN frame
- * (rounded up and ignoring bitstuffing)
+ * (rounded up, ignoring bitstuffing but including intermission)
  */
-#define CAN_FRAME_LEN_MAX (CAN_FRAME_OVERHEAD_EFF + CAN_MAX_DLEN)
+#define CAN_FRAME_LEN_MAX				\
+	DIV_ROUND_UP(CAN_FRAME_HEADER_EFF_BITS +	\
+		     CAN_MAX_DLEN * BITS_PER_BYTE +	\
+		     CAN_FRAME_CRC_FIELD_BITS +		\
+		     CAN_FRAME_FOOTER_BITS +		\
+		     CAN_INTERMISSION_BITS,		\
+		     BITS_PER_BYTE)
 
 /*
  * Maximum size of a CAN-FD frame
  * (rounded up and ignoring bitstuffing)
  */
-#define CANFD_FRAME_LEN_MAX (CANFD_FRAME_OVERHEAD_EFF + CANFD_MAX_DLEN)
+#define CANFD_FRAME_LEN_MAX				\
+	DIV_ROUND_UP(CANFD_FRAME_HEADER_EFF_BITS +	\
+		     CANFD_MAX_DLEN * BITS_PER_BYTE +	\
+		     CANFD_FRAME_CRC21_FIELD_BITS	\
+		     CAN_FRAME_FOOTER_BITS +		\
+		     CAN_INTERMISSION_BITS,		\
+		     BITS_PER_BYTE)
 
 /*
  * can_cc_dlc2len(value) - convert a given data length code (dlc) of a