netdev - Re: [PATCH net-next v5 14/15] idpf: add ethtool callbacks

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: <53a996f1-402e-dea8-9c08-51b84d02d0ac@intel.com>
Date: Sat, 19 Aug 2023 00:42:56 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>
CC: <davem@...emloft.net>, <pabeni@...hat.com>, <edumazet@...gle.com>,
	<netdev@...r.kernel.org>, Alan Brady <alan.brady@...el.com>,
	<pavan.kumar.linga@...el.com>, <emil.s.tantilov@...el.com>,
	<jesse.brandeburg@...el.com>, <sridhar.samudrala@...el.com>,
	<shiraz.saleem@...el.com>, <sindhu.devale@...el.com>, <willemb@...gle.com>,
	<decot@...gle.com>, <andrew@...n.ch>, <leon@...nel.org>, <mst@...hat.com>,
	<simon.horman@...igine.com>, <shannon.nelson@....com>,
	<stephen@...workplumber.org>, Alice Michael <alice.michael@...el.com>,
	"Joshua Hay" <joshua.a.hay@...el.com>, Phani Burra <phani.r.burra@...el.com>
Subject: Re: [PATCH net-next v5 14/15] idpf: add ethtool callbacks

On 8/18/23 20:58, Jakub Kicinski wrote:
> On Tue, 15 Aug 2023 17:43:04 -0700 Tony Nguyen wrote:

[...]

> 
>> +static const struct idpf_stats idpf_gstrings_port_stats[] = {
>> +	IDPF_PORT_STAT("rx-csum_errors", port_stats.rx_hw_csum_err),
>> +	IDPF_PORT_STAT("rx-hsplit", port_stats.rx_hsplit),
>> +	IDPF_PORT_STAT("rx-hsplit_hbo", port_stats.rx_hsplit_hbo),
>> +	IDPF_PORT_STAT("rx-bad_descs", port_stats.rx_bad_descs),
>> +	IDPF_PORT_STAT("rx-length_errors", port_stats.vport_stats.rx_invalid_frame_length),
>> +	IDPF_PORT_STAT("tx-skb_drops", port_stats.tx_drops),
>> +	IDPF_PORT_STAT("tx-dma_map_errs", port_stats.tx_dma_map_errs),
>> +	IDPF_PORT_STAT("tx-linearized_pkts", port_stats.tx_linearize),
>> +	IDPF_PORT_STAT("tx-busy_events", port_stats.tx_busy),
>> +	IDPF_PORT_STAT("rx_bytes", port_stats.vport_stats.rx_bytes),
>> +	IDPF_PORT_STAT("rx-unicast_pkts", port_stats.vport_stats.rx_unicast),
>> +	IDPF_PORT_STAT("rx-multicast_pkts", port_stats.vport_stats.rx_multicast),
>> +	IDPF_PORT_STAT("rx-broadcast_pkts", port_stats.vport_stats.rx_broadcast),
> 
> how are the basic stats different form the base stats reported via
> if_link?
> 
> Also what's up with the mix of - and _ in the names?

I see that here we (Intel) attempt for the first time to propose our 
"Unified stats" naming scheme [1].

Purpose is to have:
- common naming scheme (at least for the ice we have patch ~ready);
- less "customer frustration";
- easier job for analytical scripts, copying from wiki:
| The naming schema was created to be human readable and easily parsed
| by an analytic engine (such as a script or other entity).
| All statistic strings will be comprised of three components:
| @Where, @Instance and @Units.  Each of these components is separated
| by an underscore "_"; if a component is comprised of more than one
| word, then those words are separated by a dash "-".
|
| An example statistic that shows this is xdp-rx-dropped_q-23_packets.
| In this case the @where is xdp-rx-dropped, the @instance is q-32 and
| the @unit is packets.

Public wiki is unfortunately present only for our OOT driver:
[1] https://sourceforge.net/p/e1000/wiki/UnifiedStats/

> 
>> +	IDPF_PORT_STAT("rx-unknown_protocol", port_stats.vport_stats.rx_unknown_protocol),
>> +	IDPF_PORT_STAT("tx_bytes", port_stats.vport_stats.tx_bytes),
>> +	IDPF_PORT_STAT("tx-unicast_pkts", port_stats.vport_stats.tx_unicast),
>> +	IDPF_PORT_STAT("tx-multicast_pkts", port_stats.vport_stats.tx_multicast),
>> +	IDPF_PORT_STAT("tx-broadcast_pkts", port_stats.vport_stats.tx_broadcast),
>> +	IDPF_PORT_STAT("tx_errors", port_stats.vport_stats.tx_errors),
> 
>> +static void idpf_add_stat_strings(u8 **p, const struct idpf_stats *stats,
>> +				  const unsigned int size)
>> +{
>> +	unsigned int i;
>> +
>> +	for (i = 0; i < size; i++) {
>> +		snprintf((char *)*p, ETH_GSTRING_LEN, "%.32s",
>> +			 stats[i].stat_string);
>> +		*p += ETH_GSTRING_LEN;
> 
> ethtool_sprintf()
> 
>> +	}
>> +}
>