On Tue, 16 Sep 2025 19:12:56 +0000 Vadim Fedorenko wrote:
> From: Carolina Jubran <[email protected]>
>
> Introduce support for querying the Ports Phy Histogram Configuration
> Register (PPHCR) to retrieve RS-FEC histogram bin ranges. The ranges
> are stored in a static array and will be used to map histogram counters
> to error levels.
>
> The actual RS-FEC histogram statistics are not yet reported in this
> commit and will be handled in a downstream patch.
> @@ -6246,8 +6246,17 @@ int mlx5e_priv_init(struct mlx5e_priv *priv,
> if (!priv->channel_stats)
> goto err_free_tx_rates;
>
> + priv->fec_ranges = kcalloc_node(ETHTOOL_FEC_HIST_MAX,
> + sizeof(*priv->fec_ranges),
> + GFP_KERNEL,
> + node);
Why bother allocating his on the device node? We have no reason to
believe user will pin eth process that reads these stats to the node
where the device is :\
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> index aae0022e8736..476689cb0c1f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> @@ -1490,8 +1490,63 @@ static void fec_set_corrected_bits_total(struct
> mlx5e_priv *priv,
> phy_corrected_bits);
> }
>
> +#define MLX5E_FEC_RS_HIST_MAX 16
> +
> +static u8
> +fec_rs_histogram_fill_ranges(struct mlx5e_priv *priv,
> + const struct ethtool_fec_hist_range **ranges)
> +{
> + struct mlx5_core_dev *mdev = priv->mdev;
> + u32 out[MLX5_ST_SZ_DW(pphcr_reg)] = {0};
> + u32 in[MLX5_ST_SZ_DW(pphcr_reg)] = {0};
> + int sz = MLX5_ST_SZ_BYTES(pphcr_reg);
> + u8 active_hist_type, num_of_bins;
> +
> + memset(priv->fec_ranges, 0,
> + ETHTOOL_FEC_HIST_MAX * sizeof(*priv->fec_ranges));
> + MLX5_SET(pphcr_reg, in, local_port, 1);
> + if (mlx5_core_access_reg(mdev, in, sz, out, sz, MLX5_REG_PPHCR, 0, 0))
> + return 0;
> +
> + active_hist_type = MLX5_GET(pphcr_reg, out, active_hist_type);
> + if (!active_hist_type)
> + return 0;
> +
> + num_of_bins = MLX5_GET(pphcr_reg, out, num_of_bins);
> + if (WARN_ON_ONCE(num_of_bins > MLX5E_FEC_RS_HIST_MAX))
why does MLX5E_FEC_RS_HIST_MAX exist?
We care that bins_cnt <= ETHTOOL_FEC_HIST_MAX - 1
or is there something in the interface that hardcodes 16?
> + return 0;
> @@ -2619,3 +2675,4 @@ unsigned int mlx5e_nic_stats_grps_num(struct mlx5e_priv
> *priv)
> {
> return ARRAY_SIZE(mlx5e_nic_stats_grps);
> }
> +
spurious change
