[PATCH 0/9] net/gve: support proactive device reset

[PATCH 0/9] net/gve: support proactive device reset

[Jasper Tran O'Leary]

This series adds support for proactive device reset
(RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE) to the GVE driver, replacing the
existing passive reset implementation. When the device requests a reset,
the driver now automatically recovers the control plane and recreates RX
and TX queues without requiring the application to manually reconfigure
the port.

To support automatic recovery safely while traffic is running, this
series:
- Fixes a pre-existing leak of the flow subsystem on device init
  failure.
- Saves RX and TX queue setup parameters and versions queue memzone
  names by reset generation so queues can be recreated before old queue
  memory is freed.
- Defers freeing retired queue memory ("zombie" queues) until after a
  grace period so concurrent datapath threads can safely drain after
  packet burst functions are redirected to dummy handlers.
- Splits device resource setup and teardown into host memory allocation
  and hardware configuration so host DMA memory can be kept across
  resets and re-registered with the device.
- Adds a reset mutex and moves device status polling off the shared DPDK
  alarm/interrupt thread onto a dedicated control thread.
- Implements proactive recovery and emits the standard DPDK recovery
  lifecycle events (RTE_ETH_EVENT_ERR_RECOVERING,
  RTE_ETH_EVENT_RECOVERY_SUCCESS, and RTE_ETH_EVENT_RECOVERY_FAILED).

Jasper Tran O'Leary (8):
  net/gve: free flow subsystem on dev init failure
  net/gve: snapshot queue configurations
  net/gve: add deferred queue memory reclamation
  net/gve: separate device resource alloc and config
  net/gve: add dedicated mutex for device reset
  net/gve: add check for device reset to AQ ctrl ops
  net/gve: implement proactive device reset
  net/gve: remove unused device reset parameter

Joshua Washington (1):
  net/gve: move dev status poller to its own thread

 doc/guides/nics/gve.rst                |  67 +-
 doc/guides/rel_notes/release_26_11.rst |   5 +
 drivers/net/gve/base/gve.h             |   1 +
 drivers/net/gve/gve_ethdev.c           | 775 ++++++++++++++------
 drivers/net/gve/gve_ethdev.h           |  87 ++-
 drivers/net/gve/gve_rx.c               |  36 +-
 drivers/net/gve/gve_rx_dqo.c           |  36 +-
 drivers/net/gve/gve_tx.c               |  32 +-
 drivers/net/gve/gve_tx_dqo.c           |  35 +-
 9 files changed, 813 insertions(+), 261 deletions(-)

-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 1/9] net/gve: free flow subsystem on dev init failure

[Jasper Tran O'Leary]

If setting up device resources fails during gve_init_priv, tear down the
flow subsystem if it was initialized to avoid leaking the flow rule
bitmap.

Fixes: 82ef40fb2e7e ("net/gve: support flow API")
Cc: stable@dpdk.org

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_ethdev.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 23a4c118b..90d8797b0 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -1747,8 +1747,13 @@ setup_device:
 	}
 
 	err = gve_setup_device_resources(priv);
-	if (!err)
-		return 0;
+	if (err)
+		goto free_flow_subsystem;
+
+	return 0;
+free_flow_subsystem:
+	if (gve_get_flow_subsystem_ok(priv))
+		gve_teardown_flow_subsystem(priv);
 free_adminq:
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
 	return err;
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 2/9] net/gve: snapshot queue configurations

[Jasper Tran O'Leary]

Snapshot RX and TX queue setup parameters in priv->rxq_configs and
priv->txq_configs during queue setup so that queues can be reconstructed
identically during a proactive reset recovery.
In addition, suffix DMA ring and resource memzone names with the current
reset generation number (rx_ring_g%u, tx_ring_g%u, etc.) so that new
memzones can be allocated during recovery before old memzones are
retired.

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_ethdev.c | 67 +++++++++++++++++++++++++------
 drivers/net/gve/gve_ethdev.h | 31 ++++++++++++++
 drivers/net/gve/gve_rx.c     | 36 ++++++++++++++---
 drivers/net/gve/gve_rx_dqo.c | 36 ++++++++++++++---
 drivers/net/gve/gve_tx.c     | 32 ++++++++++++---
 drivers/net/gve/gve_tx_dqo.c | 35 +++++++++++++---
 6 files changed, 201 insertions(+), 36 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 90d8797b0..47714844e 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -40,6 +40,24 @@ gve_alloc_using_mz(const char *name, uint32_t num_pages)
 	return mz;
 }
 
+static void
+gve_free_rxq(struct gve_priv *priv, struct gve_rx_queue *rxq)
+{
+	if (gve_is_gqi(priv))
+		gve_rx_queue_release_internal(rxq);
+	else
+		gve_rx_queue_release_internal_dqo(rxq);
+}
+
+static void
+gve_free_txq(struct gve_priv *priv, struct gve_tx_queue *txq)
+{
+	if (gve_is_gqi(priv))
+		gve_tx_queue_release_internal(txq);
+	else
+		gve_tx_queue_release_internal_dqo(txq);
+}
+
 static int
 gve_alloc_using_malloc(void **bufs, uint32_t num_entries)
 {
@@ -162,8 +180,9 @@ gve_setup_queue_page_list(struct gve_priv *priv, uint16_t queue_id, bool is_rx,
 	process_priv = rte_eth_devices[priv->port_id].process_private;
 
 	/* Allocate a new QPL. */
-	snprintf(qpl_name, sizeof(qpl_name), "gve_%s_%s_qpl%d",
-		priv->pci_dev->device.name, queue_type_string, queue_id);
+	snprintf(qpl_name, sizeof(qpl_name), "gve_p%u_%s_qpl%u_g%u",
+		 priv->port_id, queue_type_string, queue_id,
+		 priv->reset_generation);
 	qpl = gve_alloc_queue_page_list(qpl_name, num_pages, is_rx);
 	if (!qpl) {
 		PMD_DRV_LOG(ERR,
@@ -639,18 +658,14 @@ gve_free_queues(struct rte_eth_dev *dev)
 	struct gve_priv *priv = dev->data->dev_private;
 	uint16_t i;
 
-	if (gve_is_gqi(priv)) {
-		for (i = 0; i < dev->data->nb_tx_queues; i++)
-			gve_tx_queue_release(dev, i);
-
-		for (i = 0; i < dev->data->nb_rx_queues; i++)
-			gve_rx_queue_release(dev, i);
-	} else {
-		for (i = 0; i < dev->data->nb_tx_queues; i++)
-			gve_tx_queue_release_dqo(dev, i);
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		gve_free_txq(priv, dev->data->tx_queues[i]);
+		dev->data->tx_queues[i] = NULL;
+	}
 
-		for (i = 0; i < dev->data->nb_rx_queues; i++)
-			gve_rx_queue_release_dqo(dev, i);
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		gve_free_rxq(priv, dev->data->rx_queues[i]);
+		dev->data->rx_queues[i] = NULL;
 	}
 }
 
@@ -779,6 +794,11 @@ gve_dev_close(struct rte_eth_dev *dev)
 	pthread_mutex_destroy(&priv->flow_rule_lock);
 	pthread_mutex_destroy(&priv->nic_ts_lock);
 
+	rte_free(priv->rxq_configs);
+	priv->rxq_configs = NULL;
+	rte_free(priv->txq_configs);
+	priv->txq_configs = NULL;
+
 	dev->data->mac_addrs = NULL;
 
 	free(dev->process_private);
@@ -1735,6 +1755,21 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
 	PMD_DRV_LOG(INFO, "Max TX queues %d, Max RX queues %d",
 		    priv->max_nb_txq, priv->max_nb_rxq);
 
+	priv->rxq_configs =
+		rte_zmalloc("gve_rxq_configs",
+			    sizeof(struct gve_rxq_config) * priv->max_nb_rxq,
+			    RTE_CACHE_LINE_SIZE);
+
+	priv->txq_configs =
+		rte_zmalloc("gve_txq_configs",
+			    sizeof(struct gve_txq_config) * priv->max_nb_txq,
+			    RTE_CACHE_LINE_SIZE);
+
+	if (!priv->rxq_configs || !priv->txq_configs) {
+		err = -ENOMEM;
+		goto free_queue_configs;
+	}
+
 setup_device:
 	if (process_priv->ctrl_ops->query_rss != NULL)
 		priv->rss_cache_dirty = true;
@@ -1754,6 +1789,11 @@ setup_device:
 free_flow_subsystem:
 	if (gve_get_flow_subsystem_ok(priv))
 		gve_teardown_flow_subsystem(priv);
+free_queue_configs:
+	rte_free(priv->txq_configs);
+	priv->txq_configs = NULL;
+	rte_free(priv->rxq_configs);
+	priv->rxq_configs = NULL;
 free_adminq:
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
 	return err;
@@ -1814,6 +1854,7 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
 	priv->pci_dev = pci_dev;
 	priv->port_id = eth_dev->data->port_id;
 	priv->state_flags = 0x0;
+	priv->reset_generation = 0;
 
 	pthread_mutexattr_init(&mutexattr);
 	pthread_mutexattr_setpshared(&mutexattr, PTHREAD_PROCESS_SHARED);
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 19eac9ec0..8df4fa54c 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -279,6 +279,21 @@ enum gve_dev_clk_type {
 	GVE_DEV_CLK_CMD,
 };
 
+struct gve_rxq_config {
+	uint16_t nb_descriptors;
+	unsigned int socket_id;
+	struct rte_eth_rxconf conf;
+	struct rte_mempool *mb_pool;
+	bool allocated;
+};
+
+struct gve_txq_config {
+	uint16_t nb_descriptors;
+	unsigned int socket_id;
+	struct rte_eth_txconf conf;
+	bool allocated;
+};
+
 struct gve_ctrl_ops {
 	int (*init_ctrl_plane)(struct gve_priv *priv);
 	void (*free_ctrl_plane)(struct gve_priv *priv);
@@ -352,6 +367,10 @@ struct gve_priv {
 	uint16_t max_nb_rxq;
 	uint32_t num_ntfy_blks; /* spilt between TX and RX so must be even */
 
+	struct gve_rxq_config *rxq_configs;
+	struct gve_txq_config *txq_configs;
+	uint32_t reset_generation;
+
 	struct gve_registers __iomem *reg_bar0; /* see gve_register.h */
 	rte_be32_t __iomem *db_bar; /* "array" of doorbells */
 	struct rte_pci_device *pci_dev;
@@ -614,6 +633,18 @@ gve_tx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid);
 void
 gve_rx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid);
 
+void
+gve_rx_queue_release_internal(struct gve_rx_queue *rxq);
+
+void
+gve_rx_queue_release_internal_dqo(struct gve_rx_queue *rxq);
+
+void
+gve_tx_queue_release_internal(struct gve_tx_queue *txq);
+
+void
+gve_tx_queue_release_internal_dqo(struct gve_tx_queue *txq);
+
 int
 gve_rx_queue_start_dqo(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 
diff --git a/drivers/net/gve/gve_rx.c b/drivers/net/gve/gve_rx.c
index 7995560d3..f09ec40f8 100644
--- a/drivers/net/gve/gve_rx.c
+++ b/drivers/net/gve/gve_rx.c
@@ -274,10 +274,8 @@ gve_release_rxq_mbufs(struct gve_rx_queue *rxq)
 }
 
 void
-gve_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
+gve_rx_queue_release_internal(struct gve_rx_queue *q)
 {
-	struct gve_rx_queue *q = dev->data->rx_queues[qid];
-
 	if (!q)
 		return;
 
@@ -293,7 +291,21 @@ gve_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
 	rte_memzone_free(q->qres_mz);
 	q->qres = NULL;
 	rte_free(q);
+}
 
+void
+gve_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
+{
+	struct gve_rx_queue *q = dev->data->rx_queues[qid];
+	struct gve_priv *priv = dev->data->dev_private;
+
+	if (!q)
+		return;
+
+	if (priv->rxq_configs)
+		priv->rxq_configs[qid].allocated = false;
+
+	gve_rx_queue_release_internal(q);
 	dev->data->rx_queues[qid] = NULL;
 }
 
@@ -303,6 +315,7 @@ gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 		const struct rte_eth_rxconf *conf, struct rte_mempool *pool)
 {
 	struct gve_priv *hw = dev->data->dev_private;
+	char ring_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
 	struct gve_rx_queue *rxq;
 	uint16_t free_thresh;
@@ -366,7 +379,8 @@ gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 		goto err_rxq;
 	}
 
-	mz = rte_eth_dma_zone_reserve(dev, "rx_ring", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "rx_ring_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      nb_desc * sizeof(struct gve_rx_desc),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -378,7 +392,8 @@ gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 	rxq->rx_ring_phys_addr = mz->iova;
 	rxq->mz = mz;
 
-	mz = rte_eth_dma_zone_reserve(dev, "gve rx data ring", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "rx_data_ring_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      sizeof(union gve_rx_data_slot) * nb_desc,
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -402,7 +417,8 @@ gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 		}
 	}
 
-	mz = rte_eth_dma_zone_reserve(dev, "rxq_res", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "rxq_res_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      sizeof(struct gve_queue_resources),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -415,6 +431,14 @@ gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 
 	gve_reset_rxq(rxq);
 
+	if (hw->rxq_configs) {
+		hw->rxq_configs[queue_id].nb_descriptors = nb_desc;
+		hw->rxq_configs[queue_id].socket_id = socket_id;
+		hw->rxq_configs[queue_id].conf = *conf;
+		hw->rxq_configs[queue_id].mb_pool = pool;
+		hw->rxq_configs[queue_id].allocated = true;
+	}
+
 	dev->data->rx_queues[queue_id] = rxq;
 
 	return 0;
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 03c62edee..448025b94 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -266,10 +266,8 @@ gve_release_rxq_mbufs_dqo(struct gve_rx_queue *rxq)
 }
 
 void
-gve_rx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid)
+gve_rx_queue_release_internal_dqo(struct gve_rx_queue *q)
 {
-	struct gve_rx_queue *q = dev->data->rx_queues[qid];
-
 	if (q == NULL)
 		return;
 
@@ -282,7 +280,21 @@ gve_rx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid)
 	rte_memzone_free(q->qres_mz);
 	q->qres = NULL;
 	rte_free(q);
