DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] net/intel: fix link status when interrupt delivery unavailable
@ 2026-06-30 14:16 Ciara Loftus
  2026-06-30 14:16 ` [PATCH 1/2] net/ice: poll AdminQ if " Ciara Loftus
  2026-06-30 14:16 ` [PATCH 2/2] net/i40e: activate alarm " Ciara Loftus
  0 siblings, 2 replies; 4+ messages in thread
From: Ciara Loftus @ 2026-06-30 14:16 UTC (permalink / raw)
  To: dev; +Cc: Ciara Loftus

On FreeBSD with nic_uio, interrupt registration fails silently and
hardware events, including link state notifications, are never processed
via the interrupt path.

For ice, this affects the typical polling-mode configuration: the
interrupt handler is never invoked, AdminQ messages go unprocessed,
and link status is not reliable. A periodic alarm is added to drain
the AdminQ when interrupt delivery is unavailable. For i40e, an
alarm already runs unconditionally in polling mode, so that
configuration is unaffected. The gap is the interrupt-mode
configuration, where the existing alarm was not activated as a
fallback when interrupt enable fails; this patch extends it to do so.

Ciara Loftus (2):
  net/ice: poll AdminQ if interrupt delivery unavailable
  net/i40e: activate alarm if interrupt delivery unavailable

 drivers/net/intel/i40e/i40e_ethdev.c |  9 +++++-
 drivers/net/intel/i40e/i40e_ethdev.h |  2 ++
 drivers/net/intel/ice/ice_ethdev.c   | 45 +++++++++++++++++++++++++++-
 drivers/net/intel/ice/ice_ethdev.h   |  1 +
 4 files changed, 55 insertions(+), 2 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] net/ice: poll AdminQ if interrupt delivery unavailable
  2026-06-30 14:16 [PATCH 0/2] net/intel: fix link status when interrupt delivery unavailable Ciara Loftus
