Re: [PATCH net] ethtool: Embed FEC hist ranges as buffer in struct

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 11/07/2026 00:00, Eric Joyner wrote:
> 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.

I didn't like the idea of dynamic range, FEC is not changing while the
link is UP, I don't see a reason to dynamically reconstruct histogram
bins every single call. And the histogram itself is stable per HW per
FEC, can be constant pre-defined struct in a driver, like in bnxt.

But if dynamic allocation is the only option, then yes, we have to
change this ABI.

> 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>