Skip to content

Commit

Permalink
can: do not increase rx statistics when generating a CAN rx error mes…
Browse files Browse the repository at this point in the history
…sage frame

[ Upstream commit 676068d ]

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/[email protected]
CC: Marc Kleine-Budde <[email protected]>
CC: Nicolas Ferre <[email protected]>
CC: Alexandre Belloni <[email protected]>
CC: Ludovic Desroches <[email protected]>
CC: Chandrasekar Ramakrishnan <[email protected]>
CC: Maxime Ripard <[email protected]>
CC: Chen-Yu Tsai <[email protected]>
CC: Jernej Skrabec <[email protected]>
CC: Appana Durga Kedareswara rao <[email protected]>
CC: Naga Sureshkumar Relli <[email protected]>
CC: Michal Simek <[email protected]>
CC: Stephane Grosjean <[email protected]>
Tested-by: Jimmy Assarsson <[email protected]> # kvaser
Signed-off-by: Vincent Mailhol <[email protected]>
Acked-by: Stefan Mätje <[email protected]> # esd_usb2
Tested-by: Stefan Mätje <[email protected]> # esd_usb2
Signed-off-by: Marc Kleine-Budde <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
  • Loading branch information
vincent-mailhol authored and Sasha Levin committed Jan 26, 2022
1 parent 188b75e commit 4fb9f4b
Show file tree
Hide file tree
Showing 27 changed files with 17 additions and 108 deletions.
6 changes: 0 additions & 6 deletions drivers/net/can/at91_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 0 additions & 5 deletions drivers/net/can/c_can/c_can_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 0 additions & 3 deletions drivers/net/can/cc770/cc770.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions drivers/net/can/dev/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions drivers/net/can/dev/rx-offload.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
5 changes: 0 additions & 5 deletions drivers/net/can/ifi_canfd/ifi_canfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 0 additions & 5 deletions drivers/net/can/kvaser_pciefd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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++;
Expand Down
7 changes: 0 additions & 7 deletions drivers/net/can/m_can/m_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
9 changes: 5 additions & 4 deletions drivers/net/can/mscan/mscan.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
3 changes: 0 additions & 3 deletions drivers/net/can/pch_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions drivers/net/can/peak_canfd/peak_canfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions drivers/net/can/rcar/rcar_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions drivers/net/can/rcar/rcar_canfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
}
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/can/sja1000/sja1000.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 2 additions & 5 deletions drivers/net/can/sun4i_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -622,13 +622,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;
}
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/can/usb/ems_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 0 additions & 2 deletions drivers/net/can/usb/esd_usb2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
7 changes: 0 additions & 7 deletions drivers/net/can/usb/etas_es58x/es58x_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 0 additions & 8 deletions drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,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;
Expand Down Expand Up @@ -919,9 +918,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);
}

Expand Down Expand Up @@ -1074,8 +1070,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;
Expand Down Expand Up @@ -1109,8 +1103,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);
}

Expand Down
4 changes: 0 additions & 4 deletions drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
Original file line number Diff line number Diff line change
Expand Up @@ -641,8 +641,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,
Expand Down Expand Up @@ -843,8 +841,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);
}

Expand Down
2 changes: 0 additions & 2 deletions drivers/net/can/usb/peak_usb/pcan_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -520,8 +520,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;
Expand Down
3 changes: 0 additions & 3 deletions drivers/net/can/usb/peak_usb/pcan_usb_fd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
Loading

0 comments on commit 4fb9f4b

Please sign in to comment.