@ 2026-06-30 14:16 ` Ciara Loftus
  2026-07-01 11:26   ` Bruce Richardson
  2026-06-30 14:16 ` [PATCH 2/2] net/i40e: activate alarm " Ciara Loftus
  1 sibling, 1 reply; 4+ messages in thread
From: Ciara Loftus @ 2026-06-30 14:16 UTC (permalink / raw)
  To: dev; +Cc: Ciara Loftus, stable

If interrupt registration fails eg. on FreeBSD with nic_uio, the ice
interrupt handler is never called and AdminQ messages, including
unsolicited link state notifications, are never processed.

This gap has always existed but was partially masked by the blocking link
status poll in the ice dev_start function, which would delay some time
until the link was up before returning. In this case, the link status was
typically correct after dev_start. However after that delay was removed in
2dd7e99855 ("net/ice: revert fix link up when starting device"), the link
status was often wrong after dev_start, and there was no mechanism to
self-correct.

Rather than restoring the blocking wait removed by that commit which
would re-impose a startup delay on every platform, instead activate a
periodic alarm that drains the AdminQ on a 50ms interval, replicating the
role of the interrupt handler on platforms where interrupt delivery is not
supported.

Fixes: 2dd7e99855 ("net/ice: revert fix link up when starting device")
Cc: stable@dpdk.org

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 drivers/net/intel/ice/ice_ethdev.c | 45 +++++++++++++++++++++++++++++-
 drivers/net/intel/ice/ice_ethdev.h |  1 +
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 022d92a2f6..447742ff8d 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -3,6 +3,7 @@
  */
 
 #include <rte_string_fns.h>
+#include <rte_alarm.h>
 #include <ethdev_pci.h>
 
 #include <ctype.h>
@@ -108,7 +109,11 @@ enum ice_link_state_on_close {
 
 #define ICE_RSS_LUT_GLOBAL_QUEUE_NB	64
 
+/* Polling interval used when the interrupt path is unavailable */
+#define ICE_AQ_ALARM_INTERVAL 50000 /* us */
+
 static int ice_dev_configure(struct rte_eth_dev *dev);
+static void ice_aq_alarm_handler(void *param);
 static int ice_dev_start(struct rte_eth_dev *dev);
 static int ice_dev_stop(struct rte_eth_dev *dev);
 static int ice_dev_close(struct rte_eth_dev *dev);
@@ -1473,6 +1478,28 @@ ice_handle_aq_msg(struct rte_eth_dev *dev)
 	}
 }
 
+/*
+ * Alarm-based AdminQ polling handler that drains the AdminQ to process
+ * async events, then re-arms itself.
+ *
+ * @param param
+ *  The address of parameter (struct rte_eth_dev *) registered before.
+ *
+ * @return
+ *  void
+ */
+static void
+ice_aq_alarm_handler(void *param)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	if (!pf->adapter_stopped) {
+		ice_handle_aq_msg(dev);
+		rte_eal_alarm_set(ICE_AQ_ALARM_INTERVAL, ice_aq_alarm_handler, dev);
+	}
+}
+
 /**
  * Interrupt handler triggered by NIC for handling
  * specific interrupt.
@@ -2913,6 +2940,11 @@ ice_dev_stop(struct rte_eth_dev *dev)
 	rte_intr_efd_disable(intr_handle);
 	rte_intr_vec_list_free(intr_handle);
 
+	if (pf->use_aq_polling) {
+		rte_eal_alarm_cancel(ice_aq_alarm_handler, dev);
+		pf->use_aq_polling = false;
+	}
+
 	pf->adapter_stopped = true;
 	dev->data->dev_started = 0;
 
@@ -4434,7 +4466,13 @@ ice_dev_start(struct rte_eth_dev *dev)
 	if (ice_rxq_intr_setup(dev))
 		return -EIO;
 
-	rte_intr_enable(pci_dev->intr_handle);
+	/* Fall back to polling the AdminQ via an alarm on platforms where
+	 * interrupt delivery is unavailable.
+	 */
+	if (rte_intr_enable(pci_dev->intr_handle) != 0) {
+		pf->use_aq_polling = true;
+		rte_eal_alarm_set(ICE_AQ_ALARM_INTERVAL, ice_aq_alarm_handler, dev);
+	}
 
 	/* Enable receiving broadcast packets and transmitting packets */
 	ice_set_bit(ICE_PROMISC_BCAST_RX, pmask);
@@ -4497,6 +4535,11 @@ ice_dev_start(struct rte_eth_dev *dev)
 	for (i = 0; i < nb_txq; i++)
 		ice_tx_queue_stop(dev, i);
 
+	if (pf->use_aq_polling) {
+		rte_eal_alarm_cancel(ice_aq_alarm_handler, dev);
+		pf->use_aq_polling = false;
+	}
+
 	return -EIO;
 }
 
diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h
index 20e8a13fe9..86b256fc3f 100644
--- a/drivers/net/intel/ice/ice_ethdev.h
+++ b/drivers/net/intel/ice/ice_ethdev.h
@@ -599,6 +599,7 @@ struct ice_pf {
 	struct ice_eth_stats internal_stats;
 	bool offset_loaded;
 	bool adapter_stopped;
+	bool use_aq_polling; /* true when interrupt path unavailable */
 	struct ice_flow_list flow_list;
 	rte_spinlock_t flow_ops_lock;
 	bool init_link_up;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] net/i40e: activate alarm if interrupt delivery unavailable
  2026-06-30 14:16 [PATCH 0/2] net/intel: fix link status when interrupt delivery unavailable Ciara Loftus
  2026-06-30 14:16 ` [PATCH 1/2] net/ice: poll AdminQ if " Ciara Loftus
