[v2,net-next,1/2] Documentation: sysfs-class-net-statistics: Clarify rx_bytes and tx_bytes

Document what is expected for the rx_bytes and tx_bytes statistics in
/sys/class/net/<device>/statistics. The FCS should be included in the
statistics. However, since this has been unclear until now, it is
expected a number of drivers don't. But maybe with time they will.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 Documentation/ABI/testing/sysfs-class-net-statistics | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

On Tue, 23 May 2017 23:10:57 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> Document what is expected for the rx_bytes and tx_bytes statistics in
> /sys/class/net/<device>/statistics. The FCS should be included in the
> statistics. However, since this has been unclear until now, it is
> expected a number of drivers don't. But maybe with time they will.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  Documentation/ABI/testing/sysfs-class-net-statistics | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-net-statistics b/Documentation/ABI/testing/sysfs-class-net-statistics
> index 397118de7b5e..a487cbb79458 100644
> --- a/Documentation/ABI/testing/sysfs-class-net-statistics
> +++ b/Documentation/ABI/testing/sysfs-class-net-statistics
> @@ -21,7 +21,8 @@ Contact:	netdev@vger.kernel.org
>  Description:
>  		Indicates the number of bytes received by this network device.
>  		See the network driver for the exact meaning of when this
> -		value is incremented.
> +		value is incremented. However, for an Ethernet frame, it should
> +		include the Ethernet header, data, and frame check sum.
>  
>  What:		/sys/class/<iface>/statistics/rx_compressed
>  Date:		April 2005
> @@ -125,7 +126,8 @@ Description:
>  		device. See the network driver for the exact meaning of this
>  		value, in particular whether this accounts for all successfully
>  		transmitted packets or all packets that have been queued for
> -		transmission.
> +		transmission. For an Ethernet frame, it should include the
> +		Ethernet header, data, and frame check sum.
>  
>  What:		/sys/class/<iface>/statistics/tx_carrier_errors
>  Date:		April 2005

Sysfs statistics must match the netlink statistics. What matters is the
values reported in netlink, proc, and sysfs are the same.
Documenting the sysfs value here is nice but less important.
I worry that some naive implementer or tester might expect sysfs values
to follow different standard than other places.

For devices the important thing is that rx and tx values match. I.e if
FCS is counted on RX then it needs to get counted on Tx. I don't think
FCS should be part of data  statistics since many devices are virtual
and have no visible FCS.  For example if a packet is sent of virtio (no FCS)
to host vhost (no FCS) and then through bridge to physical hardware
which with your proposal does have FCS; all the data counts all matched.

With the advent of overlay networks this gets even more confusing.
Does overlay header count?

Linux in general has followed the BSD model of NOT including FCS
in device statistics. Some hardware vendors do include the FCS in
their stats but that is really a porting bug.

Unfortunately, even the router vendors can't agree on this.
RFC 2665 says that FCS should be included, but BSD derived systems
like Juniper do not. This is a mess.

From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 23 May 2017 23:10:57 +0200

> Document what is expected for the rx_bytes and tx_bytes statistics in
> /sys/class/net/<device>/statistics. The FCS should be included in the
> statistics. However, since this has been unclear until now, it is
> expected a number of drivers don't. But maybe with time they will.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Precedence has not been very kind to us here.

Also, we really I want drivers doing the simplest thing possible,
which on transmit is:

	tx_bytes += skb->len;

Not:

	tx_bytes += skb->len + ETH_FCS_LEN;

I understand the problem you're trying to tackle, but software stats
should be for software state, and that state is emphatically skb->len

So if we are to strive for software statistic "consistency" it should
be against the software part of the packet, not the software part
"plus X and Y link layer stuff no visible in the software packet".

Thanks.