+}
 
+void
+gve_rx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid)
+{
+	struct gve_rx_queue *q = dev->data->rx_queues[qid];
+	struct gve_priv *priv = dev->data->dev_private;
+
+	if (q == NULL)
+		return;
+
+	if (priv->rxq_configs)
+		priv->rxq_configs[qid].allocated = false;
+
+	gve_rx_queue_release_internal_dqo(q);
 	dev->data->rx_queues[qid] = NULL;
 }
 
@@ -325,6 +337,7 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 		       struct rte_mempool *pool)
 {
 	struct gve_priv *hw = dev->data->dev_private;
+	char ring_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
 	struct gve_rx_queue *rxq;
 	uint16_t free_thresh;
@@ -409,7 +422,8 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 	}
 
 	/* Allocate RX buffer queue */
-	mz = rte_eth_dma_zone_reserve(dev, "rx_ring", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "rx_ring_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      nb_desc * sizeof(struct gve_rx_desc_dqo),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -422,7 +436,8 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 	rxq->mz = mz;
 
 	/* Allocate RX completion queue */
-	mz = rte_eth_dma_zone_reserve(dev, "compl_ring", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "rx_compl_ring_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      nb_desc * sizeof(struct gve_rx_compl_desc_dqo),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -436,7 +451,8 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 	rxq->compl_ring_phys_addr = mz->iova;
 	rxq->compl_ring_mz = mz;
 
-	mz = rte_eth_dma_zone_reserve(dev, "rxq_res", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "rxq_res_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      sizeof(struct gve_queue_resources),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -449,6 +465,14 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 
 	gve_reset_rx_ring_state_dqo(rxq);
 
+	if (hw->rxq_configs) {
+		hw->rxq_configs[queue_id].nb_descriptors = nb_desc;
+		hw->rxq_configs[queue_id].socket_id = socket_id;
+		hw->rxq_configs[queue_id].conf = *conf;
+		hw->rxq_configs[queue_id].mb_pool = pool;
+		hw->rxq_configs[queue_id].allocated = true;
+	}
+
 	dev->data->rx_queues[queue_id] = rxq;
 
 	return 0;
diff --git a/drivers/net/gve/gve_tx.c b/drivers/net/gve/gve_tx.c
index 47389836d..51c2fac09 100644
--- a/drivers/net/gve/gve_tx.c
+++ b/drivers/net/gve/gve_tx.c
@@ -537,10 +537,8 @@ gve_release_txq_mbufs(struct gve_tx_queue *txq)
 }
 
 void
-gve_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
+gve_tx_queue_release_internal(struct gve_tx_queue *q)
 {
-	struct gve_tx_queue *q = dev->data->tx_queues[qid];
-
 	if (!q)
 		return;
 
@@ -556,7 +554,21 @@ gve_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
 	rte_memzone_free(q->qres_mz);
 	q->qres = NULL;
 	rte_free(q);
+}
 
+void
+gve_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
+{
+	struct gve_tx_queue *q = dev->data->tx_queues[qid];
+	struct gve_priv *priv = dev->data->dev_private;
+
+	if (!q)
+		return;
+
+	if (priv->txq_configs)
+		priv->txq_configs[qid].allocated = false;
+
+	gve_tx_queue_release_internal(q);
 	dev->data->tx_queues[qid] = NULL;
 }
 
@@ -565,6 +577,7 @@ gve_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id, uint16_t nb_desc,
 		   unsigned int socket_id, const struct rte_eth_txconf *conf)
 {
 	struct gve_priv *hw = dev->data->dev_private;
+	char ring_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
 	struct gve_tx_queue *txq;
 	uint16_t free_thresh;
@@ -619,7 +632,8 @@ gve_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id, uint16_t nb_desc,
 		goto err_txq;
 	}
 
-	mz = rte_eth_dma_zone_reserve(dev, "tx_ring", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "tx_ring_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      nb_desc * sizeof(union gve_tx_desc),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -652,7 +666,8 @@ gve_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id, uint16_t nb_desc,
 		}
 	}
 
-	mz = rte_eth_dma_zone_reserve(dev, "txq_res", queue_id, sizeof(struct gve_queue_resources),
+	snprintf(ring_name, sizeof(ring_name), "txq_res_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id, sizeof(struct gve_queue_resources),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
 		PMD_DRV_LOG(ERR, "Failed to reserve DMA memory for TX resource");
@@ -664,6 +679,13 @@ gve_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id, uint16_t nb_desc,
 
 	gve_reset_txq(txq);
 
+	if (hw->txq_configs) {
+		hw->txq_configs[queue_id].nb_descriptors = nb_desc;
+		hw->txq_configs[queue_id].socket_id = socket_id;
+		hw->txq_configs[queue_id].conf = *conf;
+		hw->txq_configs[queue_id].allocated = true;
+	}
+
 	dev->data->tx_queues[queue_id] = txq;
 
 	return 0;
diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index e6c1cd29b..64470ce23 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -362,10 +362,8 @@ gve_release_txq_mbufs_dqo(struct gve_tx_queue *txq)
 }
 
 void
-gve_tx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid)
+gve_tx_queue_release_internal_dqo(struct gve_tx_queue *q)
 {
-	struct gve_tx_queue *q = dev->data->tx_queues[qid];
-
 	if (q == NULL)
 		return;
 
@@ -376,7 +374,21 @@ gve_tx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid)
 	rte_memzone_free(q->qres_mz);
 	q->qres = NULL;
 	rte_free(q);
+}
 
+void
+gve_tx_queue_release_dqo(struct rte_eth_dev *dev, uint16_t qid)
+{
+	struct gve_tx_queue *q = dev->data->tx_queues[qid];
+	struct gve_priv *priv = dev->data->dev_private;
+
+	if (q == NULL)
+		return;
+
+	if (priv->txq_configs)
+		priv->txq_configs[qid].allocated = false;
+
+	gve_tx_queue_release_internal_dqo(q);
 	dev->data->tx_queues[qid] = NULL;
 }
 
