[PATCH net-next v2 0/5] Expose more port stats to ethtool

[PATCH net-next v2 0/5] Expose more port stats to ethtool

[Eric Joyner]

Newer hardware collects a lot more FEC statistics than older hardware;
these include FEC histograms and corrected/uncorrected word and bit
totals. This patchset adds plumbing to pass these through to ethtool
along with another link_down_count stat that is another port-level stat.
That link_down_count was already being sent to the driver by the
firmware; it just wasn't used.

Brett's patch is a small unrelated improvement to devcmd handling that's
still nice to have and will help enable deferred probe functionality.

---
v2:
- Add missing cpu_to_le64() to FEC histogram stat assignment
- Remove unused pb_stats field that's replaced by the new FEC/extra stats
- Replace ethtool ext link stat with firmware stat instead of adding
  the firmware stat to general ethtool statistics; remove old driver
  calculated stat
- Add explanation for what EAGAIN return value could be used for in
  commit message

Brett Creeley (1):
  ionic: Small improvements in devcmd retry logic

Eric Joyner (4):
  ionic: Get "link_down_count" ext link stat from firmware
  ionic: Update ionic_if.h with new extra port stats structure
  ionic: Report rx_bits_phy stat to ethtool
  ionic: Add .get_fec_stats ethtool handler

 .../ethernet/pensando/ionic/ionic_ethtool.c   | 55 ++++++++++++++++++-
 .../net/ethernet/pensando/ionic/ionic_if.h    | 36 ++++--------
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  1 -
 .../net/ethernet/pensando/ionic/ionic_lif.h   |  1 -
 .../net/ethernet/pensando/ionic/ionic_main.c  |  7 ++-
 .../net/ethernet/pensando/ionic/ionic_stats.c | 15 ++++-
 6 files changed, 84 insertions(+), 31 deletions(-)


base-commit: 8c699be3dad7bba87cdda485dc099226cfc2f706
-- 
2.17.1

[PATCH net-next v2 1/5] ionic: Small improvements in devcmd retry logic

[Eric Joyner]

From: Brett Creeley <brett.creeley@amd.com>

If the timeout time is hit when the last attempt returned EAGAIN, the
driver returns -ETIMEDOUT. This causes the -EAGAIN result to be lost.
Fix this by returning -EAGAIN if the timeout time is hit and the
previous result matches. Another commit uses this return value to help
signal that a deferred probe should be performed due to the firmware not
being ready, which is why it would return EAGAIN.

Also, reduce the sleep between the write to done and doorbell registers.
The msleep(1000) was initially added in an arbitrary manner. However,
this long of a sleep is problematic because it reduces the number of
retries when -EAGAIN is returned, which may result in the devmcd giving
up early due to the timeout. Fix this by reducing the sleep to
msleep(50).

Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 3c5200e2fdb7..92f2ec0bd5af 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -554,6 +554,11 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
 
 	if (!done && !time_before(jiffies, max_wait)) {
 		ionic_dev_cmd_clean(ionic);
+
+		/* allow caller to manage EAGAIN from previous attempt */
+		if (err == IONIC_RC_EAGAIN)
+			return -EAGAIN;
+
 		dev_warn(ionic->dev, "DEVCMD %s (%d) timeout after %ld secs\n",
 			 ionic_opcode_to_str(opcode), opcode, max_seconds);
 		return -ETIMEDOUT;
@@ -568,7 +573,7 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
 				ionic_error_to_str(err), err);
 
 			iowrite32(0, &idev->dev_cmd_regs->done);
-			msleep(1000);
+			msleep(50);
 			iowrite32(1, &idev->dev_cmd_regs->doorbell);
 			goto try_again;
 		}
-- 
2.17.1

Re: [PATCH net-next v2 1/5] ionic: Small improvements in devcmd retry logic

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ionic: Small improvements in devcmd retry logic

