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: 4+ 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
2026-07-14 7:24 ` Vadim Fedorenko
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.