[PATCH net-next v2] ionic: Add .get_fec_stats ethtool handler

[PATCH net-next v2] ionic: Add .get_fec_stats ethtool handler

[Eric Joyner]

Reports FEC statistics totals and an 802.3ck FEC histogram. Per-lane
counts currently aren't supported. Every counter reported here comes
from the RS decoder, including fec_corrected_bits_total despite its
name, and the only histogram format firmware produces is the 16-bin one
from RS(544,514), so nothing is reported unless RS is the active FEC
mode.

These are physical port counters, so virtual functions are skipped the
same way ionic_get_link_ext_stats() skips them, rather than reporting the
port's counters as if they belonged to the VF.

The reporting of these statistics is gated by DEV_CAP_EXTRA_STATS and
checks for IONIC_STAT_INVALID, since only the newest devices support
reporting all of these stats. Older devices can only report some of the
statistics or not at all, and so the output will properly exclude those
unsupported statistics.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
Reposting since v1 was too old to apply. This also addresses the netdev
AI review comments left on v1:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260731214021.15279-1-eric.joyner@amd.com

That review also flagged a NULL dereference in ionic_get_fecparam().
It predates this patch, so it is fixed separately in the net tree
rather than here:
  [PATCH net 0/3] ionic: fix port_info lifetime problems around device reset
  https://lore.kernel.org/netdev/20260814-ionic-port-info-lifetime-v1-0-f73b1a06c5f6@amd.com/T/#t

Vadim's Reviewed-by from v1 is retained. The only change to the code
since he reviewed it is the histogram bin latch listed below.
---
Changes in v2:
- Latch each histogram bin into a local before testing it against
  IONIC_STAT_INVALID, so the sentinel check and the store cannot be
  folded into two separate reads of the firmware-updated DMA buffer.
- Explain in the commit message why the RS FEC check gates every
  reported counter rather than just the histogram: they all come from
  the RS decoder, including fec_corrected_bits_total despite its name.
- Rebased onto current net-next.
- Link to v1: https://lore.kernel.org/r/20260731214021.15279-1-eric.joyner@amd.com
---
 .../net/ethernet/pensando/ionic/ionic_ethtool.c    | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index c4ab4b5caa0a..2f2b5076dcf4 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -441,6 +441,91 @@ static int ionic_get_fecparam(struct net_device *netdev,
 	return 0;
 }
 
