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

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

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()
> 
>> +	}
>> +}
>