Re: [PATCH v4 5/5] can: do not increase tx_bytes statistics for RTR frames
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2021-12-06 16:17:48
Also in:
linux-arm-kernel, linux-can, linux-sunxi, lkml
On 03.12.2021 22:18:08, Vincent Mailhol wrote:
quoted hunk ↗ jump to hunk
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 use the DLC to indicate the length of the requested frame. As such, net_device_stats:tx_bytes should not be increased when sending RTR frames. The function can_get_echo_skb() already returns the correct length, even for RTR frames (c.f. [1]). However, for historical reasons, the drivers do not use can_get_echo_skb()'s return value and instead, most of them store a temporary length (or dlc) in some local structure or array. Using the return value of can_get_echo_skb() solves the issue. After doing this, such length/dlc fields become unused and so this patch does the adequate cleaning when needed. This patch fixes all the CAN drivers. Finally, can_get_echo_skb() is decorated with the __must_check attribute in order to force future drivers to correctly use its return value (else the compiler would emit a warning). [1] commit ed3320cec279 ("can: dev: __can_get_echo_skb(): fix real payload length return value for RTR frames") CC: Nicolas Ferre <nicolas.ferre@microchip.com> CC: Alexandre Belloni <alexandre.belloni@bootlin.com> CC: Ludovic Desroches <ludovic.desroches@microchip.com> 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: Oliver Hartkopp <socketcan@hartkopp.net> CC: Stephane Grosjean <redacted> Tested-by: Jimmy Assarsson <redacted> Signed-off-by: Vincent Mailhol <redacted> --- drivers/net/can/at91_can.c | 8 ++-- drivers/net/can/c_can/c_can.h | 1 - drivers/net/can/c_can/c_can_main.c | 7 +--- drivers/net/can/cc770/cc770.c | 8 +--- drivers/net/can/janz-ican3.c | 3 +- drivers/net/can/mscan/mscan.c | 4 +- drivers/net/can/pch_can.c | 9 ++-- drivers/net/can/peak_canfd/peak_canfd.c | 3 +- drivers/net/can/rcar/rcar_can.c | 11 +++-- drivers/net/can/rcar/rcar_canfd.c | 6 +-- drivers/net/can/sja1000/sja1000.c | 4 +- drivers/net/can/slcan.c | 3 +- drivers/net/can/softing/softing_main.c | 8 ++-- drivers/net/can/spi/hi311x.c | 24 +++++------ drivers/net/can/spi/mcp251x.c | 25 +++++------ drivers/net/can/sun4i_can.c | 5 +-- drivers/net/can/usb/ems_usb.c | 7 +--- drivers/net/can/usb/esd_usb2.c | 6 +-- drivers/net/can/usb/gs_usb.c | 7 ++-- drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 5 +-- .../net/can/usb/kvaser_usb/kvaser_usb_core.c | 2 +- .../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 42 +++++++++---------- .../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 13 +++--- drivers/net/can/usb/mcba_usb.c | 12 ++---- drivers/net/can/usb/peak_usb/pcan_usb_core.c | 20 ++++----- drivers/net/can/usb/peak_usb/pcan_usb_core.h | 1 - drivers/net/can/usb/ucan.c | 10 ++--- drivers/net/can/usb/usb_8dev.c | 6 +-- drivers/net/can/vcan.c | 7 ++-- drivers/net/can/vxcan.c | 2 +- include/linux/can/skb.h | 5 ++- 31 files changed, 114 insertions(+), 160 deletions(-)diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c index 97f1d08b4133..b37d9b4f508e 100644 --- a/drivers/net/can/at91_can.c +++ b/drivers/net/can/at91_can.c@@ -448,7 +448,6 @@ static void at91_chip_stop(struct net_device *dev, enum can_state state) static netdev_tx_t at91_start_xmit(struct sk_buff *skb, struct net_device *dev) { struct at91_priv *priv = netdev_priv(dev); - struct net_device_stats *stats = &dev->stats; struct can_frame *cf = (struct can_frame *)skb->data; unsigned int mb, prio; u32 reg_mid, reg_mcr;@@ -480,8 +479,6 @@ static netdev_tx_t at91_start_xmit(struct sk_buff *skb, struct net_device *dev) /* This triggers transmission */ at91_write(priv, AT91_MCR(mb), reg_mcr); - stats->tx_bytes += cf->len; - /* _NOTE_: subtract AT91_MB_TX_FIRST offset from mb! */ can_put_echo_skb(skb, dev, mb - get_mb_tx_first(priv), 0);@@ -852,7 +849,10 @@ static void at91_irq_tx(struct net_device *dev, u32 reg_sr) if (likely(reg_msr & AT91_MSR_MRDY && ~reg_msr & AT91_MSR_MABT)) { /* _NOTE_: subtract AT91_MB_TX_FIRST offset from mb! */ - can_get_echo_skb(dev, mb - get_mb_tx_first(priv), NULL); + dev->stats.tx_bytes =
+= ? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachments
- signature.asc [application/pgp-signature] 488 bytes