Re: [PATCH v4 9/9] test/memif: add memif PMD unit test

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 16 Oct 2026 21:58:57 +0200
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> wrote:

> Add a memif_pmd_autotest unit test, modelled on the TAP PMD test. memif
> differs from TAP in that a port is only usable once a peer connects, so
> the test creates a server and a client memif port in the same process,
> connected over an abstract-namespace socket (the loopback setup from the
> memif documentation), and waits for the asynchronous control-channel
> handshake to bring both ports up.
> 
> The suite checks dev_info reports the memif driver, that the pair reaches
> link up, statistics get and reset, a client-to-server loopback burst with
> payload verification on both the slow and the single-pool fast path (a
> second default-bsize pair), multi-segment loopback across a ring wrap
> with payload verification (chained-descriptor Rx and Tx), that the pair
> reconnects after a port restart, that a zero-length Tx burst is handled,
> that a configured-but-not-started port can be closed without crashing,
> and that a server stopped after start refuses a peer. It is a fast test,
> runs without hugepages and under ASan.
> 
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> ---

This test is getting lots of CI failures.

AI says it found the answer. The analysis is likely correct, but
the fix looks awkward.

From 9a0d594135e21e949e814df036d30d70ff93e45b Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 25 Oct 2026 09:45:50 -0700
Subject: [PATCH] net/memif: fix race between interrupt thread and stop

memif_disconnect() can run in the interrupt thread, from the control
channel handler on peer EOF, at the same time as dev_stop/dev_close
in the application thread. When the handler is active the application
side only marks the callback pending and carries on freeing queues,
regions and finally the port. The handler then calls memif_disconnect()
on a released ethdev and crashes on dev->data.

Seen as a SIGSEGV in memif_pmd_autotest teardown, where stopping the
server makes the client channel see EOF while the test is stopping and
closing the client.

When called from an application thread, detach the channel under
cc_lock and wait for the handler with
rte_intr_callback_unregister_sync() before tearing down. The handler
drops its own callback if it finds the channel detached, otherwise
the EOF keeps the fd readable and the wait never completes. Hold
cc_lock over the queue and region teardown so the two paths cannot
run it concurrently.

Close the channel fd through its own handle when freeing it; the
handle given to the pending unregister callback is the EAL copy.

Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FA4k3HfYcHiM1tu71SfXb1
---
Applies on top of "net/memif: reset FD in interrupt handle"
(v2 07/12 of David's interrupt FD series).

 drivers/net/memif/memif_socket.c | 110 +++++++++++++++++++++----------
 1 file changed, 74 insertions(+), 36 deletions(-)

diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index fb639fde91..e580f97bc4 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -572,83 +572,103 @@ memif_msg_enq_connected(struct rte_eth_dev *dev)
 }
 
 static void
-memif_intr_unregister_handler(struct rte_intr_handle *intr_handle, void *arg)
+memif_cc_free(struct memif_control_channel *cc)
 {
 	struct memif_msg_queue_elt *elt;
-	struct memif_control_channel *cc = arg;
 
-	/* close control channel fd */
-	rte_intr_fd_close(intr_handle);
-	/* clear message queue */
+	rte_intr_fd_close(cc->intr_handle);
 	while ((elt = TAILQ_FIRST(&cc->msg_queue)) != NULL) {
 		TAILQ_REMOVE(&cc->msg_queue, elt, next);
 		rte_free(elt);
 	}
 	rte_intr_instance_free(cc->intr_handle);
-	/* free control channel */
 	rte_free(cc);
 }
 
