can: do not increase rx statistics when generating a CAN rx error message frame
authorVincent Mailhol <mailhol.vincent@wanadoo.fr>
Tue, 7 Dec 2021 12:15:27 +0000 (21:15 +0900)
committerMarc Kleine-Budde <mkl@pengutronix.de>
Wed, 5 Jan 2022 11:09:05 +0000 (12:09 +0100)
The CAN error message frames (i.e. error skb) are an interface
specific to socket CAN. The payload of the CAN error message frames
does not correspond to any actual data sent on the wire. Only an error
flag and a delimiter are transmitted when an error occurs (c.f. ISO
11898-1 section 10.4.4.2 "Error flag").

For this reason, it makes no sense to increment the rx_packets and
rx_bytes fields of struct net_device_stats because no actual payload
were transmitted on the wire.

This patch fixes all the CAN drivers.

Link: https://lore.kernel.org/all/20211207121531.42941-2-mailhol.vincent@wanadoo.fr
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: 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>
Tested-by: Jimmy Assarsson <extja@kvaser.com> # kvaser
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2
Tested-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
27 files changed:
drivers/net/can/at91_can.c
drivers/net/can/c_can/c_can_main.c
drivers/net/can/cc770/cc770.c
drivers/net/can/dev/dev.c
drivers/net/can/dev/rx-offload.c
drivers/net/can/ifi_canfd/ifi_canfd.c
drivers/net/can/kvaser_pciefd.c
drivers/net/can/m_can/m_can.c
drivers/net/can/mscan/mscan.c
drivers/net/can/pch_can.c
drivers/net/can/peak_canfd/peak_canfd.c
drivers/net/can/rcar/rcar_can.c
drivers/net/can/rcar/rcar_canfd.c
drivers/net/can/sja1000/sja1000.c
drivers/net/can/sun4i_can.c
drivers/net/can/usb/ems_usb.c
drivers/net/can/usb/esd_usb2.c
drivers/net/can/usb/etas_es58x/es58x_core.c
drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
drivers/net/can/usb/peak_usb/pcan_usb.c
drivers/net/can/usb/peak_usb/pcan_usb_fd.c
drivers/net/can/usb/peak_usb/pcan_usb_pro.c
drivers/net/can/usb/ucan.c
drivers/net/can/usb/usb_8dev.c
drivers/net/can/xilinx_can.c

index 3aea32c..3cd872c 100644 (file)
@@ -553,8 +553,6 @@ static void at91_rx_overflow_err(struct net_device *dev)
        cf->can_id |= CAN_ERR_CRTL;
        cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_receive_skb(skb);
 }
 
@@ -779,8 +777,6 @@ static int at91_poll_err(struct net_device *dev, int quota, u32 reg_sr)
 
        at91_poll_err_frame(dev, cf, reg_sr);
 
-       dev->stats.rx_packets++;
-       dev->stats.rx_bytes += cf->len;
        netif_receive_skb(skb);
 
        return 1;
@@ -1037,8 +1033,6 @@ static void at91_irq_err(struct net_device *dev)
 
        at91_irq_err_state(dev, cf, new_state);
 
-       dev->stats.rx_packets++;
-       dev->stats.rx_bytes += cf->len;
        netif_rx(skb);
 
        priv->can.state = new_state;
