linux-kernel - RE: [PATCH V2 net-next 7/8] net: hns3: Add Ethtool support to HNS3 driver

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]

Date:   Sat, 17 Jun 2017 11:05:27 +0000
From:   Salil Mehta <salil.mehta@...wei.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "Zhuangyuzeng (Yisen)" <yisen.zhuang@...wei.com>,
        huangdaode <huangdaode@...ilicon.com>,
        "lipeng (Y)" <lipeng321@...wei.com>,
        "mehta.salil.lnk@...il.com" <mehta.salil.lnk@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linuxarm <linuxarm@...wei.com>
Subject: RE: [PATCH V2 net-next 7/8] net: hns3: Add Ethtool support to HNS3
 driver

Hi Stephen

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@...workplumber.org]
> Sent: Wednesday, June 14, 2017 12:55 AM
> To: Salil Mehta
> Cc: davem@...emloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil.lnk@...il.com; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; Linuxarm
> Subject: Re: [PATCH V2 net-next 7/8] net: hns3: Add Ethtool support to
> HNS3 driver
> 
> On Wed, 14 Jun 2017 00:10:34 +0100
> Salil Mehta <salil.mehta@...wei.com> wrote:
> 
> > +/* netdev related stats */
> > +#define HNS3_NETDEV_STAT(_string, _member)			\
> > +	{ _string,						\
> > +	  FIELD_SIZEOF(struct rtnl_link_stats64, _member),	\
> > +	  offsetof(struct rtnl_link_stats64, _member),		\
> > +	}
> > +
> > +static const struct hns3_stats hns3_netdev_stats[] = {
> > +	/* misc. Rx/Tx statistics */
> > +	HNS3_NETDEV_STAT("rx_packets", rx_packets),
> > +	HNS3_NETDEV_STAT("tx_packets", tx_packets),
> > +	HNS3_NETDEV_STAT("rx_bytes", rx_bytes),
> > +	HNS3_NETDEV_STAT("tx_bytes", tx_bytes),
> > +	HNS3_NETDEV_STAT("rx_errors", rx_errors),
> > +	HNS3_NETDEV_STAT("tx_errors", tx_errors),
> > +	HNS3_NETDEV_STAT("rx_dropped", rx_dropped),
> > +	HNS3_NETDEV_STAT("tx_dropped", tx_dropped),
> > +	HNS3_NETDEV_STAT("multicast", multicast),
> > +	HNS3_NETDEV_STAT("collisions", collisions),
> > +
> > +	/* detailed Rx errors */
> > +	HNS3_NETDEV_STAT("rx_length_errors", rx_length_errors),
> > +	HNS3_NETDEV_STAT("rx_over_errors", rx_over_errors),
> > +	HNS3_NETDEV_STAT("rx_crc_errors", rx_crc_errors),
> > +	HNS3_NETDEV_STAT("rx_frame_errors", rx_frame_errors),
> > +	HNS3_NETDEV_STAT("rx_fifo_errors", rx_fifo_errors),
> > +	HNS3_NETDEV_STAT("rx_missed_errors", rx_missed_errors),
> > +
> > +	/* detailed Tx errors */
> > +	HNS3_NETDEV_STAT("tx_aborted_errors", tx_aborted_errors),
> > +	HNS3_NETDEV_STAT("tx_carrier_errors", tx_carrier_errors),
> > +	HNS3_NETDEV_STAT("tx_fifo_errors", tx_fifo_errors),
> > +	HNS3_NETDEV_STAT("tx_heartbeat_errors", tx_heartbeat_errors),
> > +	HNS3_NETDEV_STAT("tx_window_errors", tx_window_errors),
> > +
> 
> Ethtool statistics should be reserved for device specific values and
> should not be used just to clone statistics that already exist in
> the network device.
We saw an example from some of the existing drivers like netronome, apm,
intel etc. in the mainline and so thought this can be done. 
But your point about ethtool to be used only for device specific stats
definitely makes sense to me. 
Do you think We can live with this for now or do you wish to change it?

Many thanks!

Best regards
Salil