netdev - Re: [PATCH net-next] net: tighten the definition of interface statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200903160228.53f68526@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 3 Sep 2020 16:02:28 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     David Ahern <dsahern@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, jwi@...ux.ibm.com,
        f.fainelli@...il.com, andrew@...n.ch, mkubecek@...e.cz,
        edwin.peer@...adcom.com, michael.chan@...adcom.com,
        saeedm@...lanox.com, rmk+kernel@...linux.org.uk
Subject: Re: [PATCH net-next] net: tighten the definition of interface
 statistics

On Thu, 3 Sep 2020 16:40:54 -0600 David Ahern wrote:
> On 9/2/20 8:03 PM, Jakub Kicinski wrote:
> > +sysfs
> > +-----
> > +
> > +Each device directory in sysfs contains a `statistics` directory (e.g.
> > +`/sys/class/net/lo/statistics/`) with files corresponding to
> > +members of :c:type:`struct rtnl_link_stats64 <rtnl_link_stats64>`.
> > +
> > +This simple interface is convenient especially in constrained/embedded
> > +environments without access to tools. However, it's sightly inefficient  
> 
> sightly seems like the wrong word. Did you mean 'highly inefficient'?

Indeed, I'll drop the "slightly". Hopefully the info below is clear
enough for users to understand what's happening. 

> > +when reading multiple stats as it internally performs a full dump of
> > +:c:type:`struct rtnl_link_stats64 <rtnl_link_stats64>`
> > +and reports only the stat corresponding to the accessed file.
> > +
> > +Sysfs files are documented in
> > +`Documentation/ABI/testing/sysfs-class-net-statistics`.
> > +
> > +
> > +netlink
> > +-------
> > +
> > +`rtnetlink` (`NETLINK_ROUTE`) is the preferred method of accessing
> > +:c:type:`struct rtnl_link_stats64 <rtnl_link_stats64>` stats.
> > +
> > +Statistics are reported both in the responses to link information
> > +requests (`RTM_GETLINK`) and statistic requests (`RTM_GETSTATS`,
> > +when `IFLA_STATS_LINK_64` bit is set in the `.filter_mask` of the request).
> > +
> > +ethtool
> > +-------
> > +
> > +Ethtool IOCTL interface allows drivers to report implementation
> > +specific statistics.  
> 
> an example here would be helpful. e.g., I use `ethool -S` primarily for
> per queue stats which show more details than the other APIs which show
> aggregated stats.

I'll mention the queue stats here.

> > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> > index 7fba4de511de..6ea0fb48739e 100644
> > --- a/include/uapi/linux/if_link.h
> > +++ b/include/uapi/linux/if_link.h
> > @@ -40,26 +40,191 @@ struct rtnl_link_stats {
> >  	__u32	rx_nohandler;		/* dropped, no handler found	*/
> >  };
> >  
> > -/* The main device statistics structure */
> > +/**
> > + * struct rtnl_link_stats64 - The main device statistics structure.
> > + *
> > + * @rx_packets: Number of good packets received by the interface.
> > + *   For hardware interfaces counts all good packets seen by the host,
> > + *   including packets which host had to drop at various stages of processing
> > + *   (even in the driver).
> > + *
> > + * @tx_packets: Number of packets successfully transmitted.
> > + *   For hardware interfaces counts packets which host was able to successfully
> > + *   hand over to the device, which does not necessarily mean that packets
> > + *   had been successfully transmitted out of the device, only that device
> > + *   acknowledged it copied them out of host memory.
> > + *
> > + * @rx_bytes: Number of good incoming bytes, corresponding to @rx_packets.  
> 
> s/incoming/received/?
> 
> > + *
> > + * @tx_bytes: Number of good incoming bytes, corresponding to @tx_packets.  
> 
> s/incoming/transmitted/

:o I wonder where I got those from.

> Thanks for taking the time to work on this; I'm sure you spent a LOT of
> hours going through all of the drivers and APIs.

And "dusty datasheets" :)