Re: [PATCH v1 2/2] can: do not increase rx_bytes statistics for RTR frames
From: Vincent MAILHOL <hidden>
Date: 2021-11-24 23:55:48
Also in:
linux-arm-kernel, linux-can, linux-sunxi, lkml
Hi Stefan, On Thu. 25 Nov 2021 at 02:59, Stefan Mätje [off-list ref] wrote:
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:quoted
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 <redacted> 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 <redacted> CC: Maxime Ripard <mripard@kernel.org> CC: Chen-Yu Tsai <redacted> 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 <redacted> CC: Michal Simek <redacted> CC: Stephane Grosjean <redacted> Signed-off-by: Vincent Mailhol <redacted> --- 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(-)
...
quoted hunk ↗ jump to hunk
quoted
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; }
Work for me! I will add this in the v2 (under preparation). Yours sincerely, Vincent Mailhol