Re: [PATCH v2 2/6] can: dev: add a helper function to get the correct length of Classical frames

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2868 bytes --]

On 9/30/20 4:45 PM, Vincent Mailhol wrote:
> 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@wanadoo.fr>
> ---
>  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 5e3d45525bd3..72a8a60c0094 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -177,6 +177,29 @@ u8 can_dlc2len(u8 can_dlc);
>  /* map the sanitized data length to an appropriate data length code */
>  u8 can_len2dlc(u8 len);
>  
> +/*
> + * get_can_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 int get_can_len(struct sk_buff *skb)

make this return an u8
make the skb const

> +{
> +	struct canfd_frame *cf = (struct canfd_frame *)skb->data;

const

> +
> +	if (can_is_canfd_skb(skb))
> +		return min_t(__u8, cf->len, CANFD_MAX_DLEN);

u8

> +	else if (cf->can_id & CAN_RTR_FLAG)
> +		return 0;
> +	else
> +		return min_t(__u8, cf->len, CAN_MAX_DLEN);

u8

> +}
> +
>  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) \
> 

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]