[PATCH net-next] mlxsw: spectrum_ethtool: expose per-PG rx_discards

[PATCH net-next] mlxsw: spectrum_ethtool: expose per-PG rx_discards

[Keno Fischer]

PPCNT group 0x10 (per-priority counters) carries an rx_discards field at
offset 0x78. These counters aggregate up into if_in_discards, but don't
show up anywhere else. Since there are many things that aggregate into
`if_in_discards`, having these counters helps distinguish what caused
those discards (in my case they were caused by headroom buffer overruns
due to inappropriately configured buffer sizes).

Of note, from emperical testing, these counter are per-"priority group"
(PG) not per-"switch priority". It's a bit confusing, because the rest
of these counter are per-"switch priority" and the header file calls
these "Per Priority Group Counters". However, that should be read as
"(Per Priority) Group Counters", not "Per (Priority Group) Counters".

I attempted to distinguish this in the counter naming by calling these
`rx_discards_pg_N` rather than `rx_discards_prio_N` (which is the
naming scheme of the other counters in this PPCNT group).

I will also note that the mlx5 driver (which already has this counter)
uses the schme `rx_prioN_discards` (and same for the other counters
in this group). However, I was unable to determine whether the mlx5
counters behave the same as the mlxsw counters with respect to PG
mapping. An attempt to remap to a different PG there did not change
which counter incremented, but the mlx5 configuration code is quite
different, so it's possible the remapping needs to be done differently.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Keno Fischer <keno@juliahub.com>
---
 drivers/net/ethernet/mellanox/mlxsw/reg.h              | 8 ++++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 3bb89045eaf5..01fcbd1c8d01 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5651,6 +5651,14 @@ MLXSW_ITEM64(reg, ppcnt, tx_pause_duration,
 MLXSW_ITEM64(reg, ppcnt, tx_pause_transition,
 	     MLXSW_REG_PPCNT_COUNTERS_OFFSET + 0x70, 0, 64);
 
+/* reg_ppcnt_rx_discards
+ * Per-PG (ingress priority group) drop count.
+ * index slot is the PG, switch priority.
+ * Access: RO
+ */
+MLXSW_ITEM64(reg, ppcnt, rx_discards,
+	     MLXSW_REG_PPCNT_COUNTERS_OFFSET + 0x78, 0, 64);
+
 /* Ethernet Per Traffic Class Counters */
 
 /* reg_ppcnt_tc_transmit_queue
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index 0a8fb9c842d3..7f78b1ef61cc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -542,6 +542,10 @@ static struct mlxsw_sp_port_hw_stats mlxsw_sp_port_hw_prio_stats[] = {
 		.str = "tx_pause_duration_prio",
 		.getter = mlxsw_reg_ppcnt_tx_pause_duration_get,
 	},
+	{
+		.str = "rx_discards_pg",
+		.getter = mlxsw_reg_ppcnt_rx_discards_get,
+	},
 };
 
 #define MLXSW_SP_PORT_HW_PRIO_STATS_LEN ARRAY_SIZE(mlxsw_sp_port_hw_prio_stats)
-- 
2.54.0

Re: [PATCH net-next] mlxsw: spectrum_ethtool: expose per-PG rx_discards

[Petr Machata]

Keno Fischer <keno@juliahub.com> writes:

> PPCNT group 0x10 (per-priority counters) carries an rx_discards field at
> offset 0x78. These counters aggregate up into if_in_discards, but don't
> show up anywhere else. Since there are many things that aggregate into
> `if_in_discards`, having these counters helps distinguish what caused
> those discards (in my case they were caused by headroom buffer overruns
> due to inappropriately configured buffer sizes).
>
> Of note, from emperical testing, these counter are per-"priority group"
> (PG) not per-"switch priority". It's a bit confusing, because the rest
> of these counter are per-"switch priority" and the header file calls
> these "Per Priority Group Counters". However, that should be read as
> "(Per Priority) Group Counters", not "Per (Priority Group) Counters".

I think it's (Per-Priority Group) Counters. As in group of counters that
are per priority. Similarly there's the "Ethernet Discard Counter Group
Counters". These names come from the HW manual.

> I attempted to distinguish this in the counter naming by calling these
> `rx_discards_pg_N` rather than `rx_discards_prio_N` (which is the
> naming scheme of the other counters in this PPCNT group).
>
> I will also note that the mlx5 driver (which already has this counter)
> uses the schme `rx_prioN_discards` (and same for the other counters
> in this group). However, I was unable to determine whether the mlx5
> counters behave the same as the mlxsw counters with respect to PG
> mapping. An attempt to remap to a different PG there did not change
> which counter incremented, but the mlx5 configuration code is quite
> different, so it's possible the remapping needs to be done differently.
>
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Keno Fischer <keno@juliahub.com>

LGTM, thanks!
Reviewed-by: Petr Machata <petrm@nvidia.com>