index 52671d1..670754a 100644 (file)
@@ -920,7 +920,6 @@ static int c_can_handle_state_change(struct net_device *dev,
        unsigned int reg_err_counter;
        unsigned int rx_err_passive;
        struct c_can_priv *priv = netdev_priv(dev);
-       struct net_device_stats *stats = &dev->stats;
        struct can_frame *cf;
        struct sk_buff *skb;
        struct can_berr_counter bec;
@@ -996,8 +995,6 @@ static int c_can_handle_state_change(struct net_device *dev,
                break;
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_receive_skb(skb);
 
        return 1;
@@ -1064,8 +1061,6 @@ static int c_can_handle_bus_err(struct net_device *dev,
                break;
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_receive_skb(skb);
        return 1;
 }
index f8a130f..a5fd8cc 100644 (file)
@@ -499,7 +499,6 @@ static void cc770_rx(struct net_device *dev, unsigned int mo, u8 ctrl1)
 static int cc770_err(struct net_device *dev, u8 status)
 {
        struct cc770_priv *priv = netdev_priv(dev);
-       struct net_device_stats *stats = &dev->stats;
        struct can_frame *cf;
        struct sk_buff *skb;
        u8 lec;
@@ -571,8 +570,6 @@ static int cc770_err(struct net_device *dev, u8 status)
        }
 
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 
        return 0;
index e3d840b..4845ae6 100644 (file)
@@ -136,7 +136,6 @@ EXPORT_SYMBOL_GPL(can_change_state);
 static void can_restart(struct net_device *dev)
 {
        struct can_priv *priv = netdev_priv(dev);
-       struct net_device_stats *stats = &dev->stats;
        struct sk_buff *skb;
        struct can_frame *cf;
        int err;
@@ -155,9 +154,6 @@ static void can_restart(struct net_device *dev)
 
        cf->can_id |= CAN_ERR_RESTARTED;
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
-
        netif_rx_ni(skb);
 
 restart:
index 37b0cc6..7dbf46b 100644 (file)
@@ -54,8 +54,10 @@ static int can_rx_offload_napi_poll(struct napi_struct *napi, int quota)
                struct can_frame *cf = (struct can_frame *)skb->data;
 
                work_done++;
-               stats->rx_packets++;
-               stats->rx_bytes += cf->len;
+               if (!(cf->can_id & CAN_ERR_FLAG)) {
+                       stats->rx_packets++;
+                       stats->rx_bytes += cf->len;
+               }
                netif_receive_skb(skb);
        }
 
index 5bb957a..e8318e9 100644 (file)
@@ -430,8 +430,6 @@ static int ifi_canfd_handle_lec_err(struct net_device *ndev)
               priv->base + IFI_CANFD_INTERRUPT);
        writel(IFI_CANFD_ERROR_CTR_ER_ENABLE, priv->base + IFI_CANFD_ERROR_CTR);
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_receive_skb(skb);
 
        return 1;
@@ -456,7 +454,6 @@ static int ifi_canfd_handle_state_change(struct net_device *ndev,
                                         enum can_state new_state)
 {
        struct ifi_canfd_priv *priv = netdev_priv(ndev);
-       struct net_device_stats *stats = &ndev->stats;
        struct can_frame *cf;
        struct sk_buff *skb;
        struct can_berr_counter bec;
@@ -522,8 +519,6 @@ static int ifi_canfd_handle_state_change(struct net_device *ndev,
                break;
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_receive_skb(skb);
 
        return 1;
index eb74cdf..ab672c9 100644 (file)
@@ -1310,9 +1310,6 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
        cf->data[6] = bec.txerr;
        cf->data[7] = bec.rxerr;
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
-
        netif_rx(skb);
        return 0;
 }
@@ -1510,8 +1507,6 @@ static void kvaser_pciefd_handle_nack_packet(struct kvaser_pciefd_can *can,
 
        if (skb) {
                cf->can_id |= CAN_ERR_BUSERROR;
-               stats->rx_bytes += cf->len;
-               stats->rx_packets++;
                netif_rx(skb);
        } else {
                stats->rx_dropped++;
index c2a8421..30d94cb 100644 (file)
@@ -647,9 +647,6 @@ static int m_can_handle_lec_err(struct net_device *dev,
                break;
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
-
        if (cdev->is_peripheral)
                timestamp = m_can_get_timestamp(cdev);
 
@@ -706,7 +703,6 @@ static int m_can_handle_state_change(struct net_device *dev,
                                     enum can_state new_state)
 {
        struct m_can_classdev *cdev = netdev_priv(dev);
-       struct net_device_stats *stats = &dev->stats;
        struct can_frame *cf;
        struct sk_buff *skb;
        struct can_berr_counter bec;
@@ -771,9 +767,6 @@ static int m_can_handle_state_change(struct net_device *dev,
                break;
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
-
        if (cdev->is_peripheral)
                timestamp = m_can_get_timestamp(cdev);
 
index fa32e41..9e1cce0 100644 (file)
@@ -401,13 +401,14 @@ static int mscan_rx_poll(struct napi_struct *napi, int quota)
                        continue;
                }
 
-               if (canrflg & MSCAN_RXF)
+               if (canrflg & MSCAN_RXF) {
                        mscan_get_rx_frame(dev, frame);
-               else if (canrflg & MSCAN_ERR_IF)
+                       stats->rx_packets++;
+                       stats->rx_bytes += frame->len;
+               } else if (canrflg & MSCAN_ERR_IF) {
                        mscan_get_err_frame(dev, frame, canrflg);
+               }
 
-               stats->rx_packets++;
-               stats->rx_bytes += frame->len;
                work_done++;
                netif_receive_skb(skb);
        }
index 964c8a0..6b45840 100644 (file)
@@ -561,9 +561,6 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 
        priv->can.state = state;
        netif_receive_skb(skb);
-
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
 }
 
 static irqreturn_t pch_can_interrupt(int irq, void *dev_id)
index d08718e..d5b8bc6 100644 (file)
@@ -409,8 +409,6 @@ static int pucan_handle_status(struct peak_canfd_priv *priv,
                return -ENOMEM;
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        pucan_netif_rx(skb, msg->ts_low, msg->ts_high);
 
        return 0;
@@ -438,8 +436,6 @@ static int pucan_handle_cache_critical(struct peak_canfd_priv *priv)
        cf->data[6] = priv->bec.txerr;
        cf->data[7] = priv->bec.rxerr;
 
-       stats->rx_bytes += cf->len;
-       stats->rx_packets++;
        netif_rx(skb);
 
        return 0;
index 8999ec9..f408ed9 100644 (file)
@@ -223,7 +223,6 @@ static void tx_failure_cleanup(struct net_device *ndev)
 static void rcar_can_error(struct net_device *ndev)
 {
        struct rcar_can_priv *priv = netdev_priv(ndev);
-       struct net_device_stats *stats = &ndev->stats;
        struct can_frame *cf;
        struct sk_buff *skb;
        u8 eifr, txerr = 0, rxerr = 0;
@@ -362,11 +361,8 @@ static void rcar_can_error(struct net_device *ndev)
                }
        }
 
-       if (skb) {
-               stats->rx_packets++;
-               stats->rx_bytes += cf->len;
+       if (skb)
                netif_rx(skb);
-       }
 }
 
 static void rcar_can_tx_done(struct net_device *ndev)
index ff9d0f5..db9d628 100644 (file)
@@ -1033,8 +1033,6 @@ static void rcar_canfd_error(struct net_device *ndev, u32 cerfl,
        /* Clear channel error interrupts that are handled */
        rcar_canfd_write(priv->base, RCANFD_CERFL(ch),
                         RCANFD_CERFL_ERR(~cerfl));
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 }
 
@@ -1174,8 +1172,6 @@ static void rcar_canfd_state_change(struct net_device *ndev,
                rx_state = txerr <= rxerr ? state : 0;
 
                can_change_state(ndev, cf, tx_state, rx_state);
-               stats->rx_packets++;
-               stats->rx_bytes += cf->len;
                netif_rx(skb);
        }
 }
index 3fad546..a65546c 100644 (file)
@@ -487,8 +487,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
                        can_bus_off(dev);
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 
        return 0;
index 37862e7..a2d8d77 100644 (file)
@@ -633,13 +633,10 @@ static int sun4i_can_err(struct net_device *dev, u8 isrc, u8 status)
                        can_bus_off(dev);
        }
 
-       if (likely(skb)) {
-               stats->rx_packets++;
-               stats->rx_bytes += cf->len;
+       if (likely(skb))
                netif_rx(skb);
-       } else {
+       else
                return -ENOMEM;
-       }
 
        return 0;
 }