@@ -453,6 +465,7 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 		       const struct rte_eth_txconf *conf)
 {
 	struct gve_priv *hw = dev->data->dev_private;
+	char ring_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
 	struct gve_tx_queue *txq;
 	uint16_t free_thresh;
@@ -506,7 +519,8 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 	txq->sw_size = sw_size;
 
 	/* Allocate TX hardware ring descriptors. */
-	mz = rte_eth_dma_zone_reserve(dev, "tx_ring", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "tx_ring_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      nb_desc * sizeof(union gve_tx_desc_dqo),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -519,7 +533,8 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 	txq->mz = mz;
 
 	/* Allocate TX completion ring descriptors. */
-	mz = rte_eth_dma_zone_reserve(dev, "tx_compl_ring", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "tx_compl_ring_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				       txq->nb_complq_desc * sizeof(struct gve_tx_compl_desc),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -531,7 +546,8 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 	txq->compl_ring_phys_addr = mz->iova;
 	txq->compl_ring_mz = mz;
 
-	mz = rte_eth_dma_zone_reserve(dev, "txq_res", queue_id,
+	snprintf(ring_name, sizeof(ring_name), "txq_res_g%u", hw->reset_generation);
+	mz = rte_eth_dma_zone_reserve(dev, ring_name, queue_id,
 				      sizeof(struct gve_queue_resources),
 				      PAGE_SIZE, socket_id);
 	if (mz == NULL) {
@@ -544,6 +560,13 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
 
 	gve_reset_tx_ring_state_dqo(txq);
 
+	if (hw->txq_configs) {
+		hw->txq_configs[queue_id].nb_descriptors = nb_desc;
+		hw->txq_configs[queue_id].socket_id = socket_id;
+		hw->txq_configs[queue_id].conf = *conf;
+		hw->txq_configs[queue_id].allocated = true;
+	}
+
 	dev->data->tx_queues[queue_id] = txq;
 
 	return 0;
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 3/9] net/gve: add deferred queue memory reclamation

[Jasper Tran O'Leary]

Add zombie queue infrastructure to defer freeing retired queue memory
and descriptor rings across resets. This gives concurrent packet burst
workers a grace period before memzones are released, preventing
use-after-free.

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_ethdev.c | 93 ++++++++++++++++++++++++++++++-
 drivers/net/gve/gve_ethdev.h | 23 ++++++++
 2 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 47714844e..680fc9947 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -58,6 +58,76 @@ gve_free_txq(struct gve_priv *priv, struct gve_tx_queue *txq)
 		gve_tx_queue_release_internal_dqo(txq);
 }
 
+void
+gve_add_zombie_rx_queue(struct gve_priv *priv, struct gve_rx_queue *rxq)
+{
+	uint16_t queue_id;
+
+	if (!rxq)
+		return;
+
+	queue_id = rxq->queue_id;
+	if (queue_id >= priv->max_nb_rxq)
+		return;
+
+	if (priv->zombie_rx_queues[queue_id])
+		gve_free_rxq(priv, priv->zombie_rx_queues[queue_id]);
+
+	priv->zombie_rx_queues[queue_id] = rxq;
+
+	rxq->zombie_retire_time =
+		rte_get_timer_cycles() + (rte_get_timer_hz() * ZOMBIE_QUEUE_TTL_S);
+}
+
+void
+gve_add_zombie_tx_queue(struct gve_priv *priv, struct gve_tx_queue *txq)
+{
+	uint16_t queue_id;
+
+	if (!txq)
+		return;
+
+	queue_id = txq->queue_id;
+	if (queue_id >= priv->max_nb_txq)
+		return;
+
+	if (priv->zombie_tx_queues[queue_id])
+		gve_free_txq(priv, priv->zombie_tx_queues[queue_id]);
+
+	priv->zombie_tx_queues[queue_id] = txq;
+
+	txq->zombie_retire_time =
+		rte_get_timer_cycles() + (rte_get_timer_hz() * ZOMBIE_QUEUE_TTL_S);
+}
+
+void
+gve_clean_zombie_queues(struct gve_priv *priv, bool force)
+{
+	uint64_t now = rte_get_timer_cycles();
+	struct gve_rx_queue *rxq;
+	struct gve_tx_queue *txq;
+	int i;
+
+	if (!priv || !priv->zombie_rx_queues || !priv->zombie_tx_queues)
+		return;
+
+	for (i = 0; i < priv->max_nb_rxq; i++) {
+		rxq = priv->zombie_rx_queues[i];
+		if (rxq && (rxq->zombie_retire_time <= now || force)) {
+			gve_free_rxq(priv, rxq);
+			priv->zombie_rx_queues[i] = NULL;
+		}
+	}
+
+	for (i = 0; i < priv->max_nb_txq; i++) {
+		txq = priv->zombie_tx_queues[i];
+		if (txq && (txq->zombie_retire_time <= now || force)) {
+			gve_free_txq(priv, txq);
+			priv->zombie_tx_queues[i] = NULL;
+		}
+	}
+}
+
 static int
 gve_alloc_using_malloc(void **bufs, uint32_t num_entries)
 {
@@ -788,6 +858,12 @@ gve_dev_close(struct rte_eth_dev *dev)
 		gve_teardown_flow_subsystem(priv);
 
 	gve_free_queues(dev);
+	gve_clean_zombie_queues(priv, true);
+	rte_free(priv->zombie_rx_queues);
+	rte_free(priv->zombie_tx_queues);
+	priv->zombie_rx_queues = NULL;
+	priv->zombie_tx_queues = NULL;
+
 	gve_teardown_device_resources(priv);
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
 
@@ -1765,7 +1841,18 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
 			    sizeof(struct gve_txq_config) * priv->max_nb_txq,
 			    RTE_CACHE_LINE_SIZE);
 
-	if (!priv->rxq_configs || !priv->txq_configs) {
+	priv->zombie_rx_queues =
+		rte_zmalloc("gve_zombie_rxqs",
+			    sizeof(struct gve_rx_queue *) * priv->max_nb_rxq,
+			    RTE_CACHE_LINE_SIZE);
+
+	priv->zombie_tx_queues =
+		rte_zmalloc("gve_zombie_txqs",
+			    sizeof(struct gve_tx_queue *) * priv->max_nb_txq,
+			    RTE_CACHE_LINE_SIZE);
+
+	if (!priv->rxq_configs || !priv->txq_configs ||
+	    !priv->zombie_rx_queues || !priv->zombie_tx_queues) {
 		err = -ENOMEM;
 		goto free_queue_configs;
 	}
@@ -1790,6 +1877,10 @@ free_flow_subsystem:
 	if (gve_get_flow_subsystem_ok(priv))
 		gve_teardown_flow_subsystem(priv);
 free_queue_configs:
+	rte_free(priv->zombie_tx_queues);
+	priv->zombie_tx_queues = NULL;
+	rte_free(priv->zombie_rx_queues);
+	priv->zombie_rx_queues = NULL;
 	rte_free(priv->txq_configs);
 	priv->txq_configs = NULL;
 	rte_free(priv->rxq_configs);
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 8df4fa54c..c56a8a693 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -39,6 +39,8 @@
 #define GVE_RX_MIN_BUF_SIZE_GQI		2048
 #define GVE_RX_MAX_BUF_SIZE_GQI		4096
 
+#define ZOMBIE_QUEUE_TTL_S		5
+
 #define GVE_RSS_HASH_KEY_SIZE 40
 #define GVE_RSS_INDIR_SIZE 128
 
@@ -199,6 +201,10 @@ struct gve_tx_queue {
 	struct gve_tx_queue *complq;
 
 	uint8_t is_gqi_qpl;
+
+	/* For handling deferred freeing after device reset */
+	struct gve_tx_queue *zombie_next;
+	uint64_t zombie_retire_time;
 };
 
 struct gve_rx_ctx {
@@ -262,6 +268,10 @@ struct gve_rx_queue {
 
 	uint8_t is_gqi_qpl;
 	bool timestamp_enabled;
+
+	/* For handling deferred freeing after device reset */
+	struct gve_rx_queue *zombie_next;
+	uint64_t zombie_retire_time;
 };
 
 struct gve_flow {
@@ -371,6 +381,10 @@ struct gve_priv {
 	struct gve_txq_config *txq_configs;
 	uint32_t reset_generation;
 
+	struct gve_rx_queue **zombie_rx_queues;
+	struct gve_tx_queue **zombie_tx_queues;
+	rte_spinlock_t zombie_lock;
+
 	struct gve_registers __iomem *reg_bar0; /* see gve_register.h */
 	rte_be32_t __iomem *db_bar; /* "array" of doorbells */
 	struct rte_pci_device *pci_dev;
@@ -645,6 +659,15 @@ gve_tx_queue_release_internal(struct gve_tx_queue *txq);
 void
 gve_tx_queue_release_internal_dqo(struct gve_tx_queue *txq);
 
+void
+gve_add_zombie_rx_queue(struct gve_priv *priv, struct gve_rx_queue *rxq);
+
+void
+gve_add_zombie_tx_queue(struct gve_priv *priv, struct gve_tx_queue *txq);
+
+void
+gve_clean_zombie_queues(struct gve_priv *priv, bool force);
+
 int
 gve_rx_queue_start_dqo(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 4/9] net/gve: separate device resource alloc and config

[Jasper Tran O'Leary]

Separate gve_setup_device_resources into two distinct functions:
- gve_alloc_device_resources: allocates host DMA memory structures
  (counter array, IRQ doorbells, ptype LUT).
- gve_configure_device_resources: registers and configures those
  allocated resources with the device hardware.

Symmetrically, partition gve_teardown_device_resources into a function
for freeing memory and one for deconfiguring resources.

This clean separation allows device resources to be re-configured
with the hardware during device reset recovery without re-allocating
host memory.

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_ethdev.c | 172 +++++++++++++++++-------------
 1 file changed, 97 insertions(+), 75 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 680fc9947..44810b864 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -742,15 +742,20 @@ gve_free_queues(struct rte_eth_dev *dev)
 static void
 gve_free_counter_array(struct gve_priv *priv)
 {
-	rte_memzone_free(priv->cnt_array_mz);
-	priv->cnt_array = NULL;
+	if (priv->cnt_array_mz) {
+		rte_memzone_free(priv->cnt_array_mz);
+		priv->cnt_array_mz = NULL;
+		priv->cnt_array = NULL;
+	}
 }
-
 static void
 gve_free_irq_db(struct gve_priv *priv)
 {
-	rte_memzone_free(priv->irq_dbs_mz);
-	priv->irq_dbs = NULL;
+	if (priv->irq_dbs_mz) {
+		rte_memzone_free(priv->irq_dbs_mz);
+		priv->irq_dbs_mz = NULL;
+		priv->irq_dbs = NULL;
+	}
 }
 
 static void
@@ -805,33 +810,38 @@ gve_teardown_flow_subsystem(struct gve_priv *priv)
 }
 
 static void
-gve_teardown_device_resources(struct gve_priv *priv)
+gve_free_device_resources(struct gve_priv *priv)
+{
+	gve_free_nic_ts_report(priv);
+	gve_free_ptype_lut_dqo(priv);
+	gve_free_counter_array(priv);
+	gve_free_irq_db(priv);
+	gve_free_rss_config(&priv->rss_config);
+}
+
+static void
+gve_deconfigure_device_resources(struct gve_priv *priv)
 {
 	struct gve_process_private *process_priv;
 	int err;
 
-	process_priv = rte_eth_devices[priv->port_id].process_private;
-
-	/* Tell device its resources are being freed */
-	if (gve_get_device_resources_ok(priv)) {
-		err = process_priv->ctrl_ops->free_db_resources(priv);
-		if (err)
-			PMD_DRV_LOG(ERR,
-				"Could not deconfigure device resources: err=%d",
-				err);
-	}
+	if (!gve_get_device_resources_ok(priv))
+		return;
 
 	if (priv->nic_ts_report_mz) {
 		rte_atomic_store_explicit(&priv->nic_ts_thread_should_stop, 1,
 					  rte_memory_order_relaxed);
 		rte_thread_join(priv->nic_ts_thread_id, NULL);
-		gve_free_nic_ts_report(priv);
 	}
 
-	gve_free_ptype_lut_dqo(priv);
-	gve_free_counter_array(priv);
-	gve_free_irq_db(priv);
-	gve_free_rss_config(&priv->rss_config);
+	process_priv = rte_eth_devices[priv->port_id].process_private;
+	if (process_priv->ctrl_ops->free_db_resources) {
+		err = process_priv->ctrl_ops->free_db_resources(priv);
+		if (err)
+			PMD_DRV_LOG(ERR,
+				    "Could not deconfigure device resources: err=%d",
+				    err);
+	}
 	gve_clear_device_resources_ok(priv);
 }
 
@@ -864,8 +874,9 @@ gve_dev_close(struct rte_eth_dev *dev)
 	priv->zombie_rx_queues = NULL;
 	priv->zombie_tx_queues = NULL;
 
-	gve_teardown_device_resources(priv);
+	gve_deconfigure_device_resources(priv);
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
+	gve_free_device_resources(priv);
 
 	pthread_mutex_destroy(&priv->flow_rule_lock);
 	pthread_mutex_destroy(&priv->nic_ts_lock);
@@ -907,8 +918,9 @@ gve_dev_reset(struct rte_eth_dev *dev)
 	 * destroyed on dev_close.
 	 */
 	gve_free_queues(dev);
-	gve_teardown_device_resources(priv);
+	gve_deconfigure_device_resources(priv);
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
+	gve_free_device_resources(priv);
 
 	err = gve_init_priv(priv, true);
 	if (err != 0) {
@@ -1574,36 +1586,27 @@ gve_setup_nic_timestamp(struct gve_priv *priv)
 {
 	int err;
 
-	if (priv->clk_read_type == GVE_DEV_CLK_UNSUPPORTED)
+	if (!priv->nic_ts_report_mz)
 		return;
 
 	rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0, rte_memory_order_relaxed);
 	rte_atomic_store_explicit(&priv->nic_ts_stale, 1, rte_memory_order_relaxed);
-	err = gve_alloc_nic_ts_report(priv);
-	if (err == 0) {
-		gve_read_nic_clock(priv);
-		rte_atomic_store_explicit(&priv->nic_ts_thread_should_stop, 0,
-					  rte_memory_order_relaxed);
-		err = rte_thread_create_internal_control(&priv->nic_ts_thread_id, "gve-ts",
-							 gve_nic_ts_thread, priv);
-		if (err != 0) {
-			PMD_DRV_LOG(ERR, "Failed to create NIC clock sync thread, err=%d", err);
-			gve_free_nic_ts_report(priv);
-		}
-	} else {
-		PMD_DRV_LOG(ERR, "Failed to allocate memory for NIC timestamping subsystem. Please reset device to retry.");
+	gve_read_nic_clock(priv);
+	rte_atomic_store_explicit(&priv->nic_ts_thread_should_stop, 0,
+				  rte_memory_order_relaxed);
+	err = rte_thread_create_internal_control(&priv->nic_ts_thread_id, "gve-ts",
+						 gve_nic_ts_thread, priv);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Failed to create NIC clock sync thread, err=%d", err);
+		gve_free_nic_ts_report(priv);
 	}
 }
 
 static int
-gve_setup_device_resources(struct gve_priv *priv)
+gve_alloc_device_resources(struct gve_priv *priv)
 {
-	struct gve_process_private *process_priv;
 	char z_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
-	int err = 0;
-
-	process_priv = rte_eth_devices[priv->port_id].process_private;
 
 	snprintf(z_name, sizeof(z_name), "gve_%s_cnt_arr", priv->pci_dev->device.name);
 	mz = rte_memzone_reserve_aligned(z_name,
@@ -1619,34 +1622,56 @@ gve_setup_device_resources(struct gve_priv *priv)
 
 	snprintf(z_name, sizeof(z_name), "gve_%s_irqmz", priv->pci_dev->device.name);
 	mz = rte_memzone_reserve_aligned(z_name,
-					 sizeof(*priv->irq_dbs) * (priv->num_ntfy_blks),
+					 sizeof(*priv->irq_dbs) * priv->num_ntfy_blks,
 					 rte_socket_id(), RTE_MEMZONE_IOVA_CONTIG,
 					 PAGE_SIZE);
 	if (mz == NULL) {
 		PMD_DRV_LOG(ERR, "Could not alloc memzone for irq_dbs");
-		err = -ENOMEM;
-		goto free_cnt_array;
+		goto err_free;
 	}
 	priv->irq_dbs = (struct gve_irq_db *)mz->addr;
 	priv->irq_dbs_mz = mz;
 
-	err = process_priv->ctrl_ops->get_interrupt_dbs(priv);
-	if (unlikely(err)) {
-		PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", err);
-		goto free_irq_dbs;
-	}
 	if (!gve_is_gqi(priv)) {
 		priv->ptype_lut_dqo = rte_zmalloc("gve_ptype_lut_dqo",
 			sizeof(struct gve_ptype_lut), 0);
 		if (priv->ptype_lut_dqo == NULL) {
 			PMD_DRV_LOG(ERR, "Failed to alloc ptype lut.");
-			err = -ENOMEM;
-			goto free_irq_dbs;
+			goto err_free;
 		}
+	}
+
+	if (priv->clk_read_type != GVE_DEV_CLK_UNSUPPORTED) {
+		if (gve_alloc_nic_ts_report(priv) != 0)
+			PMD_DRV_LOG(ERR, "Failed to allocate memory for NIC timestamping subsystem.");
+	}
+
+	return 0;
+
+err_free:
+	gve_free_device_resources(priv);
+	return -ENOMEM;
+}
+
+static int
+gve_configure_device_resources(struct gve_priv *priv)
+{
+	struct gve_process_private *process_priv;
+	int err;
+
+	process_priv = rte_eth_devices[priv->port_id].process_private;
+
+	err = process_priv->ctrl_ops->get_interrupt_dbs(priv);
+	if (unlikely(err)) {
+		PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", err);
+		return err;
+	}
+
+	if (!gve_is_gqi(priv)) {
 		err = process_priv->ctrl_ops->get_ptype_map(priv);
 		if (unlikely(err)) {
 			PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d", err);
-			goto free_ptype_lut;
+			return err;
 		}
 	}
 	gve_setup_nic_timestamp(priv);
@@ -1654,15 +1679,6 @@ gve_setup_device_resources(struct gve_priv *priv)
 	gve_set_device_resources_ok(priv);
 
 	return 0;
-free_ptype_lut:
-	rte_free(priv->ptype_lut_dqo);
-	priv->ptype_lut_dqo = NULL;
-free_irq_dbs:
-	gve_free_irq_db(priv);
-free_cnt_array:
-	gve_free_counter_array(priv);
-
-	return err;
 }
 
 static void
@@ -1797,19 +1813,19 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
 	err = process_priv->ctrl_ops->get_device_properties(priv);
 	if (err) {
 		PMD_DRV_LOG(ERR, "Could not get device information: err=%d", err);
-		goto free_adminq;
+		goto free_ctrl_plane;
 	}
 
 	num_ntfy = pci_dev_msix_vec_count(priv->pci_dev);
 	if (num_ntfy <= 0) {
 		PMD_DRV_LOG(ERR, "Could not count MSI-x vectors");
 		err = -EIO;
-		goto free_adminq;
+		goto free_ctrl_plane;
 	} else if (num_ntfy < GVE_MIN_MSIX) {
 		PMD_DRV_LOG(ERR, "GVE needs at least %d MSI-x vectors, but only has %d",
 			    GVE_MIN_MSIX, num_ntfy);
 		err = -EINVAL;
-		goto free_adminq;
+		goto free_ctrl_plane;
 	}
 
 	priv->num_registered_pages = 0;
@@ -1867,8 +1883,11 @@ setup_device:
 				    "Failed to set up flow subsystem: err=%d, flow steering will be disabled.",
 				    err);
 	}
+	err = gve_alloc_device_resources(priv);
+	if (err)
+		goto free_flow_subsystem;
 
-	err = gve_setup_device_resources(priv);
+	err = gve_configure_device_resources(priv);
 	if (err)
 		goto free_flow_subsystem;
 
@@ -1877,16 +1896,19 @@ free_flow_subsystem:
 	if (gve_get_flow_subsystem_ok(priv))
 		gve_teardown_flow_subsystem(priv);
 free_queue_configs:
-	rte_free(priv->zombie_tx_queues);
-	priv->zombie_tx_queues = NULL;
-	rte_free(priv->zombie_rx_queues);
-	priv->zombie_rx_queues = NULL;
-	rte_free(priv->txq_configs);
-	priv->txq_configs = NULL;
-	rte_free(priv->rxq_configs);
-	priv->rxq_configs = NULL;
-free_adminq:
+	if (!skip_describe_device) {
+		rte_free(priv->zombie_tx_queues);
+		priv->zombie_tx_queues = NULL;
+		rte_free(priv->zombie_rx_queues);
+		priv->zombie_rx_queues = NULL;
+		rte_free(priv->txq_configs);
+		priv->txq_configs = NULL;
+		rte_free(priv->rxq_configs);
+		priv->rxq_configs = NULL;
+	}
+free_ctrl_plane:
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
+	gve_free_device_resources(priv);
 	return err;
 }
 
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 5/9] net/gve: add dedicated mutex for device reset

[Jasper Tran O'Leary]

During a proactive device reset, a polling thread may attempt to stop
and start the device outside of the main EAL thread. Guard against
contention by having device start, stop, and reset callbacks acquire
and release the mutex.

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_ethdev.c | 35 +++++++++++++++++++++++++------
 drivers/net/gve/gve_ethdev.h |  2 ++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 44810b864..391e7923f 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -558,16 +558,19 @@ gve_dev_start(struct rte_eth_dev *dev)
 		return -EPERM;
 	}
 
+	priv = dev->data->dev_private;
+
+	pthread_mutex_lock(&priv->reset_lock);
+
 	ret = gve_start_queues(dev);
 	if (ret != 0) {
 		PMD_DRV_LOG(ERR, "Failed to start queues");
-		return ret;
+		goto unlock_and_return;
 	}
 
 	dev->data->dev_started = 1;
 	gve_link_update(dev, 0);
 
-	priv = dev->data->dev_private;
 	/* No stats available yet for Dqo. */
 	if (gve_is_gqi(priv)) {
 		ret = gve_alloc_stats_report(priv,
@@ -576,7 +579,7 @@ gve_dev_start(struct rte_eth_dev *dev)
 		if (ret != 0) {
 			PMD_DRV_LOG(ERR,
 				"Failed to allocate region for stats reporting.");
-			return ret;
+			goto unlock_and_return;
 		}
 		ret = process_priv->ctrl_ops->setup_stats_report(priv,
 				priv->stats_report_len,
@@ -584,13 +587,16 @@ gve_dev_start(struct rte_eth_dev *dev)
 				GVE_STATS_REPORT_TIMER_PERIOD);
 		if (ret != 0) {
 			PMD_DRV_LOG(ERR, "setup_stats_report command failed.");
-			return ret;
+			goto unlock_and_return;
 		}
 	}
 
 	gve_start_dev_status_polling(dev);
+	ret = 0;
 
-	return 0;
+unlock_and_return:
+	pthread_mutex_unlock(&priv->reset_lock);
+	return ret;
 }
 
 static void
@@ -700,10 +706,14 @@ gve_dev_stop(struct rte_eth_dev *dev)
 
 	/*
 	 * Block until all polling callbacks have concluded before tearing down
-	 * any device resources.
+	 * any device resources. Do so before trying to acquire the reset lock
+	 * because a polling callback could be invoking a reset which will
+	 * itself try to acquire the lock.
 	 */
 	gve_stop_dev_status_polling(dev);
 
+	pthread_mutex_lock(&priv->reset_lock);
+
 	dev->data->dev_started = 0;
 	dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN;
 
@@ -719,6 +729,8 @@ gve_dev_stop(struct rte_eth_dev *dev)
 	if (gve_is_gqi(dev->data->dev_private))
 		gve_free_stats_report(dev);
 
+	pthread_mutex_unlock(&priv->reset_lock);
+
 	return 0;
 }
 
@@ -880,6 +892,7 @@ gve_dev_close(struct rte_eth_dev *dev)
 
 	pthread_mutex_destroy(&priv->flow_rule_lock);
 	pthread_mutex_destroy(&priv->nic_ts_lock);
+	pthread_mutex_destroy(&priv->reset_lock);
 
 	rte_free(priv->rxq_configs);
 	priv->rxq_configs = NULL;
@@ -908,6 +921,8 @@ gve_dev_reset(struct rte_eth_dev *dev)
 		return -EPERM;
 	}
 
+	pthread_mutex_lock(&priv->reset_lock);
+
 	/* Tear down all device resources before re-initializing. */
 	if (gve_get_flow_subsystem_ok(priv))
 		gve_teardown_flow_subsystem(priv);
@@ -927,9 +942,12 @@ gve_dev_reset(struct rte_eth_dev *dev)
 		PMD_DRV_LOG(ERR,
 			"Failed to re-init device on port %u after reset.",
 			dev->data->port_id);
+		pthread_mutex_unlock(&priv->reset_lock);
 		return err;
 	}
 
+	pthread_mutex_unlock(&priv->reset_lock);
+
 	return 0;
 }
 
@@ -1971,8 +1989,12 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
 
 	pthread_mutexattr_init(&mutexattr);
 	pthread_mutexattr_setpshared(&mutexattr, PTHREAD_PROCESS_SHARED);
+	pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_RECURSIVE);
+
 	pthread_mutex_init(&priv->flow_rule_lock, &mutexattr);
 	pthread_mutex_init(&priv->nic_ts_lock, &mutexattr);
+	pthread_mutex_init(&priv->reset_lock, &mutexattr);
+
 	pthread_mutexattr_destroy(&mutexattr);
 
 	priv->mbuf_timestamp_offset = -1;
@@ -1980,6 +2002,7 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
 	if (err) {
 		pthread_mutex_destroy(&priv->flow_rule_lock);
 		pthread_mutex_destroy(&priv->nic_ts_lock);
+		pthread_mutex_destroy(&priv->reset_lock);
 		goto free_process_priv;
 	}
 
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index c56a8a693..d16db54a8 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -385,6 +385,8 @@ struct gve_priv {
 	struct gve_tx_queue **zombie_tx_queues;
 	rte_spinlock_t zombie_lock;
 
+	pthread_mutex_t reset_lock;
+
 	struct gve_registers __iomem *reg_bar0; /* see gve_register.h */
 	rte_be32_t __iomem *db_bar; /* "array" of doorbells */
 	struct rte_pci_device *pci_dev;
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 6/9] net/gve: add check for device reset to AQ ctrl ops

[Jasper Tran O'Leary]

Refactor the check for the device needing a reset into a control plane
operation. This will make it easier to add a corresponding Mailbox
operation in subsequent patches.

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_ethdev.c | 14 +++++++++++---
 drivers/net/gve/gve_ethdev.h |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 391e7923f..cfd043dbf 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -1711,14 +1711,14 @@ gve_set_default_ring_size_bounds(struct gve_priv *priv)
 static void
 gve_check_device_status(void *arg)
 {
+	struct gve_process_private *process_priv;
 	struct rte_eth_dev *dev = arg;
 	struct gve_priv *priv = dev->data->dev_private;
-	uint32_t dev_status;
 	int ret;
 
-	dev_status = ioread32be(&priv->reg_bar0->device_status);
+	process_priv = rte_eth_devices[priv->port_id].process_private;
 
-	if (dev_status & GVE_DEVICE_STATUS_RESET_MASK) {
+	if (process_priv->ctrl_ops->check_device_needs_reset(priv)) {
 		PMD_DRV_LOG(INFO,
 			"Device on port %u requests a reset. Stopping device status polling.",
 			dev->data->port_id);
@@ -1783,9 +1783,17 @@ gve_adminq_get_device_properties(struct gve_priv *priv)
 	return gve_adminq_describe_device(priv);
 }
 
+static bool
+gve_adminq_check_device_needs_reset(struct gve_priv *priv)
+{
+	uint32_t dev_status = ioread32be(&priv->reg_bar0->device_status);
+	return (dev_status & GVE_DEVICE_STATUS_RESET_MASK) != 0;
+}
+
 static const struct gve_ctrl_ops gve_adminq_ops = {
 	.init_ctrl_plane = gve_adminq_alloc,
 	.free_ctrl_plane = gve_adminq_free,
+	.check_device_needs_reset = gve_adminq_check_device_needs_reset,
 	.get_device_properties = gve_adminq_get_device_properties,
 	.get_ptype_map = gve_adminq_get_ptype_map_dqo,
 	.get_interrupt_dbs = gve_adminq_configure_device_resources,
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index d16db54a8..495b60c11 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -307,6 +307,7 @@ struct gve_txq_config {
 struct gve_ctrl_ops {
 	int (*init_ctrl_plane)(struct gve_priv *priv);
 	void (*free_ctrl_plane)(struct gve_priv *priv);
+	bool (*check_device_needs_reset)(struct gve_priv *priv);
 	int (*get_device_properties)(struct gve_priv *priv);
 	int (*get_ptype_map)(struct gve_priv *priv);
 	int (*get_interrupt_dbs)(struct gve_priv *priv);
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 7/9] net/gve: implement proactive device reset

[Jasper Tran O'Leary]

Report RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE in device info and monitor
for device reset requests during periodic status polling. When a reset
is detected, spawn a dedicated control recovery thread to re-establish
the control plane, re-create queues from saved configurations, redirect
the fast-path operations table to prevent concurrent crashes, and emit
proactive recovery DPDK lifecycle events (ERR_RECOVERING,
RECOVERY_SUCCESS, RECOVERY_FAILED).

Proactive device reset supercedes the previously implemented passive
device reset convention for the gve driver.

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 doc/guides/nics/gve.rst                |  67 ++--
 doc/guides/rel_notes/release_26_11.rst |   5 +
 drivers/net/gve/gve_ethdev.c           | 384 ++++++++++++++------
 3 files changed, 321 insertions(+), 135 deletions(-)

diff --git a/doc/guides/nics/gve.rst b/doc/guides/nics/gve.rst
index c5e9ab70d..1b7a72cf1 100644
--- a/doc/guides/nics/gve.rst
+++ b/doc/guides/nics/gve.rst
@@ -153,37 +153,60 @@ require the DQO queue format.
 Device Reset
 ^^^^^^^^^^^^
 
-The driver allows an application to reset the gVNIC device.
-This function will tear down and reinitialize the device's resources,
-including queues and administrative queues.
-
-It is the application's responsibility to reinitialize
-and restart the device after resetting it.
-
-In addition, the driver supports device-requested resets, which are triggered
-when the device encounters an unrecoverable error. The driver detects this via
-a device status register and raises an ``RTE_ETH_EVENT_INTR_RESET`` event.
-
-The application must register a callback to handle this event. The callback
-should handle the reset process (reset, reconfigure, restart).
+The driver supports proactive error recovery
+(``RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE``)
+for both device-requested resets and application-initiated resets
+via ``rte_eth_dev_reset()``.
+
+A background control thread periodically polls the device status register
+to detect when the device requests a reset.
+When a reset is triggered, the driver automatically recovers the port
+without requiring the application to reconfigure or restart the device:
+
+- Emits ``RTE_ETH_EVENT_ERR_RECOVERING`` and redirects Rx/Tx burst functions
+  to dummy handlers so concurrent datapath threads can continue polling safely.
+- Tears down and re-initializes the control plane,
+  re-registers device resources,
+  and recreates Rx and Tx queues from their saved configurations.
+- Restarts the device (if it was previously started),
+  restores the active Rx/Tx burst functions,
+  and emits ``RTE_ETH_EVENT_RECOVERY_SUCCESS``
+  (or ``RTE_ETH_EVENT_RECOVERY_FAILED`` if recovery fails).
+
+Applications do not need to call ``rte_eth_dev_reset()`` in response to a
+device-requested reset,
+but may optionally register callbacks for the recovery lifecycle events
+to monitor reset progress or handle unrecoverable failures:
 
 .. code-block:: c
 
    static int
-   dev_rst_req_callback(uint16_t port_id, enum rte_eth_event_type type,
-                        void *param, void *ret_param)
+   dev_recovery_callback(uint16_t port_id, enum rte_eth_event_type type,
+                         void *param, void *ret_param)
    {
        RTE_SET_USED(param);
        RTE_SET_USED(ret_param);
 
-       if (type == RTE_ETH_EVENT_INTR_RESET) {
-           printf("Device requested reset on port %u\n", port_id);
-           rte_eth_dev_reset(port_id);
-           /* Reconfigure and restart port ... */
+       switch (type) {
+       case RTE_ETH_EVENT_ERR_RECOVERING:
+           printf("Port %u: entering recovery\n", port_id);
+           break;
+       case RTE_ETH_EVENT_RECOVERY_SUCCESS:
+           printf("Port %u: recovery succeeded\n", port_id);
+           break;
+       case RTE_ETH_EVENT_RECOVERY_FAILED:
+           printf("Port %u: recovery failed\n", port_id);
+           break;
+       default:
+           break;
        }
        return 0;
    }
 
-   /* Register this callback in the main execution flow */
-   rte_eth_dev_callback_register(port_id, RTE_ETH_EVENT_INTR_RESET,
-                     dev_rst_req_callback, NULL);
+   /* Optionally register callbacks in the main execution flow */
+   rte_eth_dev_callback_register(port_id, RTE_ETH_EVENT_ERR_RECOVERING,
+                                 dev_recovery_callback, NULL);
+   rte_eth_dev_callback_register(port_id, RTE_ETH_EVENT_RECOVERY_SUCCESS,
+                                 dev_recovery_callback, NULL);
+   rte_eth_dev_callback_register(port_id, RTE_ETH_EVENT_RECOVERY_FAILED,
+                                 dev_recovery_callback, NULL);
diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index c62d9e897..abaad4677 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -146,6 +146,11 @@ New Features
   * Raised the maximum Tx and Rx ring descriptor counts
     from 4096/8192 to 16384 each.
 
+* **Updated Google Virtual Ethernet (gve) ethernet driver.**
+
+  * Added support for proactive device reset recovery
+    (``RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE``).
+
 * **Updated Intel iavf driver.**
 
   * Runtime Rx/Tx queue setup is now automatically disabled
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index cfd043dbf..66a77c123 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -13,7 +13,6 @@
 #include <ethdev_driver.h>
 #include <rte_cycles.h>
 
-static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device);
 static void gve_start_dev_status_polling(struct rte_eth_dev *dev);
 static void gve_stop_dev_status_polling(struct rte_eth_dev *dev);
 
@@ -907,50 +906,6 @@ gve_dev_close(struct rte_eth_dev *dev)
 	return err;
 }
 
-static int
-gve_dev_reset(struct rte_eth_dev *dev)
-{
-	struct gve_process_private *process_priv = dev->process_private;
-	struct gve_priv *priv = dev->data->dev_private;
-	int err;
-
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		PMD_DRV_LOG(ERR,
-			"Device reset on port %u not supported in secondary processes.",
-			dev->data->port_id);
-		return -EPERM;
-	}
-
-	pthread_mutex_lock(&priv->reset_lock);
-
-	/* Tear down all device resources before re-initializing. */
-	if (gve_get_flow_subsystem_ok(priv))
-		gve_teardown_flow_subsystem(priv);
-
-	/*
-	 * Note that gve_teardown_flow_subsystem does not destroy the
-	 * flow_rule_lock. The lock is preserved across device resets and only
-	 * destroyed on dev_close.
-	 */
-	gve_free_queues(dev);
-	gve_deconfigure_device_resources(priv);
-	process_priv->ctrl_ops->free_ctrl_plane(priv);
-	gve_free_device_resources(priv);
-
-	err = gve_init_priv(priv, true);
-	if (err != 0) {
-		PMD_DRV_LOG(ERR,
-			"Failed to re-init device on port %u after reset.",
-			dev->data->port_id);
-		pthread_mutex_unlock(&priv->reset_lock);
-		return err;
-	}
-
-	pthread_mutex_unlock(&priv->reset_lock);
-
-	return 0;
-}
-
 static int
 gve_verify_driver_compatibility(struct gve_priv *priv)
 {
@@ -1071,6 +1026,7 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->flow_type_rss_offloads = GVE_RTE_RSS_OFFLOAD_ALL;
 	dev_info->hash_key_size = GVE_RSS_HASH_KEY_SIZE;
 	dev_info->reta_size = GVE_RSS_INDIR_SIZE;
+	dev_info->err_handle_mode = RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE;
 
 	return 0;
 }
@@ -1529,63 +1485,6 @@ gve_read_clock(struct rte_eth_dev *dev, uint64_t *clock)
 	return 0;
 }
 
-static const struct eth_dev_ops gve_eth_dev_ops = {
-	.dev_configure        = gve_dev_configure,
-	.dev_start            = gve_dev_start,
-	.dev_stop             = gve_dev_stop,
-	.dev_close            = gve_dev_close,
-	.dev_reset            = gve_dev_reset,
-	.dev_infos_get        = gve_dev_info_get,
-	.rx_queue_setup       = gve_rx_queue_setup,
-	.tx_queue_setup       = gve_tx_queue_setup,
-	.rx_queue_release     = gve_rx_queue_release,
-	.tx_queue_release     = gve_tx_queue_release,
-	.rx_queue_start       = gve_rx_queue_start,
-	.tx_queue_start       = gve_tx_queue_start,
-	.rx_queue_stop        = gve_rx_queue_stop,
-	.tx_queue_stop        = gve_tx_queue_stop,
-	.flow_ops_get         = gve_flow_ops_get,
-	.link_update          = gve_link_update,
-	.stats_get            = gve_dev_stats_get,
-	.stats_reset          = gve_dev_stats_reset,
-	.mtu_set              = gve_dev_mtu_set,
-	.xstats_get           = gve_xstats_get,
-	.xstats_get_names     = gve_xstats_get_names,
-	.rss_hash_update      = gve_rss_hash_update,
-	.rss_hash_conf_get    = gve_rss_hash_conf_get,
-	.reta_update          = gve_rss_reta_update,
-	.reta_query           = gve_rss_reta_query,
-};
-
-static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
-	.dev_configure        = gve_dev_configure,
-	.dev_start            = gve_dev_start,
-	.dev_stop             = gve_dev_stop,
-	.dev_close            = gve_dev_close,
-	.dev_reset            = gve_dev_reset,
-	.dev_infos_get        = gve_dev_info_get,
-	.rx_queue_setup       = gve_rx_queue_setup_dqo,
-	.tx_queue_setup       = gve_tx_queue_setup_dqo,
-	.rx_queue_release     = gve_rx_queue_release_dqo,
-	.tx_queue_release     = gve_tx_queue_release_dqo,
-	.rx_queue_start       = gve_rx_queue_start_dqo,
-	.tx_queue_start       = gve_tx_queue_start_dqo,
-	.rx_queue_stop        = gve_rx_queue_stop_dqo,
-	.tx_queue_stop        = gve_tx_queue_stop_dqo,
-	.flow_ops_get         = gve_flow_ops_get,
-	.link_update          = gve_link_update,
-	.stats_get            = gve_dev_stats_get,
-	.stats_reset          = gve_dev_stats_reset,
-	.mtu_set              = gve_dev_mtu_set,
-	.xstats_get           = gve_xstats_get,
-	.xstats_get_names     = gve_xstats_get_names,
-	.rss_hash_update      = gve_rss_hash_update,
-	.rss_hash_conf_get    = gve_rss_hash_conf_get,
-	.reta_update          = gve_rss_reta_update,
-	.reta_query           = gve_rss_reta_query,
-	.read_clock           = gve_read_clock,
-};
-
 static int
 pci_dev_msix_vec_count(struct rte_pci_device *pdev)
 {
@@ -1708,6 +1607,171 @@ gve_set_default_ring_size_bounds(struct gve_priv *priv)
 	priv->min_rx_desc_cnt = GVE_DEFAULT_MIN_RX_RING_SIZE;
 }
 
+static int
+gve_recreate_queues(struct rte_eth_dev *dev)
+{
+	struct gve_priv *priv = dev->data->dev_private;
+	uint16_t port_id = dev->data->port_id;
+	uint16_t i;
+	int ret;
+
+	if (priv->rxq_configs == NULL || priv->txq_configs == NULL) {
+		PMD_DRV_LOG(ERR, "Port %u: Queue configurations not allocated",
+			    port_id);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		struct gve_rxq_config *rx_cfg = &priv->rxq_configs[i];
+
+		if (!rx_cfg->allocated)
+			continue;
+
+		ret = dev->dev_ops->rx_queue_setup(dev, i, rx_cfg->nb_descriptors,
+						   rx_cfg->socket_id,
+						   &rx_cfg->conf,
+						   rx_cfg->mb_pool);
+		if (ret != 0) {
+			PMD_DRV_LOG(ERR,
+				    "Port %u: Failed to re-create RX queue %u",
+				    port_id, i);
+			return ret;
+		}
+	}
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		struct gve_txq_config *tx_cfg = &priv->txq_configs[i];
+
+		if (!tx_cfg->allocated)
+			continue;
+
+		ret = dev->dev_ops->tx_queue_setup(dev, i, tx_cfg->nb_descriptors,
+						   tx_cfg->socket_id,
+						   &tx_cfg->conf);
+		if (ret != 0) {
+			PMD_DRV_LOG(ERR,
+				    "Port %u: Failed to re-create TX queue %u",
+				    port_id, i);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int
+gve_internal_recover_device(struct rte_eth_dev *dev)
+{
+	struct gve_process_private *process_priv = dev->process_private;
+	struct gve_priv *priv = dev->data->dev_private;
+	uint16_t port_id = dev->data->port_id;
+	uint16_t i;
+	int ret;
+
+	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_ERR_RECOVERING, NULL);
+
+	rte_eth_fp_ops[port_id].rx_pkt_burst = rte_eth_pkt_burst_dummy;
+	rte_eth_fp_ops[port_id].tx_pkt_burst = rte_eth_pkt_burst_dummy;
+	dev->rx_pkt_burst = rte_eth_pkt_burst_dummy;
+	dev->tx_pkt_burst = rte_eth_pkt_burst_dummy;
+
+	/* Ensure dummy burst fns are visible before clearing queue pointers. */
+	rte_atomic_thread_fence(rte_memory_order_release);
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		if (dev->data->rx_queues[i] != NULL) {
+			gve_add_zombie_rx_queue(priv, dev->data->rx_queues[i]);
+			dev->data->rx_queues[i] = NULL;
+		}
+	}
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		if (dev->data->tx_queues[i] != NULL) {
+			gve_add_zombie_tx_queue(priv, dev->data->tx_queues[i]);
+			dev->data->tx_queues[i] = NULL;
+		}
+	}
+
+	gve_stop_dev_status_polling(dev);
+
+	if (gve_is_gqi(priv))
+		gve_free_stats_report(dev);
+
+	gve_deconfigure_device_resources(priv);
+
+	process_priv->ctrl_ops->free_ctrl_plane(priv);
+
+	/*
+	 * Keep device resources allocated, except for the RSS config which must
+	 * be freed as the driver cache will be stale after reset.
+	 */
+	gve_free_rss_config(&priv->rss_config);
+	if (process_priv->ctrl_ops->query_rss != NULL)
+		priv->rss_cache_dirty = true;
+
+	priv->reset_generation++;
+	priv->num_registered_pages = 0;
+	ret = process_priv->ctrl_ops->init_ctrl_plane(priv);
+	if (ret != 0) {
+		PMD_DRV_LOG(ERR,
+			    "Port %u: Failed to re-init control plane after reset",
+			    port_id);
+		goto recover_fail;
+	}
+
+	ret = gve_configure_device_resources(priv);
+	if (ret != 0)
+		goto recover_fail;
+
+	ret = gve_recreate_queues(dev);
+	if (ret != 0)
+		goto recover_fail;
+
+	ret = gve_dev_start(dev);
+	if (ret != 0) {
+		PMD_DRV_LOG(ERR,
+			    "Port %u: Failed to restart device after reset",
+			    port_id);
+		goto recover_fail;
+	}
+
+	if (gve_is_gqi(priv)) {
+		gve_set_rx_function(dev);
+		gve_set_tx_function(dev);
+	} else {
+		gve_set_rx_function_dqo(dev);
+		gve_set_tx_function_dqo(dev);
+	}
+
+	rte_eth_fp_ops[port_id].rxq.data = dev->data->rx_queues;
+	rte_eth_fp_ops[port_id].txq.data = dev->data->tx_queues;
+
+	/* Ensure queue data pointers are visible before restoring burst fns. */
+	rte_atomic_thread_fence(rte_memory_order_release);
+
+	rte_eth_fp_ops[port_id].rx_pkt_burst = dev->rx_pkt_burst;
+	rte_eth_fp_ops[port_id].tx_pkt_burst = dev->tx_pkt_burst;
+
+	PMD_DRV_LOG(INFO, "Port %u: Proactive reset recovery successful",
+		    port_id);
+	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_RECOVERY_SUCCESS, NULL);
+
+	return 0;
+
+recover_fail:
+	gve_free_queues(dev);
+	gve_deconfigure_device_resources(priv);
+
+	process_priv->ctrl_ops->free_ctrl_plane(priv);
+
+	rte_eth_fp_ops[port_id].rx_pkt_burst = rte_eth_pkt_burst_dummy;
+	rte_eth_fp_ops[port_id].tx_pkt_burst = rte_eth_pkt_burst_dummy;
+	PMD_DRV_LOG(ERR, "Port %u: Proactive reset recovery failed",
+		    port_id);
+	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_RECOVERY_FAILED, NULL);
+
+	return ret;
+}
+
 static void
 gve_check_device_status(void *arg)
 {
@@ -1716,22 +1780,39 @@ gve_check_device_status(void *arg)
 	struct gve_priv *priv = dev->data->dev_private;
 	int ret;
 
-	process_priv = rte_eth_devices[priv->port_id].process_private;
+	process_priv = dev->process_private;
+
+	gve_clean_zombie_queues(priv, false);
 
 	if (process_priv->ctrl_ops->check_device_needs_reset(priv)) {
-		PMD_DRV_LOG(INFO,
-			"Device on port %u requests a reset. Stopping device status polling.",
-			dev->data->port_id);
-		rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
-			NULL);
-	} else {
+		pthread_mutex_lock(&priv->reset_lock);
+
+		/*
+		 * Defend against TOCTOU by checking if a reset is still needed
+		 * after acquiring the lock.
+		 */
+		if (process_priv->ctrl_ops->check_device_needs_reset(priv)) {
+			PMD_DRV_LOG(NOTICE,
+				    "Port %u: Device requested reset. Executing proactive recovery.",
+				    dev->data->port_id);
+
+			ret = gve_internal_recover_device(dev);
+			if (ret == 0) {
+				pthread_mutex_unlock(&priv->reset_lock);
+				return;
+			}
+		}
+
+		pthread_mutex_unlock(&priv->reset_lock);
+	}
+
+	if (dev->data->dev_started) {
 		ret = rte_eal_alarm_set(GVE_DEV_POLL_INTERVAL_US,
 					gve_check_device_status, dev);
-		if (ret != 0) {
+		if (ret != 0)
 			PMD_DRV_LOG(ERR,
-				"Port %u: Failed to re-arm alarm poller!",
-				dev->data->port_id);
-		}
+				    "Port %u: Failed to re-arm alarm poller!",
+				    dev->data->port_id);
 	}
 }
 
@@ -1765,6 +1846,83 @@ gve_stop_dev_status_polling(struct rte_eth_dev *dev)
 	rte_eal_alarm_cancel(gve_check_device_status, dev);
 }
 
+static int
+gve_dev_reset(struct rte_eth_dev *dev)
+{
+	struct gve_priv *priv = dev->data->dev_private;
+	int ret;
+
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		PMD_DRV_LOG(ERR,
+			    "Device reset on port %u not supported in secondary processes.",
+			    dev->data->port_id);
+		return -EPERM;
+	}
+
+	pthread_mutex_lock(&priv->reset_lock);
+	ret = gve_internal_recover_device(dev);
+	pthread_mutex_unlock(&priv->reset_lock);
+
+	return ret;
+}
+
+static const struct eth_dev_ops gve_eth_dev_ops = {
+	.dev_configure        = gve_dev_configure,
+	.dev_start            = gve_dev_start,
+	.dev_stop             = gve_dev_stop,
+	.dev_close            = gve_dev_close,
+	.dev_reset            = gve_dev_reset,
+	.dev_infos_get        = gve_dev_info_get,
+	.rx_queue_setup       = gve_rx_queue_setup,
+	.tx_queue_setup       = gve_tx_queue_setup,
+	.rx_queue_release     = gve_rx_queue_release,
+	.tx_queue_release     = gve_tx_queue_release,
+	.rx_queue_start       = gve_rx_queue_start,
+	.tx_queue_start       = gve_tx_queue_start,
+	.rx_queue_stop        = gve_rx_queue_stop,
+	.tx_queue_stop        = gve_tx_queue_stop,
+	.flow_ops_get         = gve_flow_ops_get,
+	.link_update          = gve_link_update,
+	.stats_get            = gve_dev_stats_get,
+	.stats_reset          = gve_dev_stats_reset,
+	.mtu_set              = gve_dev_mtu_set,
+	.xstats_get           = gve_xstats_get,
+	.xstats_get_names     = gve_xstats_get_names,
+	.rss_hash_update      = gve_rss_hash_update,
+	.rss_hash_conf_get    = gve_rss_hash_conf_get,
+	.reta_update          = gve_rss_reta_update,
+	.reta_query           = gve_rss_reta_query,
+};
+
+static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
+	.dev_configure        = gve_dev_configure,
+	.dev_start            = gve_dev_start,
+	.dev_stop             = gve_dev_stop,
+	.dev_close            = gve_dev_close,
+	.dev_reset            = gve_dev_reset,
+	.dev_infos_get        = gve_dev_info_get,
+	.rx_queue_setup       = gve_rx_queue_setup_dqo,
+	.tx_queue_setup       = gve_tx_queue_setup_dqo,
+	.rx_queue_release     = gve_rx_queue_release_dqo,
+	.tx_queue_release     = gve_tx_queue_release_dqo,
+	.rx_queue_start       = gve_rx_queue_start_dqo,
+	.tx_queue_start       = gve_tx_queue_start_dqo,
+	.rx_queue_stop        = gve_rx_queue_stop_dqo,
+	.tx_queue_stop        = gve_tx_queue_stop_dqo,
+	.flow_ops_get         = gve_flow_ops_get,
+	.link_update          = gve_link_update,
+	.stats_get            = gve_dev_stats_get,
+	.stats_reset          = gve_dev_stats_reset,
+	.mtu_set              = gve_dev_mtu_set,
+	.xstats_get           = gve_xstats_get,
+	.xstats_get_names     = gve_xstats_get_names,
+	.rss_hash_update      = gve_rss_hash_update,
+	.rss_hash_conf_get    = gve_rss_hash_conf_get,
+	.reta_update          = gve_rss_reta_update,
+	.reta_query           = gve_rss_reta_query,
+	.read_clock           = gve_read_clock,
+};
+
 static int
 gve_adminq_get_device_properties(struct gve_priv *priv)
 {
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 8/9] net/gve: remove unused device reset parameter

[Jasper Tran O'Leary]

With proactive device reset introduced, gve_init_priv is now only called
during initial device probe with skip_describe_device set to false.
Remove the unused skip_describe_device parameter and its associated
conditional branches.

Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_ethdev.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 66a77c123..c2cfd18dc 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -1973,7 +1973,7 @@ static const struct gve_ctrl_ops gve_adminq_ops = {
 };
 
 static int
-gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
+gve_init_priv(struct gve_priv *priv)
 {
 	struct gve_process_private *process_priv;
 	int num_ntfy;
@@ -1987,9 +1987,6 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
 		return err;
 	}
 
-	if (skip_describe_device)
-		goto setup_device;
-
 	/* Set default descriptor counts */
 	gve_set_default_ring_size_bounds(priv);
 
@@ -2057,7 +2054,6 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
 		goto free_queue_configs;
 	}
 
-setup_device:
 	if (process_priv->ctrl_ops->query_rss != NULL)
 		priv->rss_cache_dirty = true;
 	if (priv->max_flow_rules) {
@@ -2080,16 +2076,14 @@ free_flow_subsystem:
 	if (gve_get_flow_subsystem_ok(priv))
 		gve_teardown_flow_subsystem(priv);
 free_queue_configs:
-	if (!skip_describe_device) {
-		rte_free(priv->zombie_tx_queues);
-		priv->zombie_tx_queues = NULL;
-		rte_free(priv->zombie_rx_queues);
-		priv->zombie_rx_queues = NULL;
-		rte_free(priv->txq_configs);
-		priv->txq_configs = NULL;
-		rte_free(priv->rxq_configs);
-		priv->rxq_configs = NULL;
-	}
+	rte_free(priv->zombie_tx_queues);
+	priv->zombie_tx_queues = NULL;
+	rte_free(priv->zombie_rx_queues);
+	priv->zombie_rx_queues = NULL;
+	rte_free(priv->txq_configs);
+	priv->txq_configs = NULL;
+	rte_free(priv->rxq_configs);
+	priv->rxq_configs = NULL;
 free_ctrl_plane:
 	process_priv->ctrl_ops->free_ctrl_plane(priv);
 	gve_free_device_resources(priv);
@@ -2164,7 +2158,7 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
 	pthread_mutexattr_destroy(&mutexattr);
 
 	priv->mbuf_timestamp_offset = -1;
-	err = gve_init_priv(priv, false);
+	err = gve_init_priv(priv);
 	if (err) {
 		pthread_mutex_destroy(&priv->flow_rule_lock);
 		pthread_mutex_destroy(&priv->nic_ts_lock);
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 9/9] net/gve: move dev status poller to its own thread

[Jasper Tran O'Leary]

From: Joshua Washington <joshwash@google.com>

The DPDK alarm and interrupts infrastructure make use of the same global
thread for processing. This causes problems in GVE related to the device
status poller, as the reset path invoked by that poller sends mailbox
messages and blocks waiting for their completion.

When the device status poller is on the alarm thread, that blocking call
leads to a temporary lockup, as the mailbox interrupt cannot come in
while the status poller holds the global alarm thread, causing message
timeouts. Because the interrupts infrastructure is managed by DPDK
itself, that leaves moving the device status poller off of the shared
global alarm/interrupt thread as the only viable option.

Spin off a device status thread on initialization that checks for
resets. The single thread will stay alive until the driver is torn down
in gve_dev_close.

Update the polling frequency to every 100ms for quicker error recovery,
since the poller is no longer on the shared thread.

Signed-off-by: Joshua Washington <joshwash@google.com>
Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
 drivers/net/gve/base/gve.h   |  1 +
 drivers/net/gve/gve_ethdev.c | 81 ++++++++++++++++---------------
 drivers/net/gve/gve_ethdev.h | 30 +++++++++++-
 3 files changed, 72 insertions(+), 40 deletions(-)

diff --git a/drivers/net/gve/base/gve.h b/drivers/net/gve/base/gve.h
index 18363fae3..4d9890625 100644
--- a/drivers/net/gve/base/gve.h
+++ b/drivers/net/gve/base/gve.h
@@ -52,6 +52,7 @@ enum gve_state_flags_bit {
 	GVE_PRIV_FLAGS_DEVICE_RINGS_OK		= 3,
 	GVE_PRIV_FLAGS_FLOW_SUBSYSTEM_OK	= 4,
 	GVE_PRIV_FLAGS_NAPI_ENABLED		= 5,
+	GVE_PRIV_FLAGS_STATUS_POLLER_OK		= 7,
 };
 
 enum gve_rss_hash_algorithm {
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index c2cfd18dc..7c335a72d 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -590,7 +590,6 @@ gve_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
-	gve_start_dev_status_polling(dev);
 	ret = 0;
 
 unlock_and_return:
@@ -703,14 +702,6 @@ gve_dev_stop(struct rte_eth_dev *dev)
 		return -EPERM;
 	}
 
-	/*
-	 * Block until all polling callbacks have concluded before tearing down
-	 * any device resources. Do so before trying to acquire the reset lock
-	 * because a polling callback could be invoking a reset which will
-	 * itself try to acquire the lock.
-	 */
-	gve_stop_dev_status_polling(dev);
-
 	pthread_mutex_lock(&priv->reset_lock);
 
 	dev->data->dev_started = 0;
@@ -869,6 +860,16 @@ gve_dev_close(struct rte_eth_dev *dev)
 		return 0;
 	}
 
+	/*
+	 * Block until all polling callbacks have concluded before tearing down
+	 * any device resources. Do so before trying to acquire the reset lock
+	 * in gve_dev_stop() because a polling callback could be invoking a
+	 * reset which will itself try to acquire the lock, leading to a
+	 * deadlock.
+	 */
+	if (gve_get_dev_status_poller_ok(priv))
+		gve_stop_dev_status_polling(dev);
+
 	if (dev->data->dev_started) {
 		err = gve_dev_stop(dev);
 		if (err != 0)
@@ -1691,8 +1692,6 @@ gve_internal_recover_device(struct rte_eth_dev *dev)
 		}
 	}
 
-	gve_stop_dev_status_polling(dev);
-
 	if (gve_is_gqi(priv))
 		gve_free_stats_report(dev);
 
@@ -1772,63 +1771,59 @@ recover_fail:
 	return ret;
 }
 
-static void
+static unsigned int
 gve_check_device_status(void *arg)
 {
 	struct gve_process_private *process_priv;
 	struct rte_eth_dev *dev = arg;
 	struct gve_priv *priv = dev->data->dev_private;
-	int ret;
 
 	process_priv = dev->process_private;
 
-	gve_clean_zombie_queues(priv, false);
+	while (gve_get_dev_status_poller_ok(priv)) {
+		gve_clean_zombie_queues(priv, false);
+
+		if (!process_priv->ctrl_ops->check_device_needs_reset(priv))
+			goto sleep;
 
-	if (process_priv->ctrl_ops->check_device_needs_reset(priv)) {
 		pthread_mutex_lock(&priv->reset_lock);
 
 		/*
 		 * Defend against TOCTOU by checking if a reset is still needed
 		 * after acquiring the lock.
 		 */
-		if (process_priv->ctrl_ops->check_device_needs_reset(priv)) {
-			PMD_DRV_LOG(NOTICE,
-				    "Port %u: Device requested reset. Executing proactive recovery.",
-				    dev->data->port_id);
-
-			ret = gve_internal_recover_device(dev);
-			if (ret == 0) {
-				pthread_mutex_unlock(&priv->reset_lock);
-				return;
-			}
-		}
+		if (!process_priv->ctrl_ops->check_device_needs_reset(priv))
+			goto unlock;
+
+		PMD_DRV_LOG(NOTICE,
+			    "Port %u: Device requested reset. Executing proactive recovery.",
+			    dev->data->port_id);
 
+		gve_internal_recover_device(dev);
+unlock:
 		pthread_mutex_unlock(&priv->reset_lock);
+sleep:
+		rte_delay_us_sleep(GVE_DEV_POLL_INTERVAL_US);
 	}
 
-	if (dev->data->dev_started) {
-		ret = rte_eal_alarm_set(GVE_DEV_POLL_INTERVAL_US,
-					gve_check_device_status, dev);
-		if (ret != 0)
-			PMD_DRV_LOG(ERR,
-				    "Port %u: Failed to re-arm alarm poller!",
-				    dev->data->port_id);
-	}
+	return 0;
 }
 
 static void
 gve_start_dev_status_polling(struct rte_eth_dev *dev)
 {
+	struct gve_priv *priv = dev->data->dev_private;
 	int ret;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return;
 
-	ret = rte_eal_alarm_set(GVE_DEV_POLL_INTERVAL_US,
-				gve_check_device_status,
-				dev);
-
+	gve_set_dev_status_poller_ok(priv);
+	ret = rte_thread_create_internal_control(&priv->dev_status_thread,
+						 "gve-dev-status",
+						 gve_check_device_status, dev);
 	if (ret != 0) {
+		gve_clear_dev_status_poller_ok(priv);
 		PMD_DRV_LOG(ERR,
 			"Port %u: Failed to arm device reset polling alarm! Err=%d",
 			dev->data->port_id, ret);
@@ -1842,8 +1837,12 @@ gve_start_dev_status_polling(struct rte_eth_dev *dev)
 static void
 gve_stop_dev_status_polling(struct rte_eth_dev *dev)
 {
+	struct gve_priv *priv = dev->data->dev_private;
+
 	/* Blocks until all in-progress callbacks have completed. */
-	rte_eal_alarm_cancel(gve_check_device_status, dev);
+	gve_clear_dev_status_poller_ok(priv);
+
+	rte_thread_join(priv->dev_status_thread, NULL);
 }
 
 static int
@@ -1859,9 +1858,11 @@ gve_dev_reset(struct rte_eth_dev *dev)
 		return -EPERM;
 	}
 
+	gve_stop_dev_status_polling(dev);
 	pthread_mutex_lock(&priv->reset_lock);
 	ret = gve_internal_recover_device(dev);
 	pthread_mutex_unlock(&priv->reset_lock);
+	gve_start_dev_status_polling(dev);
 
 	return ret;
 }
@@ -2178,6 +2179,8 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
 
 	eth_dev->data->mac_addrs = &priv->dev_addr;
 
+	gve_start_dev_status_polling(eth_dev);
+
 	return 0;
 
 free_process_priv:
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 495b60c11..560a9501b 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -66,7 +66,7 @@
 	RTE_ETH_RSS_NONFRAG_IPV6_UDP |	\
 	RTE_ETH_RSS_IPV6_UDP_EX)
 
-#define GVE_DEV_POLL_INTERVAL_US (1 * 1000 * 1000) /* 1 second in microseconds */
+#define GVE_DEV_POLL_INTERVAL_US (100 * 1000) /* 100ms in microseconds */
 
 /* A list of pages registered with the device during setup and used by a queue
  * as buffers
@@ -386,6 +386,7 @@ struct gve_priv {
 	struct gve_tx_queue **zombie_tx_queues;
 	rte_spinlock_t zombie_lock;
 
+	rte_thread_t dev_status_thread;
 	pthread_mutex_t reset_lock;
 
 	struct gve_registers __iomem *reg_bar0; /* see gve_register.h */
@@ -581,6 +582,33 @@ gve_clear_flow_subsystem_ok(struct gve_priv *priv)
 				&priv->state_flags);
 }
 
+static inline bool
+gve_get_dev_status_poller_ok(struct gve_priv *priv)
+{
+	bool ret;
+
+	ret = !!rte_bit_relaxed_get32(GVE_PRIV_FLAGS_STATUS_POLLER_OK,
+				      &priv->state_flags);
+	rte_atomic_thread_fence(rte_memory_order_acquire);
+	return ret;
+}
+
+static inline void
+gve_set_dev_status_poller_ok(struct gve_priv *priv)
+{
+	rte_atomic_thread_fence(rte_memory_order_release);
+	rte_bit_relaxed_set32(GVE_PRIV_FLAGS_STATUS_POLLER_OK,
+			      &priv->state_flags);
+}
+
+static inline void
+gve_clear_dev_status_poller_ok(struct gve_priv *priv)
+{
+	rte_atomic_thread_fence(rte_memory_order_release);
+	rte_bit_relaxed_clear32(GVE_PRIV_FLAGS_STATUS_POLLER_OK,
+				&priv->state_flags);
+}
+
 int
 gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id, uint16_t nb_desc,
 		   unsigned int socket_id, const struct rte_eth_rxconf *conf,
-- 
2.56.0.307.g5fe7bd680c-goog

Re: [PATCH 0/9] net/gve: support proactive device reset

[Stephen Hemminger]

On Tue, 27 Oct 2026 18:54:09 +0000
"Jasper Tran O'Leary" <jtranoleary@google.com> wrote:

> This series adds support for proactive device reset
> (RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE) to the GVE driver, replacing the
> existing passive reset implementation. When the device requests a reset,
> the driver now automatically recovers the control plane and recreates RX
> and TX queues without requiring the application to manually reconfigure
> the port.
> 
> To support automatic recovery safely while traffic is running, this
> series:
> - Fixes a pre-existing leak of the flow subsystem on device init
>   failure.
> - Saves RX and TX queue setup parameters and versions queue memzone
>   names by reset generation so queues can be recreated before old queue
>   memory is freed.
> - Defers freeing retired queue memory ("zombie" queues) until after a
>   grace period so concurrent datapath threads can safely drain after
>   packet burst functions are redirected to dummy handlers.
> - Splits device resource setup and teardown into host memory allocation
>   and hardware configuration so host DMA memory can be kept across
>   resets and re-registered with the device.
> - Adds a reset mutex and moves device status polling off the shared DPDK
>   alarm/interrupt thread onto a dedicated control thread.
> - Implements proactive recovery and emits the standard DPDK recovery
>   lifecycle events (RTE_ETH_EVENT_ERR_RECOVERING,
>   RTE_ETH_EVENT_RECOVERY_SUCCESS, and RTE_ETH_EVENT_RECOVERY_FAILED).
> 
> Jasper Tran O'Leary (8):
>   net/gve: free flow subsystem on dev init failure
>   net/gve: snapshot queue configurations
>   net/gve: add deferred queue memory reclamation
>   net/gve: separate device resource alloc and config
>   net/gve: add dedicated mutex for device reset
>   net/gve: add check for device reset to AQ ctrl ops
>   net/gve: implement proactive device reset
>   net/gve: remove unused device reset parameter
> 
> Joshua Washington (1):
>   net/gve: move dev status poller to its own thread
> 
>  doc/guides/nics/gve.rst                |  67 +-
>  doc/guides/rel_notes/release_26_11.rst |   5 +
>  drivers/net/gve/base/gve.h             |   1 +
>  drivers/net/gve/gve_ethdev.c           | 775 ++++++++++++++------
>  drivers/net/gve/gve_ethdev.h           |  87 ++-
>  drivers/net/gve/gve_rx.c               |  36 +-
>  drivers/net/gve/gve_rx_dqo.c           |  36 +-
>  drivers/net/gve/gve_tx.c               |  32 +-
>  drivers/net/gve/gve_tx_dqo.c           |  35 +-
>  9 files changed, 813 insertions(+), 261 deletions(-)
>

Lots of issues in AI review

Subject: Re: [PATCH 0/9] net/gve: proactive device reset

Series applied to main except 7/9, which needs a rebase: the
gve_ethdev.c include context (<rte_cycles.h>) and the reflowed
Device Reset text in gve.rst do not match current main. With the
context fixed up, every commit builds with -Dwerror=true.

The overall approach (snapshot queue config, zombie queues,
generation-suffixed memzones, dedicated poller thread) is reasonable.
The main problems are with the ethdev contracts for rte_eth_dev_reset()
and RTE_ETH_EVENT_RECOVERY_SUCCESS, and with datapath ordering on
weakly ordered CPUs (Axion/Arm instances are a real target for gve).


Errors
------

[PATCH 7/9] net/gve: implement proactive device reset

1. gve_internal_recover_device() calls gve_dev_start() unconditionally.

   - rte_eth_dev_reset() stops the port before calling dev_reset, and
     its documented contract is that the port comes back stopped and
     the application then calls configure/queue_setup/start. With
     this patch the port comes back started, so the application's
     rte_eth_dev_configure() fails with -EBUSY.
   - After 9/9 the poller runs from probe until close, so a
     device-requested reset on a stopped or never-configured port
     sets dev_started = 1 behind the application's back.
   - gve.rst says "Restarts the device (if it was previously
     started)", which the code does not do.

   Record the state on entry and only restart if it was started:

	bool was_started = dev->data->dev_started;
	...
	if (was_started) {
		ret = gve_dev_start(dev);
		...
	}

2. Configuration that RTE_ETH_EVENT_RECOVERY_SUCCESS promises to
   retain is lost. Per rte_ethdev.h, MTU and RSS configuration set
   via rte_eth_dev_rss_* must survive recovery. The recovery path
   frees priv->rss_config and never re-issues set_mtu or
   configure_rss, so the device silently reverts to its defaults.
   Replay dev->data->mtu and the saved RSS key/hash types/RETA
   after gve_configure_device_resources().

3. Flow rule state is left stale. The device loses its rules on
   reset and gve does not advertise RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP,
   but priv->active_flows and the rule-ID bitmap are kept. A later
   rte_flow_destroy() sends a destroy for a rule the device no longer
   has, and rule IDs stay marked in use. Either re-program the rules
   during recovery and set FLOW_RULE_KEEP, or tear down and re-init
   the flow subsystem as the old dev_reset path did.

4. Datapath ordering. The writer does:

	fp_ops.rx_pkt_burst = dummy;
	fence(release);
	dev->data->rx_queues[i] = NULL;

   rte_eth_rx_burst() loads rxq.data[queue_id] and rx_pkt_burst with
   no ordering between them. On Arm a reader can observe the NULL
   queue pointer together with the old gve_recv_pkts, and dereference
   NULL. A release fence on the writer side alone does not help.

   Do not store NULL into the live queue array during recovery. Build
   the new queue first, then publish it with a single store and hand
   the old one to the zombie list. Any reader then sees either the old
   (still mapped) queue or the new one. That needs an internal setup
   variant that does not call gve_rx_queue_release() on the existing
   slot.

5. Failed recovery leaves the port in a state that crashes later.
   recover_fail frees the admin queue and NULLs all queues but leaves
   dev_started set. A subsequent rte_eth_dev_stop() goes to
   gve_stop_tx_queues(), which issues destroy_tx_queues over the freed
   admin queue, then gve_tx_queue_stop() which passes a NULL txq to
   gve_release_txq_mbufs(). Clear dev_started on failure and make
   stop/close tolerate !gve_get_admin_queue_ok() and NULL queues.


[PATCH 9/9] net/gve: move dev status poller to its own thread

6. Lost update on state_flags. rte_bit_relaxed_set32/clear32 are
   plain read-modify-write on a shared word. The poller thread
   updates DEVICE_RESOURCES_OK, ADMIN_QUEUE_OK and DEVICE_RINGS_OK
   during recovery while gve_dev_close() clears STATUS_POLLER_OK from
   the application thread. If they overlap, the poller can write back
   a stale word with STATUS_POLLER_OK still set; the loop never exits
   and rte_thread_join() in close hangs. The fences around the relaxed
   bit ops do not make the RMW atomic.

   Use a separate flag:

	RTE_ATOMIC(bool) dev_status_poller_run;

	rte_atomic_store_explicit(&priv->dev_status_poller_run, false,
				  rte_memory_order_release);
	...
	while (rte_atomic_load_explicit(&priv->dev_status_poller_run,
					rte_memory_order_acquire))


Warnings
--------

[PATCH 2/9] net/gve: snapshot queue configurations

7. Memzone names grow without bound. rte_eth_dma_zone_reserve()
   builds "eth_p%d_q%d_%s" in a 32-byte buffer. With
   "rx_compl_ring_g%u", a 3-digit port and queue already reach the
   limit at reset_generation >= 100, after which setup fails with
   ENAMETOOLONG and recovery fails. Since gve_add_zombie_*_queue()
   frees any older zombie for the same queue id, only two generations
   are ever live; use (reset_generation & 1) and shorter names
   ("rxc_g%u").

8. rxq_configs/txq_configs (and zombie_rx/tx_queues in 3/9) are
   CPU-only bookkeeping used by the primary. Use calloc()/free()
   instead of rte_zmalloc().

[PATCH 3/9] net/gve: add deferred queue memory reclamation

9. zombie_lock and zombie_next are never used. Drop them.

[PATCH 5/9] net/gve: add dedicated mutex for device reset

10. PTHREAD_MUTEX_RECURSIVE is set on the shared mutexattr, so
    flow_rule_lock and nic_ts_lock silently become recursive too.
    Recursion is only needed because recovery calls gve_dev_start()
    with reset_lock held. Split out an unlocked helper and keep the
    mutex non-recursive:

	static int gve_dev_start_locked(struct rte_eth_dev *dev);

	static int
	gve_dev_start(struct rte_eth_dev *dev)
	{
		...
		pthread_mutex_lock(&priv->reset_lock);
		ret = gve_dev_start_locked(dev);
		pthread_mutex_unlock(&priv->reset_lock);
		return ret;
	}

[PATCH 7/9] net/gve: implement proactive device reset

11. The driver no longer raises RTE_ETH_EVENT_INTR_RESET. Existing
    applications that registered for it will stop receiving it. Say
    so in the release note entry, not just in gve.rst.

12. Commit message says recovery runs in a "dedicated control
    recovery thread"; in this patch it runs from the EAL alarm
    callback. The thread only appears in 9/9. Also "supercedes" ->
    "supersedes".

[PATCH 9/9] net/gve: move dev status poller to its own thread

13. gve_dev_reset() calls gve_stop_dev_status_polling() without
    checking gve_get_dev_status_poller_ok(). If thread creation
    failed at probe, rte_thread_join() is called on an uninitialized
    rte_thread_t. Check the flag as gve_dev_close() does.