linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Stefan Mätje" <Stefan.Maetje@esd.eu>
To: "linux-can@vger.kernel.org" <linux-can@vger.kernel.org>,
	"mailhol.vincent@wanadoo.fr" <mailhol.vincent@wanadoo.fr>,
	"mkl@pengutronix.de" <mkl@pengutronix.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	"appana.durga.rao@xilinx.com" <appana.durga.rao@xilinx.com>,
	"michal.simek@xilinx.com" <michal.simek@xilinx.com>,
	"mripard@kernel.org" <mripard@kernel.org>,
	"wens@csie.org" <wens@csie.org>,
	"jernej.skrabec@gmail.com" <jernej.skrabec@gmail.com>,
	"yashi@spacecubics.com" <yashi@spacecubics.com>,
	"rcsekar@samsung.com" <rcsekar@samsung.com>,
	"naga.sureshkumar.relli@xilinx.com" 
	<naga.sureshkumar.relli@xilinx.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-sunxi@lists.linux.dev" <linux-sunxi@lists.linux.dev>,
	"s.grosjean@peak-system.com" <s.grosjean@peak-system.com>,
	"ludovic.desroches@microchip.com"
	<ludovic.desroches@microchip.com>,
	"extja@kvaser.com" <extja@kvaser.com>,
	"nicolas.ferre@microchip.com" <nicolas.ferre@microchip.com>
Subject: Re: [PATCH v1 2/2] can: do not increase rx_bytes statistics for RTR frames
Date: Wed, 24 Nov 2021 17:59:14 +0000	[thread overview]
Message-ID: <e2d90af1cf41bb9893225289822f93f036d415bb.camel@esd.eu> (raw)
In-Reply-To: <20211123115333.624335-3-mailhol.vincent@wanadoo.fr>

Hi Vincent,

I would like to suggest a slightly different patch for the esd_usb2.c (as
added below).

Best regards,
    Stefan Mätje

