On 19.10.20 21:05, Marc Kleine-Budde wrote:
From: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
In classical CAN, the length of the data (i.e. CAN payload) is not
always equal to the DLC! If the frame is a Remote Transmission Request
(RTR), data length is always zero regardless of DLC value and else, if
the DLC is greater than 8, the length is 8. Contrary to common belief,
ISO 11898-1 Chapter 8.4.2.3 (DLC field) do allow DLCs greater than 8
for Classical Frames and specifies that those DLCs shall indicate that
the data field is 8 bytes long.
Above facts are widely unknown and so many developpers uses the "len"
field of "struct canfd_frame" to get the length of classical CAN
frames: this is incorrect!
This patch introduces function get_can_len() which can be used in
remediation. The function takes the SKB as an input in order to be
able to determine if the frame is classical or FD.
Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20201002154219.4887-4-mailhol.vincent@xxxxxxxxxx
[mkl: renamed get_can_len() -> can_get_len()]
Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
---
include/linux/can/dev.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 41ff31795320..2bb132fc6d88 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -192,6 +192,29 @@ u8 can_dlc2len(u8 can_dlc);
/* map the sanitized data length to an appropriate data length code */
u8 can_len2dlc(u8 len);
+/*
+ * can_get_len(skb) - get the length of the CAN payload.
+ *
+ * In classical CAN, the length of the data (i.e. CAN payload) is not
+ * always equal to the DLC! If the frame is a Remote Transmission
+ * Request (RTR), data length is always zero regardless of DLC value
+ * and else, if the DLC is greater than 8, the length is 8. Contrary
+ * to common belief, ISO 11898-1 Chapter 8.4.2.3 (DLC field) do allow
+ * DLCs greater than 8 for Classical Frames and specifies that those
+ * DLCs shall indicate that the data field is 8 bytes long.
+ */
+static inline u8 can_get_len(const struct sk_buff *skb)
+{
+ const struct canfd_frame *cf = (const struct canfd_frame *)skb->data;
+
+ if (can_is_canfd_skb(skb))
+ return min_t(u8, cf->len, CANFD_MAX_DLEN);
+ else if (cf->can_id & CAN_RTR_FLAG)
+ return 0;
+ else
+ return min_t(u8, cf->len, CAN_MAX_DLEN);
+}
The main idea behind this patch and patch 05/16 is to provide a correct
statistic in the tx bytes, right?
A simple test for the CAN_RTR_FLAG will do the job as all the length
sanitizing is already done in the tx path by can_dropped_invalid_skb()
in all known drivers right *before* the skb is stored in the echo skb array.
IMO there's no need for a separate helper function. Maybe a macro which
should have something with 'payload' in its name - to determine the tx
byte statistics based on CAN_RTR_FLAG ...
Regards,
Oliver
+
struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
unsigned int txqs, unsigned int rxqs);
#define alloc_candev(sizeof_priv, echo_skb_max) \
