Thread (3 messages) 3 messages, 2 authors, 2021-03-19

Re: [PATCH] can: dev: do not increment rx stats when generating a CAN error skb

From: Vincent MAILHOL <hidden>
Date: 2021-03-19 09:42:13

On Fri. 19 Mar 2021 at 17:29, Marc Kleine-Budde [off-list ref] wrote:
On 17.03.2021 17:52:23, Vincent Mailhol wrote:
quoted
CAN error skb is an interface specific to socket CAN. The CAN error
skb does not correspond to any actual CAN frame sent on the wire. Only
an error flag is 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.
I think we increment the rx packets and bytes (more or less
consistently) in all drivers for CAN error skbs.
If I understand correctly your comment, you mean that this patch
alone is not good because it will create a discrepancy with the
drivers. This is fair enough.

The real question is what is the meaning of rx_packets and
rx_bytes? My understanding is that those statistics should
represent what is sent on the wire.  Do we agree? If not, what is
the rationale for counting error skb in the device statistics?
struct can_device_stats already keeps track of the errors
statistics so incrementing the struct net_device_stats adds no
information here.

I am fine to send a more complete patch which will also do the
cleanup on all drivers. But before starting that work, I prefer
to confirm that we are aligned.


Yours sincerely,
Vincent
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help