| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 25 Nov 2021 08:55:33 +0900
From: Vincent MAILHOL <mailhol.vincent@...adoo.fr>
To: Stefan Mätje <Stefan.Maetje@....eu>
Cc: "linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
"mkl@...gutronix.de" <mkl@...gutronix.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
"appana.durga.rao@...inx.com" <appana.durga.rao@...inx.com>,
"michal.simek@...inx.com" <michal.simek@...inx.com>,
"mripard@...nel.org" <mripard@...nel.org>,
"wens@...e.org" <wens@...e.org>,
"jernej.skrabec@...il.com" <jernej.skrabec@...il.com>,
"yashi@...cecubics.com" <yashi@...cecubics.com>,
"rcsekar@...sung.com" <rcsekar@...sung.com>,
"naga.sureshkumar.relli@...inx.com"
<naga.sureshkumar.relli@...inx.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-sunxi@...ts.linux.dev" <linux-sunxi@...ts.linux.dev>,
"s.grosjean@...k-system.com" <s.grosjean@...k-system.com>,
"ludovic.desroches@...rochip.com" <ludovic.desroches@...rochip.com>,
"extja@...ser.com" <extja@...ser.com>,
"nicolas.ferre@...rochip.com" <nicolas.ferre@...rochip.com>
Subject: Re: [PATCH v1 2/2] can: do not increase rx_bytes statistics for RTR frames
Hi Stefan,
On Thu. 25 Nov 2021 at 02:59, Stefan Mätje <Stefan.Maetje@....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@...ser.com>
> > CC: Marc Kleine-Budde <mkl@...gutronix.de>
> > CC: Nicolas Ferre <nicolas.ferre@...rochip.com>
> > CC: Alexandre Belloni <alexandre.belloni@...tlin.com>
> > CC: Ludovic Desroches <ludovic.desroches@...rochip.com>
> > CC: Chandrasekar Ramakrishnan <rcsekar@...sung.com>
> > CC: Maxime Ripard <mripard@...nel.org>
> > CC: Chen-Yu Tsai <wens@...e.org>
> > CC: Jernej Skrabec <jernej.skrabec@...il.com>
> > CC: Yasushi SHOJI <yashi@...cecubics.com>
> > CC: Appana Durga Kedareswara rao <appana.durga.rao@...inx.com>
> > CC: Naga Sureshkumar Relli <naga.sureshkumar.relli@...inx.com>
> > CC: Michal Simek <michal.simek@...inx.com>
> > CC: Stephane Grosjean <s.grosjean@...k-system.com>
> > Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.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
Powered by blists - more mailing lists