From: Eric Joyner <eric.joyner@amd.com>
To: <netdev@vger.kernel.org>
Cc: Michael Chan <michael.chan@broadcom.com>,
Pavan Chebbi <pavan.chebbi@broadcom.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
Saeed Mahameed <saeedm@nvidia.com>,
Leon Romanovsky <leon@kernel.org>,
Tariq Toukan <tariqt@nvidia.com>, Mark Bloch <mbloch@nvidia.com>,
Simon Horman <horms@kernel.org>,
Eric Joyner <eric.joyner@amd.com>,
Vadim Fedorenko <vadim.fedorenko@linux.dev>,
Maxime Chevallier <maxime.chevallier@bootlin.com>,
Brett Creeley <brett.creeley@amd.com>,
"Breno Leitao" <leitao@debian.org>,
"Nikhil P. Rao" <nikhil.rao@amd.com>
Subject: [PATCH net] ethtool: Embed FEC hist ranges as buffer in struct
Date: Fri, 10 Jul 2026 16:00:26 -0700 [thread overview]
Message-ID: <20260710230026.47721-1-eric.joyner@amd.com> (raw)
When a driver's .get_fec_stats() handler is called and the driver
supports FEC histogram stats, the driver supplies the histogram bin
ranges via a pointer. This pointer is assigned while under the netdev
ops lock in fec_prepare_data(), but the actual data is only read after
the lock is released; so this allows the driver to change the ranges
(e.g. from another .get_fec_stats() call) while the current call chain
is reading them in fec_fill_reply().
Fix this by embedding a buffer for the driver-supplied ranges in struct
ethtool_fec_hist instead of using a pointer; this ensures there's an
ethtool core-owned consistent copy that can be used after the netdev ops
lock is dropped and later in fec_fill_reply(). While some drivers like
bnxt use a constant struct for their ranges and won't be affected by
this issue, others like mlx5 (and eventually ionic) will use a
dynamically constructed range struct and could potentially run into an
issue.
Since the kernel API changed here, change the in-tree drivers that
report FEC histogram stats to copy their ranges instead of just
supplying a pointer.
Fixes: cc2f08129925 ("ethtool: add FEC bins histogram report")
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
This is a fix for the issue described by Jakub in:
https://lore.kernel.org/netdev/20260615182732.7d28e31a@kernel.org/
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 3 ++-
drivers/net/ethernet/mellanox/mlx5/core/en.h | 1 -
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 -------
.../net/ethernet/mellanox/mlx5/core/en_stats.c | 15 ++++++---------
drivers/net/netdevsim/ethtool.c | 3 ++-
include/linux/ethtool.h | 2 +-
net/ethtool/fec.c | 3 ++-
7 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 62bc9cae613c..2e0ec8543f7f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3291,7 +3291,8 @@ static void bnxt_hwrm_port_phy_fdrstat(struct bnxt *bp,
resp = hwrm_req_hold(bp, req);
rc = hwrm_req_send(bp, req);
if (!rc) {
- hist->ranges = bnxt_fec_ranges;
+ memcpy(hist->ranges, bnxt_fec_ranges,
+ ETHTOOL_FEC_HIST_MAX * sizeof(*hist->ranges));
for (i = 0; i <= 15; i++) {
__le64 sum = resp->accumulated_codewords_err_s[i];
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index d507289096c2..6867a5aed42c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -984,7 +984,6 @@ struct mlx5e_priv {
struct mlx5e_mqprio_rl *mqprio_rl;
struct dentry *dfs_root;
struct mlx5_devcom_comp_dev *devcom;
- struct ethtool_fec_hist_range *fec_ranges;
};
static inline u16 mlx5e_stats_nch_read(const struct mlx5e_priv *priv)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index aa8610cedaa8..8db235ac9ae4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -6415,14 +6415,8 @@ int mlx5e_priv_init(struct mlx5e_priv *priv,
if (!priv->channel_stats)
goto err_free_tx_rates;
- priv->fec_ranges = kzalloc_objs(*priv->fec_ranges, ETHTOOL_FEC_HIST_MAX);
- if (!priv->fec_ranges)
- goto err_free_channel_stats;
-
return 0;
-err_free_channel_stats:
- kfree(priv->channel_stats);
err_free_tx_rates:
kfree(priv->tx_rates);
err_free_txq2sq_stats:
@@ -6447,7 +6441,6 @@ void mlx5e_priv_cleanup(struct mlx5e_priv *priv)
if (!priv->mdev)
return;
- kfree(priv->fec_ranges);
for (i = 0; i < priv->stats_nch; i++)
kvfree(priv->channel_stats[i]);
kfree(priv->channel_stats);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index de38b60806c2..5f45e3c77cf1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -1550,7 +1550,7 @@ static bool fec_rs_validate_hist_type(int mode, int hist_type)
static u8
fec_rs_histogram_fill_ranges(struct mlx5e_priv *priv, int mode,
- const struct ethtool_fec_hist_range **ranges)
+ struct ethtool_fec_hist_range *ranges)
{
struct mlx5_core_dev *mdev = priv->mdev;
u32 out[MLX5_ST_SZ_DW(pphcr_reg)] = {0};
@@ -1558,8 +1558,6 @@ fec_rs_histogram_fill_ranges(struct mlx5e_priv *priv, int mode,
int sz = MLX5_ST_SZ_BYTES(pphcr_reg);
u8 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;
@@ -1575,12 +1573,11 @@ fec_rs_histogram_fill_ranges(struct mlx5e_priv *priv, int mode,
for (int i = 0; i < num_of_bins; i++) {
void *bin_range = MLX5_ADDR_OF(pphcr_reg, out, bin_range[i]);
- priv->fec_ranges[i].high = MLX5_GET(bin_range_layout, bin_range,
- high_val);
- priv->fec_ranges[i].low = MLX5_GET(bin_range_layout, bin_range,
- low_val);
+ ranges[i].high = MLX5_GET(bin_range_layout, bin_range,
+ high_val);
+ ranges[i].low = MLX5_GET(bin_range_layout, bin_range,
+ low_val);
}
- *ranges = priv->fec_ranges;
return num_of_bins;
}
@@ -1622,7 +1619,7 @@ static void fec_set_histograms_stats(struct mlx5e_priv *priv, int mode,
case MLX5E_FEC_LLRS_272_257_1:
case MLX5E_FEC_RS_544_514_INTERLEAVED_QUAD:
num_of_bins =
- fec_rs_histogram_fill_ranges(priv, mode, &hist->ranges);
+ fec_rs_histogram_fill_ranges(priv, mode, hist->ranges);
if (num_of_bins)
return fec_rs_histogram_fill_stats(priv, num_of_bins,
hist);
diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 025ea79879f3..69849e4192d4 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -178,7 +178,8 @@ nsim_get_fec_stats(struct net_device *dev, struct ethtool_fec_stats *fec_stats,
{
struct ethtool_fec_hist_value *values = hist->values;
- hist->ranges = netdevsim_fec_ranges;
+ memcpy(hist->ranges, netdevsim_fec_ranges,
+ ARRAY_SIZE(netdevsim_fec_ranges) * sizeof(*hist->ranges));
fec_stats->corrected_blocks.total = 123;
fec_stats->uncorrectable_blocks.total = 4;
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 5d491a98265e..d3bf1b2ddecc 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -561,7 +561,7 @@ struct ethtool_fec_hist {
u64 sum;
u64 per_lane[ETHTOOL_MAX_LANES];
} values[ETHTOOL_FEC_HIST_MAX];
- const struct ethtool_fec_hist_range *ranges;
+ struct ethtool_fec_hist_range ranges[ETHTOOL_FEC_HIST_MAX];
};
/**
* struct ethtool_fec_stats - statistics for IEEE 802.3 FEC
diff --git a/net/ethtool/fec.c b/net/ethtool/fec.c
index e2d539271060..28373e16dd51 100644
--- a/net/ethtool/fec.c
+++ b/net/ethtool/fec.c
@@ -186,7 +186,8 @@ static int fec_put_hist(struct sk_buff *skb,
int i, j;
u64 sum;
- if (!ranges)
+ if (values[0].sum == ETHTOOL_STAT_NOT_SET &&
+ values[0].per_lane[0] == ETHTOOL_STAT_NOT_SET)
return 0;
for (i = 0; i < ETHTOOL_FEC_HIST_MAX; i++) {
--
2.17.1
next reply other threads:[~2026-07-10 23:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-10 23:00 Eric Joyner [this message]
2026-07-11 21:03 ` [PATCH net] ethtool: Embed FEC hist ranges as buffer in struct Vadim Fedorenko
2026-07-13 22:37 ` Eric Joyner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260710230026.47721-1-eric.joyner@amd.com \
--to=eric.joyner@amd.com \
--cc=andrew+netdev@lunn.ch \
--cc=brett.creeley@amd.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=leitao@debian.org \
--cc=leon@kernel.org \
--cc=maxime.chevallier@bootlin.com \
--cc=mbloch@nvidia.com \
--cc=michael.chan@broadcom.com \
--cc=netdev@vger.kernel.org \
--cc=nikhil.rao@amd.com \
--cc=pabeni@redhat.com \
--cc=pavan.chebbi@broadcom.com \
--cc=saeedm@nvidia.com \
--cc=tariqt@nvidia.com \
--cc=vadim.fedorenko@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox