[PATCH RFC] can-dev: fix real payload length return value for RTR frames

[PATCH RFC] can-dev: fix real payload length return value for RTR frames

[Oliver Hartkopp]

The can_get_echo_skb() function returns the number of received bytes to
be used for netdev statistics. In the case of RTR frames we get a valid
(potential non-zero) data length value which has to be passed for further
operations. But on the wire RTR frames have no payload length. Therefore
the value to be used in the statistics has to be zero for RTR frames.

Reported-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/r/20201002154219.4887-4-mailhol.vincent@wanadoo.fr
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 drivers/net/can/dev.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index b70ded3760f2..64c8cb27154b 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -510,13 +510,17 @@ __can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr)
 		/* Using "struct canfd_frame::len" for the frame
 		 * length is supported on both CAN and CANFD frames.
 		 */
 		struct sk_buff *skb = priv->echo_skb[idx];
 		struct canfd_frame *cf = (struct canfd_frame *)skb->data;
-		u8 len = cf->len;
 
-		*len_ptr = len;
+		/* get the real payload length for netdev statistics */
+		if (cf->can_id & CAN_RTR_FLAG)
+			*len_ptr = 0;
+		else
+			*len_ptr = cf->len;
+
 		priv->echo_skb[idx] = NULL;
 
 		return skb;
 	}
 
-- 
2.28.0

Re: [PATCH RFC] can-dev: fix real payload length return value for RTR frames

[Marc Kleine-Budde]

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

On 10/20/20 8:44 AM, Oliver Hartkopp wrote:
> The can_get_echo_skb() function returns the number of received bytes to
> be used for netdev statistics. In the case of RTR frames we get a valid
> (potential non-zero) data length value which has to be passed for further
> operations. But on the wire RTR frames have no payload length. Therefore
> the value to be used in the statistics has to be zero for RTR frames.
> 
> Reported-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Link: https://lore.kernel.org/r/20201002154219.4887-4-mailhol.vincent@wanadoo.fr
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
>  drivers/net/can/dev.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index b70ded3760f2..64c8cb27154b 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -510,13 +510,17 @@ __can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr)
>  		/* Using "struct canfd_frame::len" for the frame
>  		 * length is supported on both CAN and CANFD frames.
>  		 */
>  		struct sk_buff *skb = priv->echo_skb[idx];
>  		struct canfd_frame *cf = (struct canfd_frame *)skb->data;
> -		u8 len = cf->len;
>  
> -		*len_ptr = len;
> +		/* get the real payload length for netdev statistics */
> +		if (cf->can_id & CAN_RTR_FLAG)
> +			*len_ptr = 0;
> +		else
> +			*len_ptr = cf->len;
> +
>  		priv->echo_skb[idx] = NULL;
>  
>  		return skb;
>  	}

Looks good. I've replaced patches 4 and 5 by this one.

tnx,
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 --]