+/*
+ * Pending unregister callback, run by the interrupt thread once
+ * memif_intr_handler() has returned. The handle passed in is the
+ * EAL copy, close the fd through the control channel's own handle.
+ */
+static void
+memif_intr_unregister_handler(struct rte_intr_handle *intr_handle __rte_unused, void *arg)
+{
+	memif_cc_free(arg);
+}
+
 void
 memif_disconnect(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
 	struct memif_msg_queue_elt *elt, *next;
+	struct memif_control_channel *cc;
 	struct memif_queue *mq;
-	struct rte_intr_handle *ih;
 	int i;
 	int ret;
 
+	rte_spinlock_lock(&pmd->cc_lock);
 	dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN;
 	pmd->flags &= ~ETH_MEMIF_FLAG_CONNECTING;
 	pmd->flags &= ~ETH_MEMIF_FLAG_CONNECTED;
 
-	rte_spinlock_lock(&pmd->cc_lock);
-	if (pmd->cc != NULL) {
+	cc = pmd->cc;
+	pmd->cc = NULL;
+	if (cc != NULL) {
 		/* Clear control message queue (except disconnect message if any). */
-		for (elt = TAILQ_FIRST(&pmd->cc->msg_queue); elt != NULL; elt = next) {
+		for (elt = TAILQ_FIRST(&cc->msg_queue); elt != NULL; elt = next) {
 			next = TAILQ_NEXT(elt, next);
 			if (elt->msg.type != MEMIF_MSG_TYPE_DISCONNECT) {
-				TAILQ_REMOVE(&pmd->cc->msg_queue, elt, next);
+				TAILQ_REMOVE(&cc->msg_queue, elt, next);
 				rte_free(elt);
 			}
 		}
 		/* send disconnect message (if there is any in queue) */
-		memif_msg_send_from_queue(pmd->cc);
+		memif_msg_send_from_queue(cc);
 
 		/* at this point, there should be no more messages in queue */
-		if (TAILQ_FIRST(&pmd->cc->msg_queue) != NULL) {
-			MIF_LOG(WARNING,
-				"Unexpected message(s) in message queue.");
-		}
+		if (TAILQ_FIRST(&cc->msg_queue) != NULL)
+			MIF_LOG(WARNING, "Unexpected message(s) in message queue.");
 
-		ih = pmd->cc->intr_handle;
-		if (rte_intr_fd_get(ih) >= 0) {
-			ret = rte_intr_callback_unregister(ih,
-							memif_intr_handler,
-							pmd->cc);
+		if (rte_thread_is_intr()) {
 			/*
-			 * If callback is active (disconnecting based on
-			 * received control message).
+			 * Called from the interrupt thread, possibly from
+			 * memif_intr_handler() itself: cannot wait for it.
 			 */
-			if (ret == -EAGAIN) {
-				ret = rte_intr_callback_unregister_pending(ih,
-							memif_intr_handler,
-							pmd->cc,
-							memif_intr_unregister_handler);
-			} else if (ret > 0) {
-				rte_intr_fd_close(ih);
-				rte_intr_instance_free(ih);
-				rte_free(pmd->cc);
-			}
-			pmd->cc = NULL;
+			ret = rte_intr_callback_unregister(cc->intr_handle,
+							   memif_intr_handler, cc);
+			if (ret == -EAGAIN)
+				ret = rte_intr_callback_unregister_pending(cc->intr_handle,
+						memif_intr_handler, cc,
+						memif_intr_unregister_handler);
+			else if (ret > 0)
+				memif_cc_free(cc);
 			if (ret <= 0)
-				MIF_LOG(WARNING,
-					"Failed to unregister control channel callback.");
+				MIF_LOG(WARNING, "Failed to unregister control channel callback.");
+			cc = NULL;
 		}
 	}
 	rte_spinlock_unlock(&pmd->cc_lock);
 
+	if (cc != NULL) {
+		/*
+		 * Called from an application thread (dev_stop/dev_close).
+		 * The interrupt thread may be running memif_intr_handler()
+		 * for this channel and about to call memif_disconnect() on
+		 * this device. Wait for it to finish before tearing down,
+		 * otherwise it races with the caller freeing the queues and
+		 * releasing the port. The lock must not be held here since
+		 * the handler takes it.
+		 */
+		ret = rte_intr_callback_unregister_sync(cc->intr_handle,
+							memif_intr_handler, cc);
+		/* -ENOENT: the handler saw the channel detached and dropped itself. */
+		if (ret > 0 || ret == -ENOENT)
+			memif_cc_free(cc);
+		else
+			MIF_LOG(WARNING, "Failed to unregister control channel callback.");
+	}
+
+	rte_spinlock_lock(&pmd->cc_lock);
 	/* unconfig interrupts */
 	for (i = 0; i < pmd->cfg.num_c2s_rings; i++) {
 		if (pmd->role == MEMIF_ROLE_CLIENT) {
@@ -694,6 +714,7 @@ memif_disconnect(struct rte_eth_dev *dev)
 
 	MIF_LOG(DEBUG, "Disconnected, id: %d, role: %s.", pmd->id,
 		(pmd->role == MEMIF_ROLE_SERVER) ? "server" : "client");
+	rte_spinlock_unlock(&pmd->cc_lock);
 }
 
 static int
@@ -867,6 +888,8 @@ static void
 memif_intr_handler(void *arg)
 {
 	struct memif_control_channel *cc = arg;
+	struct pmd_internals *pmd;
+	bool owned;
 	int ret;
 
 	ret = memif_msg_receive(cc);
@@ -897,6 +920,21 @@ memif_intr_handler(void *arg)
 		MIF_LOG(WARNING, "eth dev not allocated");
 		return;
 	}
+
+	pmd = cc->dev->data->dev_private;
+	rte_spinlock_lock(&pmd->cc_lock);
+	owned = (pmd->cc == cc);
+	rte_spinlock_unlock(&pmd->cc_lock);
+	if (!owned) {
+		/*
+		 * Detached by memif_disconnect() in an application thread
+		 * which is waiting for this handler to finish and will free
+		 * the channel. Stop polling the fd, it stays readable on EOF.
+		 */
+		rte_intr_callback_unregister_pending(cc->intr_handle,
+						     memif_intr_handler, cc, NULL);
+		return;
+	}
 	memif_disconnect(cc->dev);
 }
 
-- 
2.43.0