On 2021-11-20 04:06, Vincent Mailhol wrote:
On Fri. 19 Nov 2021 at 05:20, Jimmy Assarsson <extja@xxxxxxxxxx> wrote:
Do not increase net_device_stats rx_{packets,bytes} when receiving
error frames.
Signed-off-by: Jimmy Assarsson <extja@xxxxxxxxxx>
---
drivers/net/can/kvaser_pciefd.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 74d9899fc904..2c98befcf2a0 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -1304,9 +1304,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;
}
I think that this patch makes sense because the CAN error frames do
not exists on the wire: only an error flag is transmitted.
However, the current consensus is that the rx_packets and rx_bytes
statistics should be incremented for CAN error frames. And I think
that consistency between the drivers is the first priority.
I inquired here in the past to ask if it made sense to stop increasing
the rx stats for CAN error frames:
https://lore.kernel.org/linux-can/CAMZ6Rq+8YSRqXU7CPrT9FKnWZ1G9xkSr3wt185r2CswmxhXPVg@xxxxxxxxxxxxxx/t/#u
But the discussion did not raise interest. I am fine to send a tree
wide cleaning patch, but first, I would like to have people agree on
this.
Thanks, I've not seen that discussion.
I agree, it doesn't make sense for a user to see the rx_packets and
rx_bytes increase, when in reality they didn't receive any valid frames.
This change started as a request from one of our customers, that got
confused by the increase of rx_packets, when connected to a bus with
wrong bitrate.
I'm in favour of a tree wide patch.
Best regards,
jimmy