index 2b5302e..7cf6593 100644 (file)
@@ -397,8 +397,6 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg)
                stats->rx_errors++;
        }
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 }
 
index c6068a2..5f6915a 100644 (file)
@@ -293,8 +293,6 @@ static void esd_usb2_rx_event(struct esd_usb2_net_priv *priv,
                priv->bec.txerr = txerr;
                priv->bec.rxerr = rxerr;
 
-               stats->rx_packets++;
-               stats->rx_bytes += cf->len;
                netif_rx(skb);
        }
 }
index 8508a73..2ed2370 100644 (file)
@@ -849,13 +849,6 @@ int es58x_rx_err_msg(struct net_device *netdev, enum es58x_err error,
                break;
        }
 
-       /* driver/net/can/dev.c:can_restart() takes in account error
-        * messages in the RX stats. Doing the same here for
-        * consistency.
-        */
-       netdev->stats.rx_packets++;
-       netdev->stats.rx_bytes += CAN_ERR_DLC;
-
        if (cf) {
                if (cf->data[1])
                        cf->can_id |= CAN_ERR_CRTL;
index 0cc0fc8..3e682ef 100644 (file)
@@ -279,8 +279,6 @@ int kvaser_usb_can_rx_over_error(struct net_device *netdev)
        cf->can_id |= CAN_ERR_CRTL;
        cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 
        return 0;
index cec3629..8e2ff08 100644 (file)
@@ -870,7 +870,6 @@ static void kvaser_usb_hydra_update_state(struct kvaser_usb_net_priv *priv,
        struct net_device *netdev = priv->netdev;
        struct can_frame *cf;
        struct sk_buff *skb;
-       struct net_device_stats *stats;
        enum can_state new_state, old_state;
 
        old_state = priv->can.state;
@@ -920,9 +919,6 @@ static void kvaser_usb_hydra_update_state(struct kvaser_usb_net_priv *priv,
        cf->data[6] = bec->txerr;
        cf->data[7] = bec->rxerr;
 
-       stats = &netdev->stats;
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 }
 
@@ -1075,8 +1071,6 @@ kvaser_usb_hydra_error_frame(struct kvaser_usb_net_priv *priv,
        cf->data[6] = bec.txerr;
        cf->data[7] = bec.rxerr;
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 
        priv->bec.txerr = bec.txerr;
@@ -1110,8 +1104,6 @@ static void kvaser_usb_hydra_one_shot_fail(struct kvaser_usb_net_priv *priv,
        }
 
        stats->tx_errors++;
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 }
 
index aed271d..d7e8e9b 100644 (file)
@@ -642,8 +642,6 @@ static void kvaser_usb_leaf_tx_acknowledge(const struct kvaser_usb *dev,
                if (skb) {
                        cf->can_id |= CAN_ERR_RESTARTED;
 
-                       stats->rx_packets++;
-                       stats->rx_bytes += cf->len;
                        netif_rx(skb);
                } else {
                        netdev_err(priv->netdev,
@@ -844,8 +842,6 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
        cf->data[6] = es->txerr;
        cf->data[7] = es->rxerr;
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 }
 
index ac6772f..a594178 100644 (file)
@@ -521,8 +521,6 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
                                     &hwts->hwtstamp);
        }
 
-       mc->netdev->stats.rx_packets++;
-       mc->netdev->stats.rx_bytes += cf->len;
        netif_rx(skb);
 
        return 0;
index 6bd1254..185f5a9 100644 (file)
@@ -577,9 +577,6 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
        if (!skb)
                return -ENOMEM;
 
-       netdev->stats.rx_packets++;
-       netdev->stats.rx_bytes += cf->len;
-
        peak_usb_netif_rx_64(skb, le32_to_cpu(sm->ts_low),
                             le32_to_cpu(sm->ts_high));
 
index 858ab22..f6d1987 100644 (file)
@@ -660,8 +660,6 @@ static int pcan_usb_pro_handle_error(struct pcan_usb_pro_interface *usb_if,
 
        hwts = skb_hwtstamps(skb);
        peak_usb_get_ts_time(&usb_if->time_ref, le32_to_cpu(er->ts32), &hwts->hwtstamp);
-       netdev->stats.rx_packets++;
-       netdev->stats.rx_bytes += can_frame->len;
        netif_rx(skb);
 
        return 0;
index 1679cbe..d582c39 100644 (file)
@@ -621,8 +621,10 @@ static void ucan_rx_can_msg(struct ucan_priv *up, struct ucan_message_in *m)
                memcpy(cf->data, m->msg.can_msg.data, cf->len);
 
        /* don't count error frames as real packets */
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
+       if (!(cf->can_id & CAN_ERR_FLAG)) {
+               stats->rx_packets++;
+               stats->rx_bytes += cf->len;
+       }
 
        /* pass it to Linux */
        netif_rx(skb);
index 1fa0290..c81ca5e 100644 (file)
@@ -447,8 +447,6 @@ static void usb_8dev_rx_err_msg(struct usb_8dev_priv *priv,
        priv->bec.txerr = txerr;
        priv->bec.rxerr = rxerr;
 
-       stats->rx_packets++;
-       stats->rx_bytes += cf->len;
        netif_rx(skb);
 }
 
index e2b15d2..275e240 100644 (file)
@@ -965,13 +965,8 @@ static void xcan_update_error_state_after_rxtx(struct net_device *ndev)
 
                xcan_set_error_state(ndev, new_state, skb ? cf : NULL);
 
-               if (skb) {
-                       struct net_device_stats *stats = &ndev->stats;
-
-                       stats->rx_packets++;
-                       stats->rx_bytes += cf->len;
+               if (skb)
                        netif_rx(skb);
-               }
        }
 }
 
@@ -1095,8 +1090,6 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
                if (skb) {
                        skb_cf->can_id |= cf.can_id;
                        memcpy(skb_cf->data, cf.data, CAN_ERR_DLC);
-                       stats->rx_packets++;
-                       stats->rx_bytes += CAN_ERR_DLC;
                        netif_rx(skb);
                }
        }