Thread (16 messages) flat view 16 messages, 2 authors, 3d ago
WARM3d REVIEWED: 1 (1M)

1 review trailer.

[PATCH net v2 8/9] gve: ensure XDP mem model is registered when disabling XSK pools

From: Joshua Washington <joshwash@google.com>
Date: 2026-09-22 19:45:53
Also in: bpf, lkml, stable
Subsystem: networking drivers, the rest · Maintainers: Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

When disabling XSK pools, QPL and RDA modes have different behaviors,
but they are both incorrect in that the function returns just after
unregistering the memory model.

RDA mode performs an internally consistent re-configuration of rings,
making the extra logic, including the XSK pool unregistration,
unnecessary. However, it is possible for the reconfiguration to fail due
to memory allocation. Instead of freeing and re-allocating ring memory,
stop the rings and reinitialize the ring state. That way, failure to
stop the rings would result in a safer device reset, making it safe to
unregister the pool regardless of the error condition when stopping
queues. This change involves a bit of refactoring in the TX
initialization path, introducing new methods to reset ring state when
stopping the rings.

QPL mode, which does not need to reconfigure rings due to not posting
XSK umem to the hardware ring, simply misses registering the RXQ XDP
info with the MEM_TYPE_PAGE_SHARED memory model.

Make a best-effort attempt to register memory model in both cases.
Because memory model registration can fail and xp_release_deferred
cannot, the XSK pool must be unregistered and DMA-unmapped regardless of
whether memory model registration succeeds. In both cases, there should
be a guarantee against the device DMA'ing into freed memory, however.

Fixes: 077f7153fd25 ("gve: merge xdp and xsk registration")
Cc: stable@vger.kernel.org
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
v2:
- Newly introduced
---
 drivers/net/ethernet/google/gve/gve_main.c   |  77 +++++++++-----
 drivers/net/ethernet/google/gve/gve_tx_dqo.c | 101 ++++++++++++++-----
 2 files changed, 126 insertions(+), 52 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index f2bd4011de23..787d311ff99b 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -1715,15 +1715,56 @@ static int gve_xsk_pool_enable(struct net_device *dev,
 	return err;
 }
 
+static int gve_unreg_xsk_pool_live(struct gve_priv *priv,
+				   struct net_device *dev, u16 qid)
+{
+	struct gve_rx_ring *rx;
+	int err;
+
+	rx = &priv->rx[qid];
+	gve_disable_xsk_napis(priv, qid);
+
+	gve_unreg_xsk_pool(priv, qid);
+	if (gve_is_qpl(priv))
+		err = xdp_rxq_info_reg_mem_model(&rx->xdp_rxq,
+						 MEM_TYPE_PAGE_SHARED,
+						 NULL);
+	else
+		err = xdp_rxq_info_reg_mem_model(&rx->xdp_rxq,
+						 MEM_TYPE_PAGE_POOL,
+						 rx->dqo.page_pool);
+	if (err)
+		netdev_warn(dev,
+			    "Failed to register memory model after unregistering XSK pool");
+
+	smp_mb(); /* Make sure it is visible to the workers on datapath */
+
+	gve_enable_xsk_napis(priv, qid);
+
+	return err;
+}
+
+static int gve_restart_rings(struct gve_priv *priv)
+{
+	struct gve_tx_alloc_rings_cfg tx_alloc_cfg = {0};
+	struct gve_rx_alloc_rings_cfg rx_alloc_cfg = {0};
+	int err;
+
+	gve_get_curr_alloc_cfgs(priv, &tx_alloc_cfg, &rx_alloc_cfg);
+	err = gve_queues_stop(priv);
+	if (err)
+		return err;
+
+	err = gve_queues_start(priv, &tx_alloc_cfg, &rx_alloc_cfg);
+	return err;
+}
+
 static int gve_xsk_pool_disable(struct net_device *dev,
 				u16 qid)
 {
 	struct gve_priv *priv = netdev_priv(dev);
-	struct napi_struct *napi_rx;
-	struct napi_struct *napi_tx;
 	struct xsk_buff_pool *pool;
 	int err = 0;
-	int tx_qid;
 
 	if (qid >= priv->rx_cfg.num_queues) {
 		err = -EINVAL;
@@ -1735,31 +1776,11 @@ static int gve_xsk_pool_disable(struct net_device *dev,
 	if (!netif_running(dev) || !priv->tx_cfg.num_xdp_queues)
 		goto unmap_and_return;
 
-	/* Stop and start RDA queues to repost buffers. */
-	if (!gve_is_qpl(priv) && priv->xdp_prog) {
-		err = gve_configure_rings_xdp(priv, priv->rx_cfg.num_queues);
-		if (err)
-			return err;
-	}
-
-	napi_rx = &priv->ntfy_blocks[priv->rx[qid].ntfy_id].napi;
-	napi_disable_locked(napi_rx); /* make sure current rx poll is done */
-
-	tx_qid = gve_xdp_tx_queue_id(priv, qid);
-	napi_tx = &priv->ntfy_blocks[priv->tx[tx_qid].ntfy_id].napi;
-	napi_disable_locked(napi_tx); /* make sure current tx poll is done */
-
-	gve_unreg_xsk_pool(priv, qid);
-	smp_mb(); /* Make sure it is visible to the workers on datapath */
-
-	napi_enable_locked(napi_rx);
-	napi_enable_locked(napi_tx);
-	if (gve_is_gqi(priv)) {
-		if (gve_rx_work_pending(&priv->rx[qid]))
-			napi_schedule(napi_rx);
-
-		if (gve_tx_clean_pending(priv, &priv->tx[tx_qid]))
-			napi_schedule(napi_tx);
+	if (gve_is_qpl(priv)) {
+		err = gve_unreg_xsk_pool_live(priv, dev, qid);
+	} else {
+		/* Stop and start RDA queues to repost buffers. */
+		err = gve_restart_rings(priv);
 	}
 
 unmap_and_return:
diff --git a/drivers/net/ethernet/google/gve/gve_tx_dqo.c b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
index 80ab0a449ff5..78f946ae7264 100644
--- a/drivers/net/ethernet/google/gve/gve_tx_dqo.c
+++ b/drivers/net/ethernet/google/gve/gve_tx_dqo.c
@@ -208,6 +208,78 @@ static void gve_tx_clean_pending_packets(struct gve_tx_ring *tx)
 	}
 }
 
+static void gve_tx_init_ring_state_dqo(struct gve_tx_ring *tx)
+{
+	int i;
+
+	atomic_set_release(&tx->dqo_compl.hw_tx_head, 0);
+
+	/* Set up linked list of pending packets */
+	for (i = 0; i < tx->dqo.num_pending_packets - 1; i++)
+		tx->dqo.pending_packets[i].next = i + 1;
+
+	tx->dqo.pending_packets[tx->dqo.num_pending_packets - 1].next = -1;
+	atomic_set_release(&tx->dqo_compl.free_pending_packets, -1);
+
+	tx->dqo_compl.miss_completions.head = -1;
+	tx->dqo_compl.miss_completions.tail = -1;
+	tx->dqo_compl.timed_out_completions.head = -1;
+	tx->dqo_compl.timed_out_completions.tail = -1;
+
+	/* Generate free TX buf list */
+	if (tx->dqo.tx_qpl_buf_next) {
+		for (i = 0; i < tx->dqo.num_tx_qpl_bufs - 1; i++)
+			tx->dqo.tx_qpl_buf_next[i] = i + 1;
+		tx->dqo.tx_qpl_buf_next[tx->dqo.num_tx_qpl_bufs - 1] = -1;
+
+		atomic_set_release(&tx->dqo_compl.free_tx_qpl_buf_head, -1);
+		atomic_set_release(&tx->dqo_compl.free_tx_qpl_buf_cnt, 0);
+	}
+}
+
+static void gve_tx_reset_ring_dqo(struct gve_tx_ring *tx)
+{
+	size_t size;
+
+	/* Reset dqo_tx fields. */
+	tx->dqo_tx.head = 0;
+	tx->dqo_tx.tail = 0;
+	tx->dqo_tx.last_re_idx = 0;
+	tx->dqo_tx.posted_packet_desc_cnt = 0;
+	tx->dqo_tx.completed_packet_desc_cnt = 0;
+	tx->dqo_tx.free_pending_packets = 0;
+
+	/* Reset dqo_compl fields. */
+	tx->dqo_compl.head = 0;
+	tx->dqo_compl.cur_gen_bit = 0;
+	tx->dqo_compl.xsk_reorder_queue_head = 0;
+	tx->dqo_compl.xsk_reorder_queue_tail = 0;
+
+	if (tx->dqo.xsk_reorder_queue) {
+		size = (tx->dqo.complq_mask + 1) *
+			sizeof(*tx->dqo.xsk_reorder_queue);
+		memset(tx->dqo.xsk_reorder_queue, 0, size);
+		atomic_set(&tx->dqo_tx.xsk_reorder_queue_tail, 0);
+	}
+
+	size = sizeof(tx->dqo.tx_ring[0]) * (tx->mask + 1);
+	memset(tx->dqo.tx_ring, 0, size);
+
+	size = sizeof(tx->dqo.compl_ring[0]) * (tx->dqo.complq_mask + 1);
+	memset(tx->dqo.compl_ring, 0, size);
+
+	memset(tx->q_resources, 0, sizeof(*tx->q_resources));
+
+	if (tx->dqo.tx_qpl_buf_next) {
+		tx->dqo_tx.free_tx_qpl_buf_head = 0;
+		size = sizeof(tx->dqo.tx_qpl_buf_next[0]) *
+			tx->dqo.num_tx_qpl_bufs;
+		memset(tx->dqo.tx_qpl_buf_next, 0, size);
+	}
+
+	gve_tx_init_ring_state_dqo(tx);
+}
+
 void gve_tx_stop_ring_dqo(struct gve_priv *priv, int idx)
 {
 	int ntfy_idx = gve_tx_idx_to_ntfy(priv, idx);
@@ -222,6 +294,7 @@ void gve_tx_stop_ring_dqo(struct gve_priv *priv, int idx)
 		netdev_tx_reset_queue(tx->netdev_txq);
 	gve_tx_clean_pending_packets(tx);
 	gve_tx_remove_from_block(priv, idx);
+	gve_tx_reset_ring_dqo(tx);
 }
 
 static void gve_tx_free_ring_dqo(struct gve_priv *priv, struct gve_tx_ring *tx,
@@ -270,11 +343,10 @@ static void gve_tx_free_ring_dqo(struct gve_priv *priv, struct gve_tx_ring *tx,
 	netif_dbg(priv, drv, priv->dev, "freed tx queue %d\n", idx);
 }
 
-static int gve_tx_qpl_buf_init(struct gve_tx_ring *tx)
+static int gve_tx_qpl_buf_list_alloc(struct gve_tx_ring *tx)
 {
 	int num_tx_qpl_bufs = GVE_TX_BUFS_PER_PAGE_DQO *
 		tx->dqo.qpl->num_entries;
-	int i;
 
 	tx->dqo.tx_qpl_buf_next = kvzalloc_objs(tx->dqo.tx_qpl_buf_next[0],
 						num_tx_qpl_bufs);
@@ -282,13 +354,6 @@ static int gve_tx_qpl_buf_init(struct gve_tx_ring *tx)
 		return -ENOMEM;
 
 	tx->dqo.num_tx_qpl_bufs = num_tx_qpl_bufs;
-
-	/* Generate free TX buf list */
-	for (i = 0; i < num_tx_qpl_bufs - 1; i++)
-		tx->dqo.tx_qpl_buf_next[i] = i + 1;
-	tx->dqo.tx_qpl_buf_next[num_tx_qpl_bufs - 1] = -1;
-
-	atomic_set_release(&tx->dqo_compl.free_tx_qpl_buf_head, -1);
 	return 0;
 }
 
@@ -304,6 +369,7 @@ void gve_tx_start_ring_dqo(struct gve_priv *priv, int idx)
 	gve_add_napi(priv, ntfy_idx, gve_napi_poll_dqo);
 }
 
+
 static int gve_tx_alloc_ring_dqo(struct gve_priv *priv,
 				 struct gve_tx_alloc_rings_cfg *cfg,
 				 struct gve_tx_ring *tx,
@@ -313,13 +379,11 @@ static int gve_tx_alloc_ring_dqo(struct gve_priv *priv,
 	int num_pending_packets;
 	size_t bytes;
 	u32 qpl_id;
-	int i;
 
 	memset(tx, 0, sizeof(*tx));
 	tx->q_num = idx;
 	tx->dev = hdev;
 	spin_lock_init(&tx->dqo_tx.xdp_lock);
-	atomic_set_release(&tx->dqo_compl.hw_tx_head, 0);
 
 	/* Queue sizes must be a power of 2 */
 	tx->mask = cfg->ring_size - 1;
@@ -350,13 +414,6 @@ static int gve_tx_alloc_ring_dqo(struct gve_priv *priv,
 	if (!tx->dqo.pending_packets)
 		goto err;
 
-	/* Set up linked list of pending packets */
-	for (i = 0; i < tx->dqo.num_pending_packets - 1; i++)
-		tx->dqo.pending_packets[i].next = i + 1;
-
-	tx->dqo.pending_packets[tx->dqo.num_pending_packets - 1].next = -1;
-	atomic_set_release(&tx->dqo_compl.free_pending_packets, -1);
-
 	/* Only alloc xsk pool for XDP queues */
 	if (idx >= cfg->qcfg->num_queues && cfg->num_xdp_rings) {
 		tx->dqo.xsk_reorder_queue =
@@ -367,11 +424,6 @@ static int gve_tx_alloc_ring_dqo(struct gve_priv *priv,
 			goto err;
 	}
 
-	tx->dqo_compl.miss_completions.head = -1;
-	tx->dqo_compl.miss_completions.tail = -1;
-	tx->dqo_compl.timed_out_completions.head = -1;
-	tx->dqo_compl.timed_out_completions.tail = -1;
-
 	bytes = sizeof(tx->dqo.tx_ring[0]) * (tx->mask + 1);
 	tx->dqo.tx_ring = dma_alloc_coherent(hdev, bytes, &tx->bus, GFP_KERNEL);
 	if (!tx->dqo.tx_ring)
@@ -397,10 +449,11 @@ static int gve_tx_alloc_ring_dqo(struct gve_priv *priv,
 		if (!tx->dqo.qpl)
 			goto err;
 
-		if (gve_tx_qpl_buf_init(tx))
+		if (gve_tx_qpl_buf_list_alloc(tx))
 			goto err;
 	}
 
+	gve_tx_init_ring_state_dqo(tx);
 	return 0;
 
 err:
-- 
2.55.0.1082.g2b9226bbc0-goog
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help