@ 2026-06-30 14:16 ` Ciara Loftus
  1 sibling, 0 replies; 4+ messages in thread
From: Ciara Loftus @ 2026-06-30 14:16 UTC (permalink / raw)
  To: dev; +Cc: Ciara Loftus, stable

If interrupt registration fails eg. on FreeBSD with nic_uio, and Rx
queue interrupts are configured (intr_conf.rxq != 0), the interrupt
handler is never called and ICR0 events, including AdminQ messages
carrying link state notifications, are never processed. In polling
mode (rxq == 0) a periodic alarm already drains these events, but in
the interrupt-enabled configuration no such fallback existed.

This gap has always existed but was partially masked by the blocking link
status poll in the i40e dev_start function, which would delay some time
until the link was up before returning. In this case, the link status was
typically correct after dev_start. However after that delay was removed in
111965395b ("net/i40e: fix blocking link wait on device start"), the link
status was often wrong after dev_start, and there was no mechanism to
self-correct.

Rather than restoring the blocking wait removed by that commit which would
re-impose a startup delay on every platform, activate the existing periodic
alarm on platforms where interrupt delivery is unavailable.

Fixes: 111965395b ("net/i40e: fix blocking link wait on device start")
Cc: stable@dpdk.org

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 drivers/net/intel/i40e/i40e_ethdev.c | 9 ++++++++-
 drivers/net/intel/i40e/i40e_ethdev.h | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/i40e/i40e_ethdev.c b/drivers/net/intel/i40e/i40e_ethdev.c
index b2694cd33a..86eb7468c8 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.c
+++ b/drivers/net/intel/i40e/i40e_ethdev.c
@@ -2579,7 +2579,11 @@ i40e_dev_start(struct rte_eth_dev *dev)
 		i40e_dev_link_update(dev, !rte_intr_allow_others(intr_handle));
 		pf->mac_config_on_link_up = !dev->data->dev_link.link_status;
 		/* enable uio intr after callback register */
-		rte_intr_enable(intr_handle);
+		if (rte_intr_enable(intr_handle) != 0) {
+			pf->use_aq_polling = true;
+			rte_eal_alarm_set(I40E_ALARM_INTERVAL,
+					  i40e_dev_alarm_handler, dev);
+		}
 	}
 
 	i40e_filter_restore(pf);
@@ -2625,6 +2629,9 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	if (dev->data->dev_conf.intr_conf.rxq == 0) {
 		rte_eal_alarm_cancel(i40e_dev_alarm_handler, dev);
 		rte_intr_enable(intr_handle);
+	} else if (pf->use_aq_polling) {
+		rte_eal_alarm_cancel(i40e_dev_alarm_handler, dev);
+		pf->use_aq_polling = false;
 	}
 
 	/* Disable all queues */
diff --git a/drivers/net/intel/i40e/i40e_ethdev.h b/drivers/net/intel/i40e/i40e_ethdev.h
index 5a009393b0..9d9bde6aeb 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.h
+++ b/drivers/net/intel/i40e/i40e_ethdev.h
@@ -1193,6 +1193,8 @@ struct i40e_pf {
 	bool fw8_3gt;
 	/* MAC config needs re-applying when link first comes up */
 	bool mac_config_on_link_up;
+	/* true when interrupt path unavailable */
+	bool use_aq_polling;
 
 	struct i40e_vf_msg_cfg vf_msg_cfg;
 	uint64_t prev_rx_bytes;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] net/ice: poll AdminQ if interrupt delivery unavailable
  2026-06-30 14:16 ` [PATCH 1/2] net/ice: poll AdminQ if " Ciara Loftus
@ 2026-07-01 11:26   ` Bruce Richardson
  0 siblings, 0 replies; 4+ messages in thread
From: Bruce Richardson @ 2026-07-01 11:26 UTC (permalink / raw)
  To: Ciara Loftus; +Cc: dev, stable

On Tue, Jun 30, 2026 at 02:16:26PM +0000, Ciara Loftus wrote:
> If interrupt registration fails eg. on FreeBSD with nic_uio, the ice
> interrupt handler is never called and AdminQ messages, including
> unsolicited link state notifications, are never processed.
> 
> This gap has always existed but was partially masked by the blocking link
> status poll in the ice dev_start function, which would delay some time
> until the link was up before returning. In this case, the link status was
> typically correct after dev_start. However after that delay was removed in
> 2dd7e99855 ("net/ice: revert fix link up when starting device"), the link
> status was often wrong after dev_start, and there was no mechanism to
> self-correct.
> 
> Rather than restoring the blocking wait removed by that commit which
> would re-impose a startup delay on every platform, instead activate a
> periodic alarm that drains the AdminQ on a 50ms interval, replicating the
> role of the interrupt handler on platforms where interrupt delivery is not
> supported.
> 
> Fixes: 2dd7e99855 ("net/ice: revert fix link up when starting device")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---

This solution seems reasonable. Some thoughts and comments inline below.

/Bruce

>  drivers/net/intel/ice/ice_ethdev.c | 45 +++++++++++++++++++++++++++++-
>  drivers/net/intel/ice/ice_ethdev.h |  1 +
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 022d92a2f6..447742ff8d 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -3,6 +3,7 @@
>   */
>  
>  #include <rte_string_fns.h>
> +#include <rte_alarm.h>
>  #include <ethdev_pci.h>
>  
>  #include <ctype.h>
> @@ -108,7 +109,11 @@ enum ice_link_state_on_close {
>  
>  #define ICE_RSS_LUT_GLOBAL_QUEUE_NB	64
>  
> +/* Polling interval used when the interrupt path is unavailable */
> +#define ICE_AQ_ALARM_INTERVAL 50000 /* us */
> +
>  static int ice_dev_configure(struct rte_eth_dev *dev);
> +static void ice_aq_alarm_handler(void *param);
>  static int ice_dev_start(struct rte_eth_dev *dev);
>  static int ice_dev_stop(struct rte_eth_dev *dev);
>  static int ice_dev_close(struct rte_eth_dev *dev);
> @@ -1473,6 +1478,28 @@ ice_handle_aq_msg(struct rte_eth_dev *dev)
>  	}
>  }
>  
> +/*
> + * Alarm-based AdminQ polling handler that drains the AdminQ to process
> + * async events, then re-arms itself.
> + *
> + * @param param
> + *  The address of parameter (struct rte_eth_dev *) registered before.
> + *
> + * @return
> + *  void
> + */
> +static void
> +ice_aq_alarm_handler(void *param)
> +{
> +	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
> +	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
> +	if (!pf->adapter_stopped) {
> +		ice_handle_aq_msg(dev);
> +		rte_eal_alarm_set(ICE_AQ_ALARM_INTERVAL, ice_aq_alarm_handler, dev);
> +	}
> +}
> +
>  /**
>   * Interrupt handler triggered by NIC for handling
>   * specific interrupt.
> @@ -2913,6 +2940,11 @@ ice_dev_stop(struct rte_eth_dev *dev)
>  	rte_intr_efd_disable(intr_handle);
>  	rte_intr_vec_list_free(intr_handle);
>  
> +	if (pf->use_aq_polling) {
> +		rte_eal_alarm_cancel(ice_aq_alarm_handler, dev);
> +		pf->use_aq_polling = false;
> +	}
> +

Do we actually need to cancel the timer here? Since the callback itself
only rearms on started devices, we can let the timer expire and become a
no-op, rather than needing to explicitly cancel it here.

If so, and based on other suggestions I have below, we may be able to
remove this block from stop function entirely.

>  	pf->adapter_stopped = true;
>  	dev->data->dev_started = 0;
>  
> @@ -4434,7 +4466,13 @@ ice_dev_start(struct rte_eth_dev *dev)
>  	if (ice_rxq_intr_setup(dev))
>  		return -EIO;
>  
> -	rte_intr_enable(pci_dev->intr_handle);
> +	/* Fall back to polling the AdminQ via an alarm on platforms where
> +	 * interrupt delivery is unavailable.
> +	 */
> +	if (rte_intr_enable(pci_dev->intr_handle) != 0) {
> +		pf->use_aq_polling = true;
> +		rte_eal_alarm_set(ICE_AQ_ALARM_INTERVAL, ice_aq_alarm_handler, dev);
> +	}

If alarm set fails, we should at least emit a warning, even if there is
nothing we can really do about it.

Also, I don't think we need to track use_aq_polling at all. Its only use is
to cancel the timer, but all places where we cancel the timer are places
where we mark the port as stopped (with one exception, which I believe is a
separate error in the code), so the timer will self-cancel at next timeout
expiry.

>  
>  	/* Enable receiving broadcast packets and transmitting packets */
>  	ice_set_bit(ICE_PROMISC_BCAST_RX, pmask);
> @@ -4497,6 +4535,11 @@ ice_dev_start(struct rte_eth_dev *dev)
>  	for (i = 0; i < nb_txq; i++)
>  		ice_tx_queue_stop(dev, i);
>  
> +	if (pf->use_aq_polling) {
> +		rte_eal_alarm_cancel(ice_aq_alarm_handler, dev);
> +		pf->use_aq_polling = false;
> +	}
> +

As explained above, I don't think we need this block either. If we have an
error on start, then the port should be marked as stopped and the timer
won't rearm itself after it expires.

NOTE: there is an unrelated bug in the code here - at line 4550 in start,
we set "adapter_stopped = false", but there is still error handling at line
4564 which causes us to jump to rx_err and stop all the rx and tx queues
again, but neglecting to reset adapter_stopped back to true. I think we
need to either reset it immediately after the rx_err label, or move the
assignment down till after all potential rollback points are passed.

>  	return -EIO;
>  }
>  
> diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h
> index 20e8a13fe9..86b256fc3f 100644
> --- a/drivers/net/intel/ice/ice_ethdev.h
> +++ b/drivers/net/intel/ice/ice_ethdev.h
> @@ -599,6 +599,7 @@ struct ice_pf {
>  	struct ice_eth_stats internal_stats;
>  	bool offset_loaded;
>  	bool adapter_stopped;
> +	bool use_aq_polling; /* true when interrupt path unavailable */
>  	struct ice_flow_list flow_list;
>  	rte_spinlock_t flow_ops_lock;
>  	bool init_link_up;
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-01 11:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-30 14:16 [PATCH 0/2] net/intel: fix link status when interrupt delivery unavailable Ciara Loftus
2026-06-30 14:16 ` [PATCH 1/2] net/ice: poll AdminQ if " Ciara Loftus
2026-07-01 11:26   ` Bruce Richardson
2026-06-30 14:16 ` [PATCH 2/2] net/i40e: activate alarm " Ciara Loftus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox