netdev - Re: [RESEND PATCH net-next v2 1/3] net: libwx: support hardware 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] [day] [month] [year] [list]

Message-ID: <20231004164606.31e98eb6@kernel.org>
Date: Wed, 4 Oct 2023 16:46:06 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
 pabeni@...hat.com, andrew@...n.ch, mengyuanlou@...-swift.com
Subject: Re: [RESEND PATCH net-next v2 1/3] net: libwx: support hardware
 statistics

On Wed, 27 Sep 2023 14:14:55 +0800 Jiawen Wu wrote:
> +static const struct wx_stats wx_gstrings_stats[] = {
> +	WX_NETDEV_STAT("rx_packets", rx_packets),
> +	WX_NETDEV_STAT("tx_packets", tx_packets),
> +	WX_NETDEV_STAT("rx_bytes", rx_bytes),
> +	WX_NETDEV_STAT("tx_bytes", tx_bytes),
> +	WX_NETDEV_STAT("multicast", multicast),
> +	WX_NETDEV_STAT("rx_errors", rx_errors),
> +	WX_NETDEV_STAT("rx_length_errors", rx_length_errors),
> +	WX_NETDEV_STAT("rx_crc_errors", rx_crc_errors),

Please don't report standard netdev statistics in ethtool -S.
Users can get them thru any of the (too) many standard APIs.

> +	WX_STAT("rx_pkts_nic", stats.gprc),
> +	WX_STAT("tx_pkts_nic", stats.gptc),
> +	WX_STAT("rx_bytes_nic", stats.gorc),
> +	WX_STAT("tx_bytes_nic", stats.gotc),
> +	WX_STAT("rx_total_pkts", stats.tpr),
> +	WX_STAT("tx_total_pkts", stats.tpt),
> +	WX_STAT("rx_broadcast", stats.bprc),
> +	WX_STAT("tx_broadcast", stats.bptc),
> +	WX_STAT("rx_multicast", stats.mprc),
> +	WX_STAT("tx_multicast", stats.mptc),
> +	WX_STAT("rx_long_length_count", stats.roc),
> +	WX_STAT("rx_short_length_count", stats.ruc),
> +	WX_STAT("rx_flow_control_xon_xoff", stats.lxonoffrxc),
> +	WX_STAT("tx_flow_control_xon", stats.lxontxc),
> +	WX_STAT("tx_flow_control_xoff", stats.lxofftxc),

Please take a look at the statistics defined in ethtool.h
Look for callbacks in struct ethtool_ops with "stats" in the name.
Anything that matches one of the callbacks should go into those
APIs no to ethtool -S.

> +	WX_STAT("os2bmc_rx_by_bmc", stats.o2bgptc),
> +	WX_STAT("os2bmc_tx_by_bmc", stats.b2ospc),
> +	WX_STAT("os2bmc_tx_by_host", stats.o2bspc),
> +	WX_STAT("os2bmc_rx_by_host", stats.b2ogprc),
> +	WX_STAT("rx_no_dma_resources", stats.rdmdrop),
> +	WX_STAT("tx_busy", tx_busy),
> +	WX_STAT("non_eop_descs", non_eop_descs),
> +	WX_STAT("tx_restart_queue", restart_queue),
> +	WX_STAT("rx_csum_offload_good_count", hw_csum_rx_good),
> +	WX_STAT("rx_csum_offload_errors", hw_csum_rx_error),
> +	WX_STAT("alloc_rx_buff_failed", alloc_rx_buff_failed),
> +};
> +
> +static const char wx_gstrings_test[][ETH_GSTRING_LEN] = {
> +	"Register test  (offline)", "Eeprom test    (offline)",
> +	"Interrupt test (offline)", "Loopback test  (offline)",
> +	"Link test   (on/offline)"
> +};

You said this adds stats. Why are strings for tests here?
-- 
pw-bot: cr