Am Dienstag, den 23.11.2021, 20:53 +0900 schrieb Vincent Mailhol:
> The actual payload length of the CAN Remote Transmission Request (RTR)
> frames is always 0, i.e. nothing is transmitted on the wire. However,
> those RTR frames still uses the DLC to indicate the length of the
> requested frame.
> 
> As such, net_device_stats:rx_bytes should not be increased for the RTR
> frames.
> 
> This patch fixes all the CAN drivers.
> 
> CC: Jimmy Assarsson <extja@kvaser.com>
> CC: Marc Kleine-Budde <mkl@pengutronix.de>
> CC: Nicolas Ferre <nicolas.ferre@microchip.com>
> CC: Alexandre Belloni <alexandre.belloni@bootlin.com>
> CC: Ludovic Desroches <ludovic.desroches@microchip.com>
> CC: Chandrasekar Ramakrishnan <rcsekar@samsung.com>
> CC: Maxime Ripard <mripard@kernel.org>
> CC: Chen-Yu Tsai <wens@csie.org>
> CC: Jernej Skrabec <jernej.skrabec@gmail.com>
> CC: Yasushi SHOJI <yashi@spacecubics.com>
> CC: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
> CC: Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com>
> CC: Michal Simek <michal.simek@xilinx.com>
> CC: Stephane Grosjean <s.grosjean@peak-system.com>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
>  drivers/net/can/at91_can.c                        | 3 ++-
>  drivers/net/can/c_can/c_can_main.c                | 3 ++-
>  drivers/net/can/cc770/cc770.c                     | 3 ++-
>  drivers/net/can/dev/rx-offload.c                  | 3 ++-
>  drivers/net/can/grcan.c                           | 3 ++-
>  drivers/net/can/ifi_canfd/ifi_canfd.c             | 3 ++-
>  drivers/net/can/janz-ican3.c                      | 3 ++-
>  drivers/net/can/kvaser_pciefd.c                   | 3 ++-
>  drivers/net/can/m_can/m_can.c                     | 3 ++-
>  drivers/net/can/mscan/mscan.c                     | 3 ++-
>  drivers/net/can/pch_can.c                         | 3 ++-
>  drivers/net/can/peak_canfd/peak_canfd.c           | 3 ++-
>  drivers/net/can/rcar/rcar_can.c                   | 3 ++-
>  drivers/net/can/rcar/rcar_canfd.c                 | 3 ++-
>  drivers/net/can/sja1000/sja1000.c                 | 3 ++-
>  drivers/net/can/slcan.c                           | 3 ++-
>  drivers/net/can/spi/hi311x.c                      | 3 ++-
>  drivers/net/can/spi/mcp251x.c                     | 3 ++-
>  drivers/net/can/sun4i_can.c                       | 3 ++-
>  drivers/net/can/usb/ems_usb.c                     | 3 ++-
>  drivers/net/can/usb/esd_usb2.c                    | 3 ++-
>  drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 6 ++++--
>  drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c  | 3 ++-
>  drivers/net/can/usb/mcba_usb.c                    | 3 ++-
>  drivers/net/can/usb/peak_usb/pcan_usb.c           | 3 ++-
>  drivers/net/can/usb/peak_usb/pcan_usb_fd.c        | 8 ++++----
>  drivers/net/can/usb/peak_usb/pcan_usb_pro.c       | 9 +++++----
>  drivers/net/can/usb/ucan.c                        | 3 ++-
>  drivers/net/can/usb/usb_8dev.c                    | 8 ++++----
>  drivers/net/can/xilinx_can.c                      | 8 +++++---
>  30 files changed, 72 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index 3cd872cf9be6..89b22c3f9a01 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -617,7 +617,8 @@ static void at91_read_msg(struct net_device *dev, unsigned int mb)
>  	at91_read_mb(dev, mb, cf);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_receive_skb(skb);
>  
>  	can_led_event(dev, CAN_LED_EVENT_RX);
> diff --git a/drivers/net/can/c_can/c_can_main.c b/drivers/net/can/c_can/c_can_main.c
> index 670754a12984..f2eb9ebc875f 100644
> --- a/drivers/net/can/c_can/c_can_main.c
> +++ b/drivers/net/can/c_can/c_can_main.c
> @@ -406,7 +406,8 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, u32 ctrl)
>  	}
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += frame->len;
> +	if (!(frame->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += frame->len;
>  
>  	netif_receive_skb(skb);
>  	return 0;
> diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
> index a5fd8ccedec2..7c703bcbb433 100644
> --- a/drivers/net/can/cc770/cc770.c
> +++ b/drivers/net/can/cc770/cc770.c
> @@ -492,7 +492,8 @@ static void cc770_rx(struct net_device *dev, unsigned int mo, u8 ctrl1)
>  	}
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_rx(skb);
>  }
>  
> diff --git a/drivers/net/can/dev/rx-offload.c b/drivers/net/can/dev/rx-offload.c
> index bb47e9a49240..b10b73e5ed87 100644
> --- a/drivers/net/can/dev/rx-offload.c
> +++ b/drivers/net/can/dev/rx-offload.c
> @@ -56,7 +56,8 @@ static int can_rx_offload_napi_poll(struct napi_struct *napi, int quota)
>  		work_done++;
>  		if (!(cf->can_id & CAN_ERR_MASK)) {
>  			stats->rx_packets++;
> -			stats->rx_bytes += cf->len;
> +			if (!(cf->can_id & CAN_RTR_FLAG))
> +				stats->rx_bytes += cf->len;
>  		}
>  		netif_receive_skb(skb);
>  	}
> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 78e27940b2af..2f8a08321b41 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -1215,7 +1215,8 @@ static int grcan_receive(struct net_device *dev, int budget)
>  
>  		/* Update statistics and read pointer */
>  		stats->rx_packets++;
> -		stats->rx_bytes += cf->len;
> +		if (!(cf->can_id & CAN_RTR_FLAG))
> +			stats->rx_bytes += cf->len;
>  		netif_receive_skb(skb);
>  
>  		rd = grcan_ring_add(rd, GRCAN_MSG_SIZE, dma->rx.size);
> diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
> index e8318e984bf2..1f90c829321e 100644
> --- a/drivers/net/can/ifi_canfd/ifi_canfd.c
> +++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
> @@ -316,7 +316,8 @@ static void ifi_canfd_read_fifo(struct net_device *ndev)
>  	writel(rx_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  
>  	netif_receive_skb(skb);
>  }
> diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
> index 32006dbf5abd..5c589aa9dff8 100644
> --- a/drivers/net/can/janz-ican3.c
> +++ b/drivers/net/can/janz-ican3.c
> @@ -1421,7 +1421,8 @@ static int ican3_recv_skb(struct ican3_dev *mod)
>  
>  	/* update statistics, receive the skb */
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_receive_skb(skb);
>  
>  err_noalloc:
> diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
> index 483fbd9e6952..88a570476500 100644
> --- a/drivers/net/can/kvaser_pciefd.c
> +++ b/drivers/net/can/kvaser_pciefd.c
> @@ -1193,7 +1193,8 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
>  		ns_to_ktime(div_u64(p->timestamp * 1000,
>  				    pcie->freq_to_ticks_div));
>  
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	stats->rx_packets++;
>  
>  	return netif_rx(skb);
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index c33035e706bc..eedaf66bff31 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -524,7 +524,8 @@ static int m_can_read_fifo(struct net_device *dev, u32 rxfs)
>  	m_can_write(cdev, M_CAN_RXF0A, fgi);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  
>  	timestamp = FIELD_GET(RX_BUF_RXTS_MASK, fifo_header.dlc);
>  
> diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
> index 9e1cce0260da..59b8284d00e5 100644
> --- a/drivers/net/can/mscan/mscan.c
> +++ b/drivers/net/can/mscan/mscan.c
> @@ -404,7 +404,8 @@ static int mscan_rx_poll(struct napi_struct *napi, int quota)
>  		if (canrflg & MSCAN_RXF) {
>  			mscan_get_rx_frame(dev, frame);
>  			stats->rx_packets++;
> -			stats->rx_bytes += frame->len;
> +			if (!(frame->can_id & CAN_RTR_FLAG))
> +				stats->rx_bytes += frame->len;
>  		} else if (canrflg & MSCAN_ERR_IF) {
>  			mscan_get_err_frame(dev, frame, canrflg);
>  		}
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 6b45840db1f9..8fbe3560216b 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -692,7 +692,8 @@ static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int quota)
>  		rcv_pkts++;
>  		stats->rx_packets++;
>  		quota--;
> -		stats->rx_bytes += cf->len;
> +		if (!(cf->can_id & CAN_RTR_FLAG))
> +			stats->rx_bytes += cf->len;
>  		netif_receive_skb(skb);
>  
>  		pch_fifo_thresh(priv, obj_num);
> diff --git a/drivers/net/can/peak_canfd/peak_canfd.c b/drivers/net/can/peak_canfd/peak_canfd.c
> index d5b8bc6d2980..baae39fa14fc 100644
> --- a/drivers/net/can/peak_canfd/peak_canfd.c
> +++ b/drivers/net/can/peak_canfd/peak_canfd.c
> @@ -315,7 +315,8 @@ static int pucan_handle_can_rx(struct peak_canfd_priv *priv,
>  	else
>  		memcpy(cf->data, msg->d, cf->len);
>  
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	stats->rx_packets++;
>  
>  	pucan_netif_rx(skb, msg->ts_low, msg->ts_high);
> diff --git a/drivers/net/can/rcar/rcar_can.c b/drivers/net/can/rcar/rcar_can.c
> index f408ed9a6ccd..237a97cd2f78 100644
> --- a/drivers/net/can/rcar/rcar_can.c
> +++ b/drivers/net/can/rcar/rcar_can.c
> @@ -666,7 +666,8 @@ static void rcar_can_rx_pkt(struct rcar_can_priv *priv)
>  
>  	can_led_event(priv->ndev, CAN_LED_EVENT_RX);
>  
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	stats->rx_packets++;
>  	netif_receive_skb(skb);
>  }
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index db9d62874e15..b1eded2f2c5d 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1550,7 +1550,8 @@ static void rcar_canfd_rx_pkt(struct rcar_canfd_channel *priv)
>  
>  	can_led_event(priv->ndev, CAN_LED_EVENT_RX);
>  
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	stats->rx_packets++;
>  	netif_receive_skb(skb);
>  }
> diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
> index a65546ca9461..d2e444968087 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -380,7 +380,8 @@ static void sja1000_rx(struct net_device *dev)
>  	sja1000_write_cmdreg(priv, CMD_RRB);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_rx(skb);
>  
>  	can_led_event(dev, CAN_LED_EVENT_RX);
> diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
> index 9a4ebda30510..c53ad59763e0 100644
> --- a/drivers/net/can/slcan.c
> +++ b/drivers/net/can/slcan.c
> @@ -218,7 +218,8 @@ static void slc_bump(struct slcan *sl)
>  	skb_put_data(skb, &cf, sizeof(struct can_frame));
>  
>  	sl->dev->stats.rx_packets++;
> -	sl->dev->stats.rx_bytes += cf.len;
> +	if (!(cf.can_id & CAN_RTR_FLAG))
> +		sl->dev->stats.rx_bytes += cf.len;
>  	netif_rx_ni(skb);
>  }
>  
> diff --git a/drivers/net/can/spi/hi311x.c b/drivers/net/can/spi/hi311x.c
> index 89d9c986a229..1e2929a9521b 100644
> --- a/drivers/net/can/spi/hi311x.c
> +++ b/drivers/net/can/spi/hi311x.c
> @@ -350,7 +350,8 @@ static void hi3110_hw_rx(struct spi_device *spi)
>  		       frame->len);
>  
>  	priv->net->stats.rx_packets++;
> -	priv->net->stats.rx_bytes += frame->len;
> +	if (!(frame->can_id & CAN_RTR_FLAG))
> +		priv->net->stats.rx_bytes += frame->len;
>  
>  	can_led_event(priv->net, CAN_LED_EVENT_RX);
>  
> diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
> index 0579ab74f728..1f47c588b69f 100644
> --- a/drivers/net/can/spi/mcp251x.c
> +++ b/drivers/net/can/spi/mcp251x.c
> @@ -736,7 +736,8 @@ static void mcp251x_hw_rx(struct spi_device *spi, int buf_idx)
>  	memcpy(frame->data, buf + RXBDAT_OFF, frame->len);
>  
>  	priv->net->stats.rx_packets++;
> -	priv->net->stats.rx_bytes += frame->len;
> +	if (!(frame->can_id & CAN_RTR_FLAG))
> +		priv->net->stats.rx_bytes += frame->len;
>  
>  	can_led_event(priv->net, CAN_LED_EVENT_RX);
>  
> diff --git a/drivers/net/can/sun4i_can.c b/drivers/net/can/sun4i_can.c
> index 599174098883..cd3278e5d4f6 100644
> --- a/drivers/net/can/sun4i_can.c
> +++ b/drivers/net/can/sun4i_can.c
> @@ -501,7 +501,8 @@ static void sun4i_can_rx(struct net_device *dev)
>  	sun4i_can_write_cmdreg(priv, SUN4I_CMD_RELEASE_RBUF);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_rx(skb);
>  
>  	can_led_event(dev, CAN_LED_EVENT_RX);
> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
> index 7cf65936d02e..f4288f98c006 100644
> --- a/drivers/net/can/usb/ems_usb.c
> +++ b/drivers/net/can/usb/ems_usb.c
> @@ -323,7 +323,8 @@ static void ems_usb_rx_can_msg(struct ems_usb *dev, struct ems_cpc_msg *msg)
>  	}
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_rx(skb);
>  }
>  
> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
> index 5f6915a27b3d..ac65ddfe814d 100644
> --- a/drivers/net/can/usb/esd_usb2.c
> +++ b/drivers/net/can/usb/esd_usb2.c
> @@ -335,7 +335,8 @@ static void esd_usb2_rx_can_msg(struct esd_usb2_net_priv *priv,
>  		}
>  
>  		stats->rx_packets++;
> -		stats->rx_bytes += cf->len;
> +		if (!(cf->can_id & CAN_RTR_FLAG))
> +			stats->rx_bytes += cf->len;
>  		netif_rx(skb);
>  	}
> 

