From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
To: "Stefan Mätje" <Stefan.Maetje@esd.eu>
Cc: "linux-can@vger.kernel.org" <linux-can@vger.kernel.org>,
"mkl@pengutronix.de" <mkl@pengutronix.de>,
"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: Thu, 25 Nov 2021 08:55:33 +0900 [thread overview]
Message-ID: <CAMZ6RqKKe=hooYH7xSGyJC2n+MGG8vSOZtZVfbT7-QQ_Ax=Hww@mail.gmail.com> (raw)
In-Reply-To: <e2d90af1cf41bb9893225289822f93f036d415bb.camel@esd.eu>
Hi Stefan,
On Thu. 25 Nov 2021 at 02:59, Stefan Mätje <Stefan.Maetje@esd.eu> 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:
> > 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/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
next prev parent reply other threads:[~2021-11-24 23:55 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
2021-11-24 23:55 ` Vincent MAILHOL [this message]
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='CAMZ6RqKKe=hooYH7xSGyJC2n+MGG8vSOZtZVfbT7-QQ_Ax=Hww@mail.gmail.com' \
--to=mailhol.vincent@wanadoo.fr \
--cc=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=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).