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:

[...]

quoted

+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/

quoted

+	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),

quoted

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

quoted

+	}
+}