On Fri, 16 Oct 2026 21:58:57 +0200 Sriram Yagnaraman <[email protected]> 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 <[email protected]> > --- 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 <[email protected]> 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: [email protected] Signed-off-by: Stephen Hemminger <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> 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
