Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 14 Feb 2025 15:11:00 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 73c3fe4db3eb - main - gve: Fix qpl_buf_head being initialized improperly
Message-ID:  <202502141511.51EFB0tZ063413@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=73c3fe4db3ebc2bd6cb732aae77ea017fd376d22

commit 73c3fe4db3ebc2bd6cb732aae77ea017fd376d22
Author:     Jasper Tran O'Leary <jtranoleary@google.com>
AuthorDate: 2025-02-14 15:05:46 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-02-14 15:08:23 +0000

    gve: Fix qpl_buf_head being initialized improperly
    
    Currently, for DQO QPL our MPASS assertion on qpl_buf_head for available
    pending_pkts (i.e. not holding a packet) fails due to incorrect
    initialization. The MPASS fails on the first run of packets through the
    ring when INVARIANTS is on, and when INVARIANTS is off, things work
    without a bug.
    
    The MPASS guards against improper reaping of "pending_pkt" objects,
    and thus was failing for the first run through the ring. By correctly
    initializing the objects in this patch we make the MPASS not fail on the
    first run too.
    
    Signed-off-by: Vee Agarwal <veethebee@google.com>
    Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
    
    Reviewed by:    delphij, markj
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D48968
---
 sys/dev/gve/gve_tx_dqo.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/sys/dev/gve/gve_tx_dqo.c b/sys/dev/gve/gve_tx_dqo.c
index b4bcb1fd01c5..bf314ef95173 100644
--- a/sys/dev/gve/gve_tx_dqo.c
+++ b/sys/dev/gve/gve_tx_dqo.c
@@ -43,6 +43,13 @@ gve_unmap_packet(struct gve_tx_ring *tx,
 	bus_dmamap_unload(tx->dqo.buf_dmatag, pending_pkt->dmamap);
 }
 
+static void
+gve_clear_qpl_pending_pkt(struct gve_tx_pending_pkt_dqo *pending_pkt)
+{
+	pending_pkt->qpl_buf_head = -1;
+	pending_pkt->num_qpl_bufs = 0;
+}
+
 static void
 gve_free_tx_mbufs_dqo(struct gve_tx_ring *tx)
 {
@@ -54,10 +61,9 @@ gve_free_tx_mbufs_dqo(struct gve_tx_ring *tx)
 		if (!pending_pkt->mbuf)
 			continue;
 
-		if (gve_is_qpl(tx->com.priv)) {
-			pending_pkt->qpl_buf_head = -1;
-			pending_pkt->num_qpl_bufs = 0;
-		} else
+		if (gve_is_qpl(tx->com.priv))
+			gve_clear_qpl_pending_pkt(pending_pkt);
+		else
 			gve_unmap_packet(tx, pending_pkt);
 
 		m_freem(pending_pkt->mbuf);
@@ -880,8 +886,7 @@ gve_reap_qpl_bufs_dqo(struct gve_tx_ring *tx,
 	 */
 	atomic_add_rel_32(&tx->dqo.qpl_bufs_produced, pkt->num_qpl_bufs);
 
-	pkt->qpl_buf_head = -1;
-	pkt->num_qpl_bufs = 0;
+	gve_clear_qpl_pending_pkt(pkt);
 }
 
 static uint64_t
@@ -981,11 +986,13 @@ gve_clear_tx_ring_dqo(struct gve_priv *priv, int i)
 
 	gve_free_tx_mbufs_dqo(tx);
 
-	for (j = 0; j < tx->dqo.num_pending_pkts - 1; j++) {
-		tx->dqo.pending_pkts[j].next = j + 1;
+	for (j = 0; j < tx->dqo.num_pending_pkts; j++) {
+		if (gve_is_qpl(tx->com.priv))
+			gve_clear_qpl_pending_pkt(&tx->dqo.pending_pkts[j]);
+		tx->dqo.pending_pkts[j].next =
+		    (j == tx->dqo.num_pending_pkts - 1) ? -1 : j + 1;
 		tx->dqo.pending_pkts[j].state = GVE_PACKET_STATE_FREE;
 	}
-	tx->dqo.pending_pkts[tx->dqo.num_pending_pkts - 1].next = -1;
 	tx->dqo.free_pending_pkts_csm = 0;
 	atomic_store_rel_32(&tx->dqo.free_pending_pkts_prd, -1);
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502141511.51EFB0tZ063413>