Hi Stefan,
On Thu. 25 Nov 2021 at 02:59, Stefan Mätje <Stefan.Maetje@xxxxxx> 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@xxxxxxxxxx>
> > CC: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> > CC: Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx>
> > CC: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> > CC: Ludovic Desroches <ludovic.desroches@xxxxxxxxxxxxx>
> > CC: Chandrasekar Ramakrishnan <rcsekar@xxxxxxxxxxx>
> > CC: Maxime Ripard <mripard@xxxxxxxxxx>
> > CC: Chen-Yu Tsai <wens@xxxxxxxx>
> > CC: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> > CC: Yasushi SHOJI <yashi@xxxxxxxxxxxxxxx>
> > CC: Appana Durga Kedareswara rao <appana.durga.rao@xxxxxxxxxx>
> > CC: Naga Sureshkumar Relli <naga.sureshkumar.relli@xxxxxxxxxx>
> > CC: Michal Simek <michal.simek@xxxxxxxxxx>
> > CC: Stephane Grosjean <s.grosjean@xxxxxxxxxxxxxxx>
> > Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
> > ---
> > 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