The version below would save us adding another if() statement to check for RTR or
normal frame that is already tested in the if() statement directly before.

diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index c6068a251fbe..2f91a18fe592 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -332,14 +332,14 @@ static void esd_usb2_rx_can_msg(struct esd_usb2_net_priv *priv,
                if (msg->msg.rx.dlc & ESD_RTR) {
                        cf->can_id |= CAN_RTR_FLAG;
                } else {
                        for (i = 0; i < cf->len; i++)
                                cf->data[i] = msg->msg.rx.data[i];
+                       stats->rx_bytes += cf->len;
                }
-
                stats->rx_packets++;
-               stats->rx_bytes += cf->len;
+
                netif_rx(skb);
        }
 
        return;
 }

>  
> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> index 3398da323126..adab5c2a8b99 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
> @@ -1206,7 +1206,8 @@ static void kvaser_usb_hydra_rx_msg_std(const struct kvaser_usb *dev,
>  		memcpy(cf->data, cmd->rx_can.data, cf->len);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_rx(skb);
>  }
>  
> @@ -1284,7 +1285,8 @@ static void kvaser_usb_hydra_rx_msg_ext(const struct kvaser_usb *dev,
>  		memcpy(cf->data, cmd->rx_can.kcan_payload, cf->len);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_rx(skb);
>  }
>  
> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> index 4aebaab9ea9c..14b445643554 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> @@ -1001,7 +1001,8 @@ static void kvaser_usb_leaf_rx_can_msg(const struct kvaser_usb *dev,
>  	}
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	netif_rx(skb);
>  }
>  
> diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
> index a1a154c08b7f..372f0a5ebb9f 100644
> --- a/drivers/net/can/usb/mcba_usb.c
> +++ b/drivers/net/can/usb/mcba_usb.c
> @@ -458,7 +458,8 @@ static void mcba_usb_process_can(struct mcba_priv *priv,
>  	memcpy(cf->data, msg->data, cf->len);
>  
>  	stats->rx_packets++;
> -	stats->rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  
>  	can_led_event(priv->netdev, CAN_LED_EVENT_RX);
>  	netif_rx(skb);
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
> index 21b06a738595..d1a4f847f310 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
> @@ -684,7 +684,8 @@ static int pcan_usb_decode_data(struct pcan_usb_msg_context *mc, u8 status_len)
>  
>  	/* update statistics */
>  	mc->netdev->stats.rx_packets++;
> -	mc->netdev->stats.rx_bytes += cf->len;
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		mc->netdev->stats.rx_bytes += cf->len;
>  	/* push the skb */
>  	netif_rx(skb);
>  
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> index 185f5a98d217..65487ec33566 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> @@ -507,13 +507,13 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
>  	if (rx_msg_flags & PUCAN_MSG_EXT_ID)
>  		cfd->can_id |= CAN_EFF_FLAG;
>  
> -	if (rx_msg_flags & PUCAN_MSG_RTR)
> +	if (rx_msg_flags & PUCAN_MSG_RTR) {
>  		cfd->can_id |= CAN_RTR_FLAG;
> -	else
> +	} else {
>  		memcpy(cfd->data, rm->d, cfd->len);
> -
> +		netdev->stats.rx_bytes += cfd->len;
> +	}
>  	netdev->stats.rx_packets++;
> -	netdev->stats.rx_bytes += cfd->len;
>  
>  	peak_usb_netif_rx_64(skb, le32_to_cpu(rm->ts_low),
>  			     le32_to_cpu(rm->ts_high));
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> index f6d19879bf40..69fdddcbba98 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> @@ -536,17 +536,18 @@ static int pcan_usb_pro_handle_canmsg(struct pcan_usb_pro_interface *usb_if,
>  	if (rx->flags & PCAN_USBPRO_EXT)
>  		can_frame->can_id |= CAN_EFF_FLAG;
>  
> -	if (rx->flags & PCAN_USBPRO_RTR)
> +	if (rx->flags & PCAN_USBPRO_RTR) {
>  		can_frame->can_id |= CAN_RTR_FLAG;
> -	else
> +	} else {
>  		memcpy(can_frame->data, rx->data, can_frame->len);
> +		netdev->stats.rx_bytes += can_frame->len;
> +	}
> +	netdev->stats.rx_packets++;
>  
>  	hwts = skb_hwtstamps(skb);
>  	peak_usb_get_ts_time(&usb_if->time_ref, le32_to_cpu(rx->ts32),
>  			     &hwts->hwtstamp);
>  
> -	netdev->stats.rx_packets++;
> -	netdev->stats.rx_bytes += can_frame->len;
>  	netif_rx(skb);
>  
>  	return 0;
> diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
> index d582c39fc8d0..388899019955 100644
> --- a/drivers/net/can/usb/ucan.c
> +++ b/drivers/net/can/usb/ucan.c
> @@ -623,7 +623,8 @@ static void ucan_rx_can_msg(struct ucan_priv *up, struct ucan_message_in *m)
>  	/* don't count error frames as real packets */
>  	if (!(cf->can_id & CAN_ERR_FLAG)) {
>  		stats->rx_packets++;
> -		stats->rx_bytes += cf->len;
> +		if (!(cf->can_id & CAN_RTR_FLAG))
> +			stats->rx_bytes += cf->len;
>  	}
>  
>  	/* pass it to Linux */
> diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c
> index 040324362b26..14ae8ed85f80 100644
> --- a/drivers/net/can/usb/usb_8dev.c
> +++ b/drivers/net/can/usb/usb_8dev.c
> @@ -474,13 +474,13 @@ static void usb_8dev_rx_can_msg(struct usb_8dev_priv *priv,
>  		if (msg->flags & USB_8DEV_EXTID)
>  			cf->can_id |= CAN_EFF_FLAG;
>  
> -		if (msg->flags & USB_8DEV_RTR)
> +		if (msg->flags & USB_8DEV_RTR) {
>  			cf->can_id |= CAN_RTR_FLAG;
> -		else
> +		} else {
>  			memcpy(cf->data, msg->data, cf->len);
> -
> +			stats->rx_bytes += cf->len;
> +		}
>  		stats->rx_packets++;
> -		stats->rx_bytes += cf->len;
>  		netif_rx(skb);
>  
>  		can_led_event(priv->netdev, CAN_LED_EVENT_RX);
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 275e240ab293..7d7239ec9920 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -787,9 +787,9 @@ static int xcan_rx(struct net_device *ndev, int frame_base)
>  			*(__be32 *)(cf->data) = cpu_to_be32(data[0]);
>  		if (cf->len > 4)
>  			*(__be32 *)(cf->data + 4) = cpu_to_be32(data[1]);
> -	}
>  
> -	stats->rx_bytes += cf->len;
> +		stats->rx_bytes += cf->len;
> +	}
>  	stats->rx_packets++;
>  	netif_receive_skb(skb);
>  
> @@ -871,7 +871,9 @@ static int xcanfd_rx(struct net_device *ndev, int frame_base)
>  			*(__be32 *)(cf->data + i) = cpu_to_be32(data[0]);
>  		}
>  	}
> -	stats->rx_bytes += cf->len;
> +
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->rx_bytes += cf->len;
>  	stats->rx_packets++;
>  	netif_receive_skb(skb);
>  

  parent reply	other threads:[~2021-11-24 17:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-23 11:53 [PATCH v1 0/2] fix statistics for CAN RTR and Error frames Vincent Mailhol
2021-11-23 11:53 ` [PATCH v1 1/2] can: do not increase rx statistics when receiving CAN error frames Vincent Mailhol
2021-11-23 21:01   ` Oliver Hartkopp
2021-11-23 23:21     ` Vincent MAILHOL
2021-11-23 11:53 ` [PATCH v1 2/2] can: do not increase rx_bytes statistics for RTR frames Vincent Mailhol
2021-11-23 18:10   ` Vincent MAILHOL
2021-11-24 17:59   ` Stefan Mätje [this message]
2021-11-24 23:55     ` Vincent MAILHOL
2021-11-23 21:10 ` [PATCH v1 0/2] fix statistics for CAN RTR and Error frames Oliver Hartkopp
2021-11-23 23:35   ` Vincent MAILHOL

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e2d90af1cf41bb9893225289822f93f036d415bb.camel@esd.eu \
    --to=stefan.maetje@esd.eu \
    --cc=alexandre.belloni@bootlin.com \
    --cc=appana.durga.rao@xilinx.com \
    --cc=extja@kvaser.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=ludovic.desroches@microchip.com \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=michal.simek@xilinx.com \
    --cc=mkl@pengutronix.de \
    --cc=mripard@kernel.org \
    --cc=naga.sureshkumar.relli@xilinx.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=rcsekar@samsung.com \
    --cc=s.grosjean@peak-system.com \
    --cc=wens@csie.org \
    --cc=yashi@spacecubics.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).