+#define IONIC_FEC_STAT(dst, src)			\
+	do {						\
+		__le64 __val = (src);			\
+							\
+		if (__val != IONIC_STAT_INVALID)	\
+			(dst) = le64_to_cpu(__val);	\
+	} while (0)
+
+static const struct ethtool_fec_hist_range ionic_fec_hist_ranges[] = {
+	{ 0, 0},
+	{ 1, 1},
+	{ 2, 2},
+	{ 3, 3},
+	{ 4, 4},
+	{ 5, 5},
+	{ 6, 6},
+	{ 7, 7},
+	{ 8, 8},
+	{ 9, 9},
+	{ 10, 10},
+	{ 11, 11},
+	{ 12, 12},
+	{ 13, 13},
+	{ 14, 14},
+	{ 15, 15},
+	{ 0, 0},
+};
+
+static void
+ionic_fill_fec_hist(const struct ionic_port_extra_stats *port_extra_stats,
+		    struct ethtool_fec_hist *hist)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(port_extra_stats->fec_codeword_error_bin); i++) {
+		__le64 val = port_extra_stats->fec_codeword_error_bin[i];
+
+		if (val == IONIC_STAT_INVALID)
+			return;
+
+		hist->values[i].sum = le64_to_cpu(val);
+	}
+
+	hist->ranges = ionic_fec_hist_ranges;
+}
+
+static void ionic_get_fec_stats(struct net_device *netdev,
+				struct ethtool_fec_stats *fec_stats,
+				struct ethtool_fec_hist *hist)
+{
+	struct ionic_port_extra_stats port_extra_stats;
+	struct ionic_lif *lif = netdev_priv(netdev);
+	struct ionic_port_info *port_info;
+
+	if (lif->ionic->pdev->is_virtfn)
+		return;
+
+	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
+		return;
+
+	if (!(lif->ionic->ident.dev.capabilities &
+	      cpu_to_le64(IONIC_DEV_CAP_EXTRA_STATS)))
+		return;
+
+	port_info = lif->ionic->idev.port_info;
+	if (!port_info) {
+		netdev_err_once(netdev, "port_info not initialized\n");
+		return;
+	}
+
+	if (port_info->config.fec_type != IONIC_PORT_FEC_TYPE_RS)
+		return;
+
+	port_extra_stats = port_info->extra_stats;
+
+	IONIC_FEC_STAT(fec_stats->corrected_blocks.total,
+		       port_extra_stats.rsfec_correctable_blocks);
+	IONIC_FEC_STAT(fec_stats->uncorrectable_blocks.total,
+		       port_extra_stats.rsfec_uncorrectable_blocks);
+	IONIC_FEC_STAT(fec_stats->corrected_bits.total,
+		       port_extra_stats.fec_corrected_bits_total);
+
+	ionic_fill_fec_hist(&port_extra_stats, hist);
+}
+
 static int ionic_set_fecparam(struct net_device *netdev,
 			      struct ethtool_fecparam *fec)
 {
@@ -1177,6 +1262,7 @@ static const struct ethtool_ops ionic_ethtool_ops = {
 	.get_module_eeprom_by_page	= ionic_get_module_eeprom_by_page,
 	.get_pauseparam		= ionic_get_pauseparam,
 	.set_pauseparam		= ionic_set_pauseparam,
+	.get_fec_stats		= ionic_get_fec_stats,
 	.get_fecparam		= ionic_get_fecparam,
 	.set_fecparam		= ionic_set_fecparam,
 	.get_ts_info		= ionic_get_ts_info,

---
base-commit: e6a5d573d24cd375e09d24f136523cb3cc85c9d3
change-id: 20260814-ionic-get-fec-stats-990736964908

Best regards,
-- 
Eric Joyner <eric.joyner@amd.com>

Re: [PATCH net-next v2] ionic: Add .get_fec_stats ethtool handler

[Simon Horman]

On Fri, Aug 14, 2026 at 05:33:12PM -0700, Eric Joyner wrote:
> Reports FEC statistics totals and an 802.3ck FEC histogram. Per-lane
> counts currently aren't supported. Every counter reported here comes
> from the RS decoder, including fec_corrected_bits_total despite its
> name, and the only histogram format firmware produces is the 16-bin one
> from RS(544,514), so nothing is reported unless RS is the active FEC
> mode.
> 
> These are physical port counters, so virtual functions are skipped the
> same way ionic_get_link_ext_stats() skips them, rather than reporting the
> port's counters as if they belonged to the VF.
> 
> The reporting of these statistics is gated by DEV_CAP_EXTRA_STATS and
> checks for IONIC_STAT_INVALID, since only the newest devices support
> reporting all of these stats. Older devices can only report some of the
> statistics or not at all, and so the output will properly exclude those
> unsupported statistics.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Eric Joyner <eric.joyner@amd.com>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> ---
> Reposting since v1 was too old to apply. This also addresses the netdev
> AI review comments left on v1:
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260731214021.15279-1-eric.joyner@amd.com
> 
> That review also flagged a NULL dereference in ionic_get_fecparam().
> It predates this patch, so it is fixed separately in the net tree
> rather than here:
>   [PATCH net 0/3] ionic: fix port_info lifetime problems around device reset
>   https://lore.kernel.org/netdev/20260814-ionic-port-info-lifetime-v1-0-f73b1a06c5f6@amd.com/T/#t
> 
> Vadim's Reviewed-by from v1 is retained. The only change to the code
> since he reviewed it is the histogram bin latch listed below.
> ---
> Changes in v2:
> - Latch each histogram bin into a local before testing it against
>   IONIC_STAT_INVALID, so the sentinel check and the store cannot be
>   folded into two separate reads of the firmware-updated DMA buffer.
> - Explain in the commit message why the RS FEC check gates every
>   reported counter rather than just the histogram: they all come from
>   the RS decoder, including fec_corrected_bits_total despite its name.
> - Rebased onto current net-next.
> - Link to v1: https://lore.kernel.org/r/20260731214021.15279-1-eric.joyner@amd.com
> ---
>  .../net/ethernet/pensando/ionic/ionic_ethtool.c    | 86 ++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index c4ab4b5caa0a..2f2b5076dcf4 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -441,6 +441,91 @@ static int ionic_get_fecparam(struct net_device *netdev,
>  	return 0;
>  }
>  
> +#define IONIC_FEC_STAT(dst, src)			\
> +	do {						\
> +		__le64 __val = (src);			\
> +							\
> +		if (__val != IONIC_STAT_INVALID)	\
> +			(dst) = le64_to_cpu(__val);	\
> +	} while (0)

Hi Eric,

There is an AI-generated review of this patch available at
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260814-ionic-get-fec-stats-v2-1-13f0e7d2e702%40amd.com

In that review, the following stands out to me.
I'd appreciate it if you could take a look over it in particular.

  The v2 changelog says the per-bin local "latches" the value so "the
  sentinel check and the store cannot be folded into two separate reads of
  the firmware-updated DMA buffer".  Is that guaranteed by this code?

  Both this macro and the loop body in ionic_fill_fec_hist() use plain,
  non-volatile loads:

	__le64 __val = (src);
	if (__val != IONIC_STAT_INVALID)
		(dst) = le64_to_cpu(__val);

  and

	__le64 val = port_extra_stats->fec_codeword_error_bin[i];
	if (val == IONIC_STAT_INVALID)
		return;
	hist->values[i].sum = le64_to_cpu(val);

  Nothing stops the compiler from discarding the local and re-loading the
  word from the coherent buffer for the comparison and again for the store.

  Would READ_ONCE() on each word, or a memcpy() of the block, be needed to
  actually get the single-read property described in the changelog?

  The same applies to the struct assignment in ionic_get_fec_stats():

	port_extra_stats = port_info->extra_stats;

  This is an ordinary aggregate copy out of memory the device writes
  asynchronously, so members may be re-read at each later use rather than
  coming from a stack snapshot.

  If a word transitions while being read, can the 0xffffffffffffffff
  sentinel that ionic_port_init() memsets into extra_stats be reported to
  user space as a real counter, or a counter firmware did fill in be
  dropped, and can the 16 histogram bins end up stitched together from
  different firmware snapshots?

...