net/iavf: fix reset race condition

PATCH Superseded Ciara Loftus 3 participants 4 messages Sep 24 – Sep 29 Network drivers

Share & tools

Ciara Loftus

author

2026-09-24 08:42 UTC #0 lore raw

PATCH 1/1 net/iavf: fix reset race condition

iavf_handle_hw_reset() can be called concurrently from the iavf-event thread handling RTE_ETH_EVENT_INTR_RESET and from any thread calling rte_pmd_iavf_reinit() (VF-initiated reset), with no synchronisation between the two. Make in_reset_recovery atomic and use it as a guard so only one reset runs at a time.

Fixes 28a1a72eac26 ("net/iavf: add VF initiated reset")

Signed-off-by Ciara Loftus <[email protected]>

---
 drivers/net/intel/iavf/iavf.h        |  4 +--
 drivers/net/intel/iavf/iavf_ethdev.c | 39 ++++++++++++++++++----------
 drivers/net/intel/iavf/iavf_rxtx.c   |  2 +-
 drivers/net/intel/iavf/iavf_vchnl.c  |  3 ++-
 4 files changed, 30 insertions(+), 18 deletions(-)

drivers/net/intel/iavf/iavf.h +2 -2

index f16f66a3e9..35e96f881a 100644
@@ -295,7 +295,7 @@ struct iavf_info {
 
 	struct rte_eth_dev *eth_dev;
 
-	bool in_reset_recovery;
+	RTE_ATOMIC(bool) in_reset_recovery;
 	bool reset_pending;
 	bool pf_reset_in_progress;
 	bool start_pending;

@@ -534,7 +534,7 @@ int iavf_flow_sub_check(struct iavf_adapter *adapter,
 			struct iavf_fsub_conf *filter);
 void iavf_dev_watchdog_enable(struct iavf_adapter *adapter);
 void iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
-void iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset);
+int iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset);
 void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change);
 bool is_iavf_supported(struct rte_eth_dev *dev);
 void iavf_hash_uninit(struct iavf_adapter *ad);

drivers/net/intel/iavf/iavf_ethdev.c +25 ~~-14~~

index a8daa507bb..b323a39e55 100644
@@ -767,7 +767,8 @@ iavf_dev_configure(struct rte_eth_dev *dev)
 	 * recovery, in which case the reset handler restores them once at the
 	 * end (avoiding a double restore).
 	 */
-	if (reset_done && !vf->in_reset_recovery) {
+	if (reset_done && !rte_atomic_load_explicit(&vf->in_reset_recovery,
+			rte_memory_order_relaxed)) {
 		ret = iavf_post_reset_reconfig(dev);
 		if (ret)
 			return ret;

@@ -3095,7 +3096,8 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	adapter->tpid = RTE_ETHER_TYPE_VLAN; /* VLAN TPID set to 0x8100 by default */
 	rte_spinlock_init(&adapter->phc_sync_lock);
 
-	if (!vf->in_reset_recovery && iavf_dev_event_handler_init())
+	if (!rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed) &&
+	    iavf_dev_event_handler_init())
 		goto init_vf_err;
 
 	if (iavf_init_vf(eth_dev) != 0) {

@@ -3342,7 +3344,7 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 
 	iavf_dev_close(dev);
 
-	if (!vf->in_reset_recovery)
+	if (!rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed))
 		iavf_dev_event_handler_fini();
 
 	return 0;

@@ -3463,11 +3465,12 @@ iavf_post_reset_reconfig(struct rte_eth_dev *dev)
 /*
  * Handle hardware reset
  */
-void
+int
 iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)
 {
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct iavf_adapter *adapter = dev->data->dev_private;
+	bool expected = false;
 	int ret;
 	bool restart_device = false;

@@ -3475,13 +3478,21 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)
 		restart_device = dev->data->dev_started;
 	} else {
 		if (!dev->data->dev_started)
-			return;
+			return 0;
 
 		if (!iavf_is_reset_detected(adapter))
 			PMD_DRV_LOG(WARNING, "VFR not observed; recovering anyway");
 	}
 
-	vf->in_reset_recovery = true;
+	/* serialize against a concurrent reset from another thread */
+	if (!rte_atomic_compare_exchange_strong_explicit(&vf->in_reset_recovery,
+			&expected, true, rte_memory_order_acquire,
+			rte_memory_order_acquire)) {
+		PMD_DRV_LOG(INFO, "Reset already in progress on port %u, skipping",
+				dev->data->port_id);
+		return -EBUSY;
+	}
+
 	vf->pf_reset_in_progress = !vf_initiated_reset;
 	vf->start_pending = false;
 	iavf_set_no_poll(adapter, false);