This commit aims to preserve the -EAGAIN error code when a device command
times out on its final retry. It also reduces the sleep time between retries
from 1000ms to 50ms to allow more attempts before timing out.

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> index 3c5200e2fdb7d..92f2ec0bd5af9 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> @@ -554,6 +554,11 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
>  
>  	if (!done && !time_before(jiffies, max_wait)) {
>  		ionic_dev_cmd_clean(ionic);
> +
> +		/* allow caller to manage EAGAIN from previous attempt */
> +		if (err == IONIC_RC_EAGAIN)
> +			return -EAGAIN;

Could err be uninitialized here?

Looking at __ionic_dev_cmd_wait(), err is declared at the start of the
function but is not assigned until later via ionic_dev_cmd_status(). If the
initial wait loop times out on the very first attempt (for example, if the
firmware is unresponsive or the system stalls), execution falls through to
this block. Checking err here would read an uninitialized stack variable,
potentially leading to unpredictable driver initialization behavior.

> +
>  		dev_warn(ionic->dev, "DEVCMD %s (%d) timeout after %ld secs\n",
>  			 ionic_opcode_to_str(opcode), opcode, max_seconds);
>  		return -ETIMEDOUT;

[ ... ]

> @@ -568,7 +573,7 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
>  				ionic_error_to_str(err), err);
>  
>  			iowrite32(0, &idev->dev_cmd_regs->done);
> -			msleep(1000);
> +			msleep(50);

Does this logic abandon retries prematurely?

The surrounding retry condition still requires at least one full second of
remaining time:

    if (err == IONIC_RC_EAGAIN &&
        time_before(jiffies, (max_wait - HZ))) {

The - HZ check was likely originally added to ensure there was enough time
left for the 1000ms sleep. Now that the sleep is reduced to 50ms, requiring
a full second of remaining time means the driver will abort retries during
the final second of the timeout. If max_seconds is small, it may not retry
at all, which seems to contradict the intent of increasing the number of
retries.

>  			iowrite32(1, &idev->dev_cmd_regs->doorbell);
>  			goto try_again;
>  		}
-- 
pw-bot: cr

[PATCH net-next v2 2/5] ionic: Get "link_down_count" ext link stat from firmware

[Eric Joyner]

The number of times that link has gone down at the port level is tracked
by the firmware and sent to the driver via regular DMA writes to an
instance of struct ionic_port_status in the driver's memory.

This statistic was never reported in favor of a driver-derived stat, but
doing it in the driver was never necessary since firmware had been
reporting it the whole time. Since it would be more accurate and true to
the description of the statistic to get this count at the PHY level,
replace the driver-calculated statistic with the firmware one and remove
the driver-calculated one entirely.

Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 3 ++-
 drivers/net/ethernet/pensando/ionic/ionic_lif.c     | 1 -
 drivers/net/ethernet/pensando/ionic/ionic_lif.h     | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index 78a802eb159f..af0c4cc8ad8e 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -117,7 +117,8 @@ static void ionic_get_link_ext_stats(struct net_device *netdev,
 	struct ionic_lif *lif = netdev_priv(netdev);
 
 	if (lif->ionic->pdev->is_physfn)
-		stats->link_down_events = lif->link_down_count;
+		stats->link_down_events =
+			    lif->ionic->idev.port_info->status.link_down_count;
 }
 
 static int ionic_get_link_ksettings(struct net_device *netdev,
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 637e635bbf03..eb7e552bc12e 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -179,7 +179,6 @@ static void ionic_link_status_check(struct ionic_lif *lif)
 		}
 	} else {
 		if (netif_carrier_ok(netdev)) {
-			lif->link_down_count++;
 			netdev_info(netdev, "Link down\n");
 			netif_carrier_off(netdev);
 		}
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index 8e10f66dc50e..d34692462036 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -214,7 +214,6 @@ struct ionic_lif {
 	bool registered;
 	bool doorbell_wa;
 	u16 lif_type;
-	unsigned int link_down_count;
 	unsigned int nmcast;
 	unsigned int nucast;
 	unsigned int nvlans;
-- 
2.17.1

Re: [PATCH net-next v2 2/5] ionic: Get "link_down_count" ext link stat from firmware

[Jakub Kicinski]

On Tue, 5 May 2026 21:35:22 -0700 Eric Joyner wrote:
> The number of times that link has gone down at the port level is tracked
> by the firmware and sent to the driver via regular DMA writes to an
> instance of struct ionic_port_status in the driver's memory.
> 
> This statistic was never reported in favor of a driver-derived stat, but
> doing it in the driver was never necessary since firmware had been
> reporting it the whole time. Since it would be more accurate and true to
> the description of the statistic to get this count at the PHY level,
> replace the driver-calculated statistic with the firmware one and remove
> the driver-calculated one entirely.

missing le16_to_cpu() ?

drivers/net/ethernet/pensando/ionic/ionic_ethtool.c:120:41: warning: incorrect type in assignment (different base types)
drivers/net/ethernet/pensando/ionic/ionic_ethtool.c:120:41:    expected unsigned long long [usertype] link_down_events
drivers/net/ethernet/pensando/ionic/ionic_ethtool.c:120:41:    got restricted __le16 [usertype] link_down_count

Re: [PATCH net-next v2 2/5] ionic: Get "link_down_count" ext link stat from firmware

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ionic: Get "link_down_count" ext link stat from firmware

This commit replaces the driver-calculated software link down count with the
hardware count provided by the firmware in the extended link statistics. It
updates the ethtool callback to read the counter from the device's port_info
and removes the software tracking completely.

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 78a802eb159f0..af0c4cc8ad8e4 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -117,7 +117,8 @@ static void ionic_get_link_ext_stats(struct net_device *netdev,
>  	struct ionic_lif *lif = netdev_priv(netdev);
>  
>  	if (lif->ionic->pdev->is_physfn)
> -		stats->link_down_events = lif->link_down_count;
> +		stats->link_down_events =
> +			    lif->ionic->idev.port_info->status.link_down_count;

Could this lead to a NULL pointer dereference?

If a PCI error triggers device recovery via .reset_prepare, the device stops
but the net_device remains registered. If the subsequent ionic_setup_one()
fails during .reset_done, port_info is explicitly freed and set to NULL.
Calling ethtool in this state would crash the kernel.

Other callbacks like ionic_get_link_ksettings() guard against this:

	if (!idev->port_info) {
		netdev_err(netdev, "port_info not initialized\n");
		return -EOPNOTSUPP;
	}

Does link_down_count also need a byte-order conversion?

The firmware-provided field link_down_count in struct ionic_port_status is
defined as a little-endian 16-bit integer (__le16). Assigning it directly
to stats->link_down_events without wrapping it in le16_to_cpu() will result
in incorrect statistic values on big-endian architectures.

[PATCH net-next v2 3/5] ionic: Update ionic_if.h with new extra port stats structure

[Eric Joyner]

A new structure to report additional statistics from the firmware has
been added to struct ionic_port_info. It currently only contains FEC
related statistics, but new statistics collected by the firmware for the
port would go in it.

This structure is located in the same area as the unused
ionic_port_pb_stats structure, so this patch also removes that since it
was never used in this driver.

Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_if.h    | 36 ++++++-------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_if.h b/drivers/net/ethernet/pensando/ionic/ionic_if.h
index 23d6e2b4791e..01668dd10c0a 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_if.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_if.h
@@ -2855,6 +2855,14 @@ struct ionic_mgmt_port_stats {
 	__le64 frames_tx_pause;
 };
 
+struct ionic_port_extra_stats {
+	__le64 rsfec_correctable_blocks;
+	__le64 rsfec_uncorrectable_blocks;
+	__le64 fec_corrected_bits_total;
+	__le64 rx_bits_phy;
+	__le64 fec_codeword_error_bin[16];
+};
+
 enum ionic_pb_buffer_drop_stats {
 	IONIC_BUFFER_INTRINSIC_DROP = 0,
 	IONIC_BUFFER_DISCARDED,
@@ -2883,28 +2891,6 @@ enum ionic_oflow_drop_stats {
 	IONIC_OFLOW_DROP_MAX,
 };
 
-/* struct ionic_port_pb_stats - packet buffers system stats
- * uses ionic_pb_buffer_drop_stats for drop_counts[]
- */
-struct ionic_port_pb_stats {
-	__le64 sop_count_in;
-	__le64 eop_count_in;
-	__le64 sop_count_out;
-	__le64 eop_count_out;
-	__le64 drop_counts[IONIC_BUFFER_DROP_MAX];
-	__le64 input_queue_buffer_occupancy[IONIC_QOS_TC_MAX];
-	__le64 input_queue_port_monitor[IONIC_QOS_TC_MAX];
-	__le64 output_queue_port_monitor[IONIC_QOS_TC_MAX];
-	__le64 oflow_drop_counts[IONIC_OFLOW_DROP_MAX];
-	__le64 input_queue_good_pkts_in[IONIC_QOS_TC_MAX];
-	__le64 input_queue_good_pkts_out[IONIC_QOS_TC_MAX];
-	__le64 input_queue_err_pkts_in[IONIC_QOS_TC_MAX];
-	__le64 input_queue_fifo_depth[IONIC_QOS_TC_MAX];
-	__le64 input_queue_max_fifo_depth[IONIC_QOS_TC_MAX];
-	__le64 input_queue_peak_occupancy[IONIC_QOS_TC_MAX];
-	__le64 output_queue_buffer_occupancy[IONIC_QOS_TC_MAX];
-};
-
 /**
  * struct ionic_port_identity - port identity structure
  * @version:        identity structure version
@@ -2950,7 +2936,7 @@ union ionic_port_identity {
  * @sprom_page2:     Extended Transceiver sprom, page 2
  * @sprom_page17:    Extended Transceiver sprom, page 17
  * @rsvd:            reserved byte(s)
- * @pb_stats:        uplink pb drop stats
+ * @extra_stats:     Extra port statistics data
  */
 struct ionic_port_info {
 	union ionic_port_config config;
@@ -2968,9 +2954,7 @@ struct ionic_port_info {
 		};
 	};
 	u8     rsvd[376];
-
-	/* pb_stats must start at 2k offset */
-	struct ionic_port_pb_stats  pb_stats;
+	struct ionic_port_extra_stats extra_stats;
 };
 
 /*
-- 
2.17.1

[PATCH net-next v2 4/5] ionic: Report rx_bits_phy stat to ethtool

[Eric Joyner]

This stat contains the number of total bits that the PHY has received;
it's useful for BER calculations. Add it to the ethtool stats output.

Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_stats.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.c b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
index 0107599a9dd4..c45ed0db0582 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_stats.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
@@ -167,6 +167,7 @@ static const struct ionic_stat_desc ionic_rx_stats_desc[] = {
 #define IONIC_NUM_PORT_STATS ARRAY_SIZE(ionic_port_stats_desc)
 #define IONIC_NUM_TX_STATS ARRAY_SIZE(ionic_tx_stats_desc)
 #define IONIC_NUM_RX_STATS ARRAY_SIZE(ionic_rx_stats_desc)
+#define IONIC_NUM_EXTRA_PORT_STATS	1
 
 #define MAX_Q(lif)   ((lif)->netdev->real_num_tx_queues)
 
@@ -243,7 +244,7 @@ static u64 ionic_sw_stats_get_count(struct ionic_lif *lif)
 		rx_queues += 1;
 
 	total += IONIC_NUM_LIF_STATS;
-	total += IONIC_NUM_PORT_STATS;
+	total += IONIC_NUM_PORT_STATS + IONIC_NUM_EXTRA_PORT_STATS;
 
 	total += tx_queues * IONIC_NUM_TX_STATS;
 	total += rx_queues * IONIC_NUM_RX_STATS;
@@ -280,6 +281,8 @@ static void ionic_sw_stats_get_strings(struct ionic_lif *lif, u8 **buf)
 
 	for (i = 0; i < IONIC_NUM_PORT_STATS; i++)
 		ethtool_puts(buf, ionic_port_stats_desc[i].name);
+	/* extra port stats */
+	ethtool_puts(buf, "rx_bits_phy");
 
 	for (q_num = 0; q_num < MAX_Q(lif); q_num++)
 		ionic_sw_stats_get_tx_strings(lif, buf, q_num);
@@ -322,6 +325,15 @@ static void ionic_sw_stats_get_rxq_values(struct ionic_lif *lif, u64 **buf,
 	}
 }
 
+static void ionic_extra_port_stats_get_values(struct ionic_lif *lif, u64 **buf)
+{
+	struct ionic_port_info *port_info = lif->ionic->idev.port_info;
+
+	/* The # of stats added here == IONIC_NUM_EXTRA_PORT_STATS */
+	**buf = le64_to_cpu(port_info->extra_stats.rx_bits_phy);
+	(*buf)++;
+}
+
 static void ionic_sw_stats_get_values(struct ionic_lif *lif, u64 **buf)
 {
 	struct ionic_port_stats *port_stats;
@@ -341,6 +353,7 @@ static void ionic_sw_stats_get_values(struct ionic_lif *lif, u64 **buf)
 					     &ionic_port_stats_desc[i]);
 		(*buf)++;
 	}
+	ionic_extra_port_stats_get_values(lif, buf);
 
 	for (q_num = 0; q_num < MAX_Q(lif); q_num++)
 		ionic_sw_stats_get_txq_values(lif, buf, q_num);
-- 
2.17.1

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

[Eric Joyner]

Several FEC error statistics being collected can be reported in a
dedicated ethtool callback for FEC errors, so implement the handler that
does so. This includes 802.3ck FEC histogram data that some newer
hardware collects.

Assisted-by: Claude:claude-4.6-sonnet
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 .../ethernet/pensando/ionic/ionic_ethtool.c   | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index af0c4cc8ad8e..b0d7b5a9d189 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -419,6 +419,57 @@ static int ionic_get_fecparam(struct net_device *netdev,
 	return 0;
 }
 
+static const struct ethtool_fec_hist_range ionic_fec_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 *extra_stats,
+		    struct ethtool_fec_hist *hist)
+{
+	int i;
+
+	hist->ranges = ionic_fec_ranges;
+	for (i = 0; i < ETHTOOL_FEC_HIST_MAX - 1; i++)
+		hist->values[i].sum =
+			le64_to_cpu(extra_stats->fec_codeword_error_bin[i]);
+}
+
+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 *extra_stats;
+	struct ionic_lif *lif = netdev_priv(netdev);
+
+	extra_stats = &lif->ionic->idev.port_info->extra_stats;
+
+	fec_stats->corrected_blocks.total =
+		le64_to_cpu(extra_stats->rsfec_correctable_blocks);
+	fec_stats->uncorrectable_blocks.total =
+		le64_to_cpu(extra_stats->rsfec_uncorrectable_blocks);
+	fec_stats->corrected_bits.total =
+		le64_to_cpu(extra_stats->fec_corrected_bits_total);
+
+	ionic_fill_fec_hist(extra_stats, hist);
+}
+
 static int ionic_set_fecparam(struct net_device *netdev,
 			      struct ethtool_fecparam *fec)
 {
@@ -1155,6 +1206,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,
-- 
2.17.1