netdev - Re: [PATCH net-next v3 3/4] net/mlx5e: Add logic to read RS-FEC histogram bin ranges from PPHCR

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: <20250917174638.238fa5fc@kernel.org>
Date: Wed, 17 Sep 2025 17:46:38 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Andrew Lunn <andrew@...n.ch>, Michael Chan <michael.chan@...adcom.com>,
 Pavan Chebbi <pavan.chebbi@...adcom.com>, Tariq Toukan <tariqt@...dia.com>,
 Gal Pressman <gal@...dia.com>, intel-wired-lan@...ts.osuosl.org, Donald
 Hunter <donald.hunter@...il.com>, Carolina Jubran <cjubran@...dia.com>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 netdev@...r.kernel.org, Yael Chemla <ychemla@...dia.com>, Dragos Tatulea
 <dtatulea@...dia.com>
Subject: Re: [PATCH net-next v3 3/4] net/mlx5e: Add logic to read RS-FEC
 histogram bin ranges from PPHCR

On Tue, 16 Sep 2025 19:12:56 +0000 Vadim Fedorenko wrote:
> From: Carolina Jubran <cjubran@...dia.com>
> 
> 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