@@ -3533,11 +3544,11 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)
 	if (vf->post_reset_cb != NULL)
 		vf->post_reset_cb(dev->data->port_id, ret, vf->post_reset_cb_arg);
 
-	vf->in_reset_recovery = false;
+	rte_atomic_store_explicit(&vf->in_reset_recovery, false, rte_memory_order_release);
 	vf->pf_reset_in_progress = false;
 	iavf_set_no_poll(adapter, false);
 
-	return;
+	return ret;
 }
 
 RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_pmd_iavf_reinit, 25.11)

@@ -3568,9 +3579,7 @@ rte_pmd_iavf_reinit(uint16_t port)
 		return -EINVAL;
 	}
 
-	iavf_handle_hw_reset(dev, true);
-
-	return 0;
+	return iavf_handle_hw_reset(dev, true);
 }
 
 static int

@@ -3593,7 +3602,7 @@ iavf_validate_reset_cb(uint16_t port, void *cb, void *cb_arg)
 	}
 
 	vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
-	if (vf->in_reset_recovery) {
+	if (rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed)) {
 		PMD_DRV_LOG(ERR, "Cannot modify reset cb on port %u, VF is resetting.", port);
 		return -EBUSY;
 	}

@@ -3652,7 +3661,8 @@ iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change)
 	bool no_poll;
 
 	no_poll = (link_change & !vf->link_up) ||
-		vf->vf_reset || vf->in_reset_recovery;
+		vf->vf_reset ||
+		rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed);
 
 	rte_atomic_store_explicit(&adapter->no_poll, no_poll,
 				  rte_memory_order_release);

@@ -3734,7 +3744,8 @@ iavf_resume_pending_start(struct rte_eth_dev *dev)
 	if (!vf->start_pending)
 		return;
 
-	if (vf->vf_reset || vf->in_reset_recovery)
+	if (vf->vf_reset || rte_atomic_load_explicit(&vf->in_reset_recovery,
+			rte_memory_order_relaxed))
 		return;
 
 	/*

drivers/net/intel/iavf/iavf_rxtx.c +1 -1

index fc47a2cf1a..84b931e78a 100644
@@ -1106,7 +1106,7 @@ iavf_stop_queues(struct rte_eth_dev *dev)
 	int ret;
 
 	/* adminq will be disabled when vf is resetting. */
-	if (vf->in_reset_recovery) {
+	if (rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed)) {
 		iavf_reset_queues(dev);
 		return;
 	}

drivers/net/intel/iavf/iavf_vchnl.c +2 -1

index 604399125f..6c054acf80 100644
@@ -260,7 +260,8 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,
 	 * (link is down or a VF reset is in progress); the watchdog drives
 	 * auto-reset recovery, so it must remain armed in those cases.
 	 */
-	if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)
+	if (vf->link_up && !vf->vf_reset &&
+	    !rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed))
 		iavf_dev_watchdog_disable(adapter);
 	else
 		iavf_dev_watchdog_enable(adapter);

Signature · 1 line

2.43.0

Burakov, Anatoly

Acked-by on v1 1/1

2026-09-25 10:46 UTC #1 lore raw

Quoted 10 lines · Ciara Loftus

iavf_handle_hw_reset() can be called concurrently from the iavf-event thread handling RTE_ETH_EVENT_INTR_RESET and from any thread calling rte_pmd_iavf_reinit() (VF-initiated reset), with no synchronisation between the two. Make in_reset_recovery atomic and use it as a guard so only one reset runs at a time.
Fixes 28a1a72eac26 ("net/iavf: add VF initiated reset")
Signed-off-by Ciara Loftus <[email protected]>
---

There's an AI review comment about a potential stale in_reset_recovery state when init/uninit fails in certain cases. I've taken the liberty to check this against the code, and it seems to be a false positive.

My own AI review flagged the following:

```
iavf_ethdev.c:3747: after observing false, this function reads start_pending, dev_started, link_up that the reset thread wrote before its release-store. A relaxed load does not formally synchronize with that release, so on weakly-ordered CPUs those reads could be stale. Use rte_memory_order_acquire here.

iavf_ethdev.c:3547-3548: vf->pf_reset_in_progress = false is written after the release-store, so it is not covered by it. A concurrent dev_close() on another thread that sees the flag cleared could still see pf_reset_in_progress == true and skip iavf_vf_reset()/flow flush. Swap the two lines.
```

However, if we're running two dev_close() calls concurrently we've got way bigger problems so I don't think the second finding is meaningful. The first one may be a "correctness" issue but not blocking either.

So, LGTM (with or without the above suggested fixed)

Acked-by Anatoly Burakov <[email protected]>

Signature · 2 lines

Thanks,
Anatoly

Ciara Loftus

author

2026-09-28 09:53 UTC #2 lore raw

PATCH v2 1/1 net/iavf: fix reset race condition

In reply to Ciara Loftus

Fixes 28a1a72eac26 ("net/iavf: add VF initiated reset")

Signed-off-by Ciara Loftus <[email protected]>

Acked-by Anatoly Burakov <[email protected]>

---
v2:
* Use acquire ordering in iavf_resume_pending_start()
* Clear pf_reset_in_progress before the release store

---
 drivers/net/intel/iavf/iavf.h        |  4 +--
 drivers/net/intel/iavf/iavf_ethdev.c | 39 ++++++++++++++++++----------
 drivers/net/intel/iavf/iavf_rxtx.c   |  2 +-
 drivers/net/intel/iavf/iavf_vchnl.c  |  3 ++-
 4 files changed, 30 insertions(+), 18 deletions(-)

drivers/net/intel/iavf/iavf.h +2 -2

index f16f66a3e9..c40b97444b 100644
@@ -295,7 +295,7 @@ struct iavf_info {
 
 	struct rte_eth_dev *eth_dev;
 
-	bool in_reset_recovery;
+	RTE_ATOMIC(bool)in_reset_recovery;
 	bool reset_pending;
 	bool pf_reset_in_progress;
 	bool start_pending;

@@ -534,7 +534,7 @@ int iavf_flow_sub_check(struct iavf_adapter *adapter,
 			struct iavf_fsub_conf *filter);
 void iavf_dev_watchdog_enable(struct iavf_adapter *adapter);
 void iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
-void iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset);
+int iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset);
 void iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change);
 bool is_iavf_supported(struct rte_eth_dev *dev);
 void iavf_hash_uninit(struct iavf_adapter *ad);

drivers/net/intel/iavf/iavf_ethdev.c +25 ~~-14~~

index dafe22bdd4..6cf12ec34f 100644
@@ -767,7 +767,8 @@ iavf_dev_configure(struct rte_eth_dev *dev)
 	 * recovery, in which case the reset handler restores them once at the
 	 * end (avoiding a double restore).
 	 */
-	if (reset_done && !vf->in_reset_recovery) {
+	if (reset_done && !rte_atomic_load_explicit(&vf->in_reset_recovery,
+			rte_memory_order_relaxed)) {
 		ret = iavf_post_reset_reconfig(dev);
 		if (ret)
 			return ret;

@@ -3083,7 +3084,8 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	adapter->tpid = RTE_ETHER_TYPE_VLAN; /* VLAN TPID set to 0x8100 by default */
 	rte_spinlock_init(&adapter->phc_sync_lock);
 
-	if (!vf->in_reset_recovery && iavf_dev_event_handler_init())
+	if (!rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed) &&
+	    iavf_dev_event_handler_init())
 		goto init_vf_err;
 
 	if (iavf_init_vf(eth_dev) != 0) {

@@ -3330,7 +3332,7 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
 
 	iavf_dev_close(dev);
 
-	if (!vf->in_reset_recovery)
+	if (!rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed))
 		iavf_dev_event_handler_fini();
 
 	return 0;

@@ -3451,11 +3453,12 @@ iavf_post_reset_reconfig(struct rte_eth_dev *dev)
 /*
  * Handle hardware reset
  */
-void
+int
 iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)
 {
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct iavf_adapter *adapter = dev->data->dev_private;
+	bool expected = false;
 	int ret;
 	bool restart_device = false;

@@ -3463,13 +3466,21 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)
 		restart_device = dev->data->dev_started;
 	} else {
 		if (!dev->data->dev_started)
-			return;
+			return 0;
 
 		if (!iavf_is_reset_detected(adapter))
 			PMD_DRV_LOG(WARNING, "VFR not observed; recovering anyway");
 	}
 
-	vf->in_reset_recovery = true;
+	/* serialize against a concurrent reset from another thread */
+	if (!rte_atomic_compare_exchange_strong_explicit(&vf->in_reset_recovery,
+			&expected, true, rte_memory_order_acquire,
+			rte_memory_order_acquire)) {
+		PMD_DRV_LOG(INFO, "Reset already in progress on port %u, skipping",
+				dev->data->port_id);
+		return -EBUSY;
+	}
+
 	vf->pf_reset_in_progress = !vf_initiated_reset;
 	vf->start_pending = false;
 	iavf_set_no_poll(adapter, false);

@@ -3520,11 +3531,11 @@ iavf_handle_hw_reset(struct rte_eth_dev *dev, bool vf_initiated_reset)
 	if (vf->post_reset_cb != NULL)
 		vf->post_reset_cb(dev->data->port_id, ret, vf->post_reset_cb_arg);
 
-	vf->in_reset_recovery = false;
 	vf->pf_reset_in_progress = false;
+	rte_atomic_store_explicit(&vf->in_reset_recovery, false, rte_memory_order_release);
 	iavf_set_no_poll(adapter, false);
 
-	return;
+	return ret;
 }
 
 RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_pmd_iavf_reinit, 25.11)

@@ -3555,9 +3566,7 @@ rte_pmd_iavf_reinit(uint16_t port)
 		return -EINVAL;
 	}
 
-	iavf_handle_hw_reset(dev, true);
-
-	return 0;
+	return iavf_handle_hw_reset(dev, true);
 }
 
 static int

@@ -3580,7 +3589,7 @@ iavf_validate_reset_cb(uint16_t port, void *cb, void *cb_arg)
 	}
 
 	vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
-	if (vf->in_reset_recovery) {
+	if (rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed)) {
 		PMD_DRV_LOG(ERR, "Cannot modify reset cb on port %u, VF is resetting.", port);
 		return -EBUSY;
 	}

@@ -3639,7 +3648,8 @@ iavf_set_no_poll(struct iavf_adapter *adapter, bool link_change)
 	bool no_poll;
 
 	no_poll = (link_change & !vf->link_up) ||
-		vf->vf_reset || vf->in_reset_recovery;
+		vf->vf_reset ||
+		rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed);
 
 	rte_atomic_store_explicit(&adapter->no_poll, no_poll,
 				  rte_memory_order_release);

@@ -3721,7 +3731,8 @@ iavf_resume_pending_start(struct rte_eth_dev *dev)
 	if (!vf->start_pending)
 		return;
 
-	if (vf->vf_reset || vf->in_reset_recovery)
+	if (vf->vf_reset || rte_atomic_load_explicit(&vf->in_reset_recovery,
+			rte_memory_order_acquire))
 		return;
 
 	/*

drivers/net/intel/iavf/iavf_rxtx.c +1 -1

index fc47a2cf1a..84b931e78a 100644
@@ -1106,7 +1106,7 @@ iavf_stop_queues(struct rte_eth_dev *dev)
 	int ret;
 
 	/* adminq will be disabled when vf is resetting. */
-	if (vf->in_reset_recovery) {
+	if (rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed)) {
 		iavf_reset_queues(dev);
 		return;
 	}

drivers/net/intel/iavf/iavf_vchnl.c +2 -1

index 604399125f..6c054acf80 100644
@@ -260,7 +260,8 @@ iavf_handle_link_change_event(struct rte_eth_dev *dev,
 	 * (link is down or a VF reset is in progress); the watchdog drives
 	 * auto-reset recovery, so it must remain armed in those cases.
 	 */
-	if (vf->link_up && !vf->vf_reset && !vf->in_reset_recovery)
+	if (vf->link_up && !vf->vf_reset &&
+	    !rte_atomic_load_explicit(&vf->in_reset_recovery, rte_memory_order_relaxed))
 		iavf_dev_watchdog_disable(adapter);
 	else
 		iavf_dev_watchdog_enable(adapter);

Signature · 1 line

2.43.0

Bruce Richardson

Applied

2026-09-29 10:25 UTC #3 lore raw

Quoted 8 lines · Ciara Loftus

iavf_handle_hw_reset() can be called concurrently from the iavf-event thread handling RTE_ETH_EVENT_INTR_RESET and from any thread calling rte_pmd_iavf_reinit() (VF-initiated reset), with no synchronisation between the two. Make in_reset_recovery atomic and use it as a guard so only one reset runs at a time.
Fixes 28a1a72eac26 ("net/iavf: add VF initiated reset")

Quoted 7 lines

Signed-off-by Ciara Loftus <[email protected]>
Acked-by Anatoly Burakov <[email protected]>
---
v2:
* Use acquire ordering in iavf_resume_pending_start()
* Clear pf_reset_in_progress before the release store
---

Applied to dpdk-next-net-intel.
Thanks,
/Bruce