* [PATCH v4 0/9] net/memif: lifecycle fixes and unit test
@ 2026-10-16 19:58 Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 1/9] net/memif: guard against empty Tx burst Sriram Yagnaraman
` (9 more replies)
0 siblings, 10 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
This series fixes a set of error-path and lifecycle bugs in the memif
PMD and adds a unit test for the driver.
The first eight patches are fixes:
- guard eth_memif_tx() against an empty burst (nb_pkts == 0);
- close file descriptor 0 instead of leaking it;
- do not close the EAL-owned memseg fd in zero-copy mode;
- free the MAC address allocated during probe on failure;
- unwind a partially-created device on create failure;
- refuse connections on a stopped server so it does not come up on a
stopped port;
- disconnect on close so a stopped server's control channel is not
leaked, and make memif_disconnect() safe on a never-started device;
- fix eth_memif_tx_zc() returning 65535 on an invalid interrupt fd.
The last patch adds a memif PMD unit test (app/test, memif_pmd_autotest):
a server/client vdev pair over a loopback socket covering dev_info, link
state, stats, single-segment loopback (slow and fast path), multi-segment
loopback across a ring wrap, stop/start reconnect, empty Tx burst, close
without stop, and a stopped server refusing a peer.
Verified: memif_pmd_autotest passes 11/11 (~1.4s); devtools/test-memif.sh
two-process forwarding OK; each commit builds with -Dwerror=true;
check-git-log and checkpatch are clean.
Thanks to Stephen Hemminger and the ai-code-review bot for the reviews
that shaped this series.
v4:
- Patch 7 (disconnect on close): reword the commit message so it no
longer describes the tree relative to patch 6 ordering; state the
started-then-stopped condition directly and frame the close-time call
as a backstop.
- Test: bound the stopped-server negative check to a short window
(REJECT_TRIES, ~1s) instead of the full 5s connect timeout, so the
fast test stays well under its budget; NULL-check rte_pktmbuf_append()
in create_big_packet(); list the fast-path and stopped-server cases
in the commit message.
v3:
- Rebase on main, picking up rte_intr_fd_close() from
d9191f684c ("interrupts: add helpers to close FD"): patch 2 keeps
only the owned-fd ">= 0" fixes not superseded by that helper, and
patch 7 keeps the NULL-queue guards above the rte_intr_fd_close()
calls.
- New patch 6: refuse connections on a stopped server. The started-
then-stopped case is the actual trigger for the control-channel
leak; dev_stop now sets ETH_MEMIF_FLAG_DISABLED.
- Patch 3 (memseg fd): also stop closing the EAL-owned fd in
memif_connect(), not only memif_free_regions().
- Test: new test_memif_stopped_server_rejects_peer covering patch 6
(fails without it); reword close-without-stop to what it actually
covers; verify the full fast-path payload.
v2:
- P1: correct Fixes tag (Tx fast path); move the empty-burst check
after the ring == NULL block.
- P2: split the memseg-fd change into its own patch.
- P4 (MAC leak): document the secondary -1 -> -ENOMEM change.
- P5 (unwind): add Fixes: multi-process support.
- New patch: fix eth_memif_tx_zc() bogus Tx count on invalid fd.
- Test: add to MAINTAINERS; TEST_FAILED (not SKIPPED) on probe failure;
add a default-bsize pair for the Tx fast path; set ring size via
rsize.
Sriram Yagnaraman (9):
net/memif: guard against empty Tx burst
net/memif: close file descriptor zero
net/memif: do not close EAL-owned memseg file descriptor
net/memif: fix MAC address leak on probe failure
net/memif: unwind partial device on create failure
net/memif: refuse connection on stopped device
net/memif: disconnect on device close
net/memif: fix bogus Tx count on invalid interrupt FD
test/memif: add memif PMD unit test
MAINTAINERS | 1 +
app/test/meson.build | 1 +
app/test/test_pmd_memif.c | 764 ++++++++++++++++++++++++++++++
drivers/net/memif/memif_socket.c | 10 +-
drivers/net/memif/rte_eth_memif.c | 76 ++-
5 files changed, 825 insertions(+), 27 deletions(-)
create mode 100644 app/test/test_pmd_memif.c
--
2.43.7
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/9] net/memif: guard against empty Tx burst
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 2/9] net/memif: close file descriptor zero Sriram Yagnaraman
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
eth_memif_tx() reads bufs[0]->pool to pick the mempool for its
single-pool fast-path test, before any check on nb_pkts. A caller that
passes nb_pkts == 0 therefore dereferences bufs[0], out of bounds for an
empty burst. Return early when there is nothing to transmit, after the
ring == NULL check so a secondary process still requests regions.
Fixes: b5613c8f9d0a ("net/memif: add a Tx fast path")
Cc: stable@dpdk.org
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/rte_eth_memif.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 3ed177ffb2..36db03a268 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -904,6 +904,8 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
mq->in_port, rte_strerror(-ret));
return 0;
}
+ if (unlikely(nb_pkts == 0))
+ return 0;
ring_size = 1 << mq->log2_ring_size;
mask = ring_size - 1;
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 2/9] net/memif: close file descriptor zero
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 1/9] net/memif: guard against empty Tx burst Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 3/9] net/memif: do not close EAL-owned memseg file descriptor Sriram Yagnaraman
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
Several cleanup paths guard close() with "fd > 0", which skips a valid
file descriptor of 0 and leaks it. A descriptor of 0 is legitimate (for
example when stdin has been closed before the region or interrupt fd is
created). Use ">= 0" consistently with the -1 sentinel the code already
uses elsewhere, in the memif_free_regions() shm munmap path, the
memif_region_init_shm() error path, the control-channel teardown in
memif_disconnect(), and the SCM_RIGHTS fd check in memif_msg_send().
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/memif_socket.c | 4 ++--
drivers/net/memif/rte_eth_memif.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 87b23aa8b4..550e2e468a 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -41,7 +41,7 @@ memif_msg_send(int fd, memif_msg_t *msg, int afd)
mh.msg_iov = iov;
mh.msg_iovlen = 1;
- if (afd > 0) {
+ if (afd >= 0) {
memset(&ctl, 0, sizeof(ctl));
mh.msg_control = ctl;
mh.msg_controllen = sizeof(ctl);
@@ -623,7 +623,7 @@ memif_disconnect(struct rte_eth_dev *dev)
}
ih = pmd->cc->intr_handle;
- if (rte_intr_fd_get(ih) > 0) {
+ if (rte_intr_fd_get(ih) >= 0) {
ret = rte_intr_callback_unregister(ih,
memif_intr_handler,
pmd->cc);
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 36db03a268..1d1d12e242 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1295,7 +1295,7 @@ memif_free_regions(struct rte_eth_dev *dev)
}
if (r->addr != NULL) {
munmap(r->addr, r->region_size);
- if (r->fd > 0) {
+ if (r->fd >= 0) {
close(r->fd);
r->fd = -1;
}
@@ -1416,7 +1416,7 @@ memif_region_init_shm(struct rte_eth_dev *dev, uint8_t has_buffers)
return ret;
error:
- if (r->fd > 0)
+ if (r->fd >= 0)
close(r->fd);
r->fd = -1;
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 3/9] net/memif: do not close EAL-owned memseg file descriptor
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 1/9] net/memif: guard against empty Tx burst Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 2/9] net/memif: close file descriptor zero Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 4/9] net/memif: fix MAC address leak on probe failure Sriram Yagnaraman
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
In zero-copy mode the per-memseg regions (i > 0) take their fd from
rte_memseg_get_fd(), which rte_memory.h documents as EAL-internal and
read-only. Both memif_connect() and memif_free_regions() closed that fd,
so the next connect handed the peer a stale (possibly reused)
descriptor and the PMD closed a descriptor it does not own. Stop closing
it in both paths and just clear the region fd; the fd is not ours.
Fixes: 43b815d88188 ("net/memif: support zero-copy slave")
Cc: stable@dpdk.org
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/rte_eth_memif.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 1d1d12e242..e753243440 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1289,9 +1289,9 @@ memif_free_regions(struct rte_eth_dev *dev)
if (r != NULL) {
/* This is memzone */
if (i > 0 && (pmd->flags & ETH_MEMIF_FLAG_ZERO_COPY)) {
+ /* memseg fd is owned by EAL, do not close */
r->addr = NULL;
- if (r->fd > 0)
- close(r->fd);
+ r->fd = -1;
}
if (r->addr != NULL) {
munmap(r->addr, r->region_size);
@@ -1611,8 +1611,7 @@ memif_connect(struct rte_eth_dev *dev)
}
}
if (i > 0 && (pmd->flags & ETH_MEMIF_FLAG_ZERO_COPY)) {
- /* close memseg file */
- close(mr->fd);
+ /* memseg fd is owned by EAL, do not close */
mr->fd = -1;
}
}
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 4/9] net/memif: fix MAC address leak on probe failure
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (2 preceding siblings ...)
2026-10-16 19:58 ` [PATCH v4 3/9] net/memif: do not close EAL-owned memseg file descriptor Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 5/9] net/memif: unwind partial device on create failure Sriram Yagnaraman
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
rte_pmd_memif_probe() allocates the default MAC address up front and
only hands ownership to the device in memif_create() on success. Every
argument-parsing error jumps to the exit label, which freed the kvargs
but not the MAC address, leaking it. The allocation was also dereferenced
by rte_eth_random_addr() with no check that rte_zmalloc() succeeded, and
was done even in the secondary process which never uses it.
Allocate the MAC address only in the primary process after the secondary
path has returned, check it for NULL, and free it at the exit label on
any failure (on success memif_create() owns it via data->mac_addrs).
While here, return -ENOMEM instead of -1 on the secondary-process
process-private allocation failure, matching the primary path.
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/rte_eth_memif.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index e753243440..70359554e6 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -2232,12 +2232,9 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
gid_t owner_gid = -1;
uint32_t flags = 0;
const char *secret = NULL;
- struct rte_ether_addr *ether_addr = rte_zmalloc("",
- sizeof(struct rte_ether_addr), 0);
+ struct rte_ether_addr *ether_addr = NULL;
struct rte_eth_dev *eth_dev;
- rte_eth_random_addr(ether_addr->addr_bytes);
-
MIF_LOG(INFO, "Initialize MEMIF: %s.", name);
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
@@ -2264,7 +2261,7 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
if (eth_dev->process_private == NULL) {
MIF_LOG(ERR,
"Failed to alloc memory for process private");
- return -1;
+ return -ENOMEM;
}
rte_eth_dev_probing_finish(eth_dev);
@@ -2272,6 +2269,13 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
return 0;
}
+ ether_addr = rte_zmalloc("", sizeof(struct rte_ether_addr), 0);
+ if (ether_addr == NULL) {
+ MIF_LOG(ERR, "Failed to alloc MAC address");
+ return -ENOMEM;
+ }
+ rte_eth_random_addr(ether_addr->addr_bytes);
+
ret = rte_mp_action_register(MEMIF_MP_SEND_REGION, memif_mp_send_region);
/*
* Primary process can continue probing, but secondary process won't
@@ -2346,6 +2350,12 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
log2_ring_size, pkt_buffer_size, secret, ether_addr);
exit:
+ /*
+ * On success memif_create() takes ownership of ether_addr
+ * (data->mac_addrs); free it on any failure or early exit.
+ */
+ if (ret != 0)
+ rte_free(ether_addr);
rte_kvargs_free(kvlist);
return ret;
}
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 5/9] net/memif: unwind partial device on create failure
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (3 preceding siblings ...)
2026-10-16 19:58 ` [PATCH v4 4/9] net/memif: fix MAC address leak on probe failure Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 6/9] net/memif: refuse connection on stopped device Sriram Yagnaraman
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
memif_create() returned -1 directly when the process-private allocation
or memif_socket_init() failed, leaving the port allocated by
rte_eth_vdev_allocate() registered and the process-private memory
leaked. Jump to an error label that frees the process-private data and
releases the port instead.
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Fixes: c41a04958b09 ("net/memif: support multi-process")
Cc: stable@dpdk.org
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/rte_eth_memif.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 70359554e6..e70443976d 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1957,18 +1957,18 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
const unsigned int numa_node = vdev->device.numa_node;
const char *name = rte_vdev_device_name(vdev);
+ /* Allocate process-private data before reserving the port. */
+ process_private = rte_zmalloc(name, sizeof(struct pmd_process_private),
+ RTE_CACHE_LINE_SIZE);
+ if (process_private == NULL) {
+ MIF_LOG(ERR, "Failed to alloc memory for process private");
+ return -ENOMEM;
+ }
+
eth_dev = rte_eth_vdev_allocate(vdev, sizeof(*pmd));
if (eth_dev == NULL) {
MIF_LOG(ERR, "%s: Unable to allocate device struct.", name);
- return -1;
- }
-
- process_private = (struct pmd_process_private *)
- rte_zmalloc(name, sizeof(struct pmd_process_private),
- RTE_CACHE_LINE_SIZE);
-
- if (process_private == NULL) {
- MIF_LOG(ERR, "Failed to alloc memory for process private");
+ rte_free(process_private);
return -1;
}
eth_dev->process_private = process_private;
@@ -1988,7 +1988,7 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
ret = memif_socket_init(eth_dev, socket_filename);
if (ret < 0)
- return ret;
+ goto error;
memset(pmd->secret, 0, sizeof(char) * ETH_MEMIF_SECRET_SIZE);
if (secret != NULL)
@@ -2023,6 +2023,13 @@ memif_create(struct rte_vdev_device *vdev, enum memif_role_t role,
rte_eth_dev_probing_finish(eth_dev);
return 0;
+
+error:
+ /* rte_eth_dev_release_port() does not free process_private. */
+ rte_free(eth_dev->process_private);
+ eth_dev->process_private = NULL;
+ rte_eth_dev_release_port(eth_dev);
+ return ret;
}
static int
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 6/9] net/memif: refuse connection on stopped device
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (4 preceding siblings ...)
2026-10-16 19:58 ` [PATCH v4 5/9] net/memif: unwind partial device on create failure Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 7/9] net/memif: disconnect on device close Sriram Yagnaraman
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
A memif server cleared ETH_MEMIF_FLAG_DISABLED at dev_start (in
memif_connect_server()) but dev_stop never set it again. memif_create()
sets it, so a never-started server rejects peers, but once started and
then stopped the flag stayed clear and memif_msg_receive_init() kept
accepting connections. A peer connecting to a stopped server bound
pmd->cc and, on the CONNECT message, memif_connect() set
ETH_MEMIF_FLAG_CONNECTED and reported link up on a stopped port.
Set ETH_MEMIF_FLAG_DISABLED in memif_dev_stop() before disconnecting so
a stopped server refuses peers.
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/rte_eth_memif.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index e70443976d..852a7e2e8e 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1699,6 +1699,12 @@ memif_dev_stop(struct rte_eth_dev *dev)
rte_eal_alarm_cancel(memif_bad_desc_disconnect, dev);
rte_atomic_store_explicit(&pmd->bad_desc, false, rte_memory_order_relaxed);
+ /* Refuse peers while stopped: a server must not accept a connection
+ * after dev_stop, which would leave a live control channel on a
+ * stopped port.
+ */
+ pmd->flags |= ETH_MEMIF_FLAG_DISABLED;
+
memif_disconnect(dev);
for (i = 0; i < dev->data->nb_rx_queues; i++)
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 7/9] net/memif: disconnect on device close
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (5 preceding siblings ...)
2026-10-16 19:58 ` [PATCH v4 6/9] net/memif: refuse connection on stopped device Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 8/9] net/memif: fix bogus Tx count on invalid interrupt FD Sriram Yagnaraman
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
memif_dev_close() enqueued a disconnect message but never flushed it and
never tore down the control channel, leaking the control-channel
interrupt callback, its file descriptor and the pmd->cc allocation.
A server that was started and then stopped could still accept a peer and
hold a live pmd->cc at close. Call memif_disconnect() from close to flush
the queued disconnect and release the control channel; it is guarded on
pmd->cc and clears it, so it is safe when dev_stop already ran, and it is
a backstop for an init that races with stop. The companion fix that makes
dev_stop refuse peers removes the common trigger.
memif_disconnect() also walked the per-queue interrupt handles indexed
by the configured ring count without checking that the queue pointer was
set. Closing a device that was configured but never started (so queues
were never allocated) then dereferenced a NULL queue; skip NULL queue
entries.
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/memif_socket.c | 6 ++++++
drivers/net/memif/rte_eth_memif.c | 4 ++++
2 files changed, 10 insertions(+)
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 550e2e468a..cdf85e09a1 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -663,6 +663,9 @@ memif_disconnect(struct rte_eth_dev *dev)
continue;
}
+ if (mq == NULL)
+ continue;
+
rte_intr_fd_close(mq->intr_handle);
}
for (i = 0; i < pmd->cfg.num_s2c_rings; i++) {
@@ -678,6 +681,9 @@ memif_disconnect(struct rte_eth_dev *dev)
continue;
}
+ if (mq == NULL)
+ continue;
+
rte_intr_fd_close(mq->intr_handle);
}
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 852a7e2e8e..cdf77d77b0 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1723,6 +1723,10 @@ memif_dev_close(struct rte_eth_dev *dev)
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
memif_msg_enq_disconnect(pmd->cc, "Device closed", 0);
+ /* Flush the disconnect and tear down the control channel;
+ * memif_disconnect() is idempotent if dev_stop already ran.
+ */
+ memif_disconnect(dev);
for (i = 0; i < dev->data->nb_rx_queues; i++)
memif_rx_queue_release(dev, i);
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 8/9] net/memif: fix bogus Tx count on invalid interrupt FD
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (6 preceding siblings ...)
2026-10-16 19:58 ` [PATCH v4 7/9] net/memif: disconnect on device close Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 9/9] test/memif: add memif PMD unit test Sriram Yagnaraman
2026-10-17 21:17 ` [PATCH v4 0/9] net/memif: lifecycle fixes and " Stephen Hemminger
9 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
eth_memif_tx_zc() returns uint16_t but did "return -1" when the
interrupt FD was invalid, so the caller saw 65535 packets sent after
ring->head had already been published. The FD can legitimately be -1:
memif_init_queues() only warns and continues when eventfd() fails. Skip
the kick when the FD is invalid, as eth_memif_tx() does, and return the
real count.
Fixes: aedd054c5c2a ("drivers: check interrupt file descriptor validity")
Cc: stable@dpdk.org
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/rte_eth_memif.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index cdf77d77b0..0680dc3947 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1254,11 +1254,9 @@ eth_memif_tx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
rte_atomic_store_explicit(&ring->head, slot, rte_memory_order_release);
/* Send interrupt, if enabled. */
- if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) {
+ if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0 &&
+ rte_intr_fd_get(mq->intr_handle) >= 0) {
uint64_t a = 1;
- if (rte_intr_fd_get(mq->intr_handle) < 0)
- return -1;
-
ssize_t size = write(rte_intr_fd_get(mq->intr_handle),
&a, sizeof(a));
if (unlikely(size < 0)) {
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 9/9] test/memif: add memif PMD unit test
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (7 preceding siblings ...)
2026-10-16 19:58 ` [PATCH v4 8/9] net/memif: fix bogus Tx count on invalid interrupt FD Sriram Yagnaraman
@ 2026-10-16 19:58 ` Sriram Yagnaraman
2026-10-25 17:00 ` Stephen Hemminger
2026-10-17 21:17 ` [PATCH v4 0/9] net/memif: lifecycle fixes and " Stephen Hemminger
9 siblings, 1 reply; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-16 19:58 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Jakub Grajciar, stable, Sriram Yagnaraman
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>
---
MAINTAINERS | 1 +
app/test/meson.build | 1 +
app/test/test_pmd_memif.c | 764 ++++++++++++++++++++++++++++++++++++++
3 files changed, 766 insertions(+)
create mode 100644 app/test/test_pmd_memif.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 93ad53027d..ec154f3a05 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1164,6 +1164,7 @@ F: drivers/net/memif/
F: devtools/test-memif.sh
F: doc/guides/nics/memif.rst
F: doc/guides/nics/features/memif.ini
+F: app/test/test_pmd_memif.c
Crypto Drivers
diff --git a/app/test/meson.build b/app/test/meson.build
index 9805cf5bee..5ef1ced7ce 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -148,6 +148,7 @@ source_file_deps = {
'test_pflock.c': [],
'test_pie.c': ['sched'],
'test_pmd_af_packet.c': ['net_af_packet', 'ethdev', 'bus_vdev'],
+ 'test_pmd_memif.c': ['ethdev', 'net_memif', 'bus_vdev'],
'test_pmd_pcap.c': ['net_pcap', 'ethdev', 'bus_vdev'] + packet_burst_generator_deps,
'test_pmd_perf.c': ['ethdev', 'net'] + packet_burst_generator_deps,
'test_pmd_ring.c': ['net_ring', 'ethdev', 'bus_vdev'],
diff --git a/app/test/test_pmd_memif.c b/app/test/test_pmd_memif.c
new file mode 100644
index 0000000000..840ba4bf96
--- /dev/null
+++ b/app/test/test_pmd_memif.c
@@ -0,0 +1,764 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Ericsson AB
+ */
+
+/*
+ * Basic test of memif PMD functionality.
+ * Based off of the TAP PMD test (test_pmd_tap.c).
+ *
+ * memif connects two endpoints over shared memory. Unlike a NIC or TAP
+ * device, a memif port is only usable once its peer connects, so the test
+ * creates a server and a client memif port in this one process, connected
+ * over an abstract-namespace socket (the loopback setup documented in
+ * doc/guides/nics/memif.rst). The control-channel handshake is asynchronous,
+ * so the test waits for both ports to report link up before exercising the
+ * data path.
+ */
+
+#include "test.h"
+
+#include <stdio.h>
+
+#ifndef RTE_EXEC_ENV_LINUX
+
+/* memif PMD is only available on Linux */
+static int
+test_pmd_memif(void)
+{
+ printf("memif PMD not supported on this platform, skipping test\n");
+ return TEST_SKIPPED;
+}
+
+#else /* RTE_EXEC_ENV_LINUX */
+
+#include <string.h>
+#include <unistd.h>
+
+#include <rte_ethdev.h>
+#include <rte_bus_vdev.h>
+#include <rte_mbuf.h>
+#include <rte_ether.h>
+
+#define SOCKET0 0
+#define RING_SIZE 256
+#define NB_MBUF 4096
+#define MAX_PKT_BURST 32
+#define PKT_LEN 64
+
+/* memif ignores nb_tx_desc; the ring size is set via the rsize arg. */
+#define MEMIF_LOG2_RING 8
+
+/*
+ * Same socket name and id on both ports makes them a connected pair.
+ * The small-bsize pair forces chained descriptors; the default-bsize pair
+ * exercises the single-pool Tx fast path.
+ */
+#define MEMIF_SRV_NAME "net_memif_test_srv"
+#define MEMIF_CLI_NAME "net_memif_test_cli"
+#define MEMIF_SRV_ARGS "role=server,id=0,bsize=512,rsize=8,socket-abstract=yes,socket=memif-autotest"
+#define MEMIF_CLI_ARGS "role=client,id=0,bsize=512,rsize=8,socket-abstract=yes,socket=memif-autotest"
+#define MEMIF_FP_SRV_NAME "net_memif_test_fsrv"
+#define MEMIF_FP_CLI_NAME "net_memif_test_fcli"
+#define MEMIF_FP_SRV_ARGS "role=server,id=2,socket-abstract=yes,socket=memif-autotest-fp"
+#define MEMIF_FP_CLI_ARGS "role=client,id=2,socket-abstract=yes,socket=memif-autotest-fp"
+
+/* Time to wait for the asynchronous control-channel handshake. */
+#define CONNECT_TRIES 50
+#define CONNECT_DELAY_US 100000
+/* Shorter window for a negative check (peer must NOT connect). */
+#define REJECT_TRIES 10
+
+static struct rte_mempool *mp;
+static int memif_srv_port = -1;
+static int memif_cli_port = -1;
+static int memif_fp_srv_port = -1;
+static int memif_fp_cli_port = -1;
+
+static int
+find_port(const char *frag)
+{
+ uint16_t port_id;
+ char name[RTE_ETH_NAME_MAX_LEN];
+
+ RTE_ETH_FOREACH_DEV(port_id) {
+ if (rte_eth_dev_get_name_by_port(port_id, name) == 0 &&
+ strstr(name, frag) != NULL)
+ return port_id;
+ }
+ return -1;
+}
+
+static int
+configure_port(int port)
+{
+ struct rte_eth_conf port_conf;
+ int ret;
+
+ memset(&port_conf, 0, sizeof(port_conf));
+
+ ret = rte_eth_dev_configure(port, 1, 1, &port_conf);
+ if (ret < 0) {
+ printf("Configure failed for port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ ret = rte_eth_tx_queue_setup(port, 0, RING_SIZE, SOCKET0, NULL);
+ if (ret < 0) {
+ printf("TX queue setup failed for port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ ret = rte_eth_rx_queue_setup(port, 0, RING_SIZE, SOCKET0, NULL, mp);
+ if (ret < 0) {
+ printf("RX queue setup failed for port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ ret = rte_eth_dev_start(port);
+ if (ret < 0) {
+ printf("Error starting port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Wait until both ports report link up, i.e. the peers have connected. */
+static int
+wait_for_link_pair_tries(int p0, int p1, int tries)
+{
+ struct rte_eth_link l0, l1;
+ int i;
+
+ for (i = 0; i < tries; i++) {
+ if (rte_eth_link_get_nowait(p0, &l0) == 0 &&
+ rte_eth_link_get_nowait(p1, &l1) == 0 &&
+ l0.link_status == RTE_ETH_LINK_UP &&
+ l1.link_status == RTE_ETH_LINK_UP)
+ return 0;
+ usleep(CONNECT_DELAY_US);
+ }
+ return -1;
+}
+
+static int
+wait_for_link_pair(int p0, int p1)
+{
+ return wait_for_link_pair_tries(p0, p1, CONNECT_TRIES);
+}
+
+static int
+wait_for_link(void)
+{
+ return wait_for_link_pair(memif_srv_port, memif_cli_port);
+}
+
+static struct rte_mbuf *
+create_test_packet(uint16_t pkt_len, uint8_t seed)
+{
+ struct rte_mbuf *mbuf;
+ struct rte_ether_hdr *eth_hdr;
+ uint8_t *payload;
+ uint16_t i;
+
+ mbuf = rte_pktmbuf_alloc(mp);
+ if (mbuf == NULL)
+ return NULL;
+
+ if (pkt_len < RTE_ETHER_MIN_LEN)
+ pkt_len = RTE_ETHER_MIN_LEN;
+
+ eth_hdr = (struct rte_ether_hdr *)rte_pktmbuf_append(mbuf, pkt_len);
+ if (eth_hdr == NULL) {
+ rte_pktmbuf_free(mbuf);
+ return NULL;
+ }
+
+ memset(ð_hdr->dst_addr, 0xFF, RTE_ETHER_ADDR_LEN);
+ memset(ð_hdr->src_addr, 0x02, RTE_ETHER_ADDR_LEN);
+ eth_hdr->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
+
+ payload = (uint8_t *)(eth_hdr + 1);
+ for (i = 0; i < pkt_len - sizeof(*eth_hdr); i++)
+ payload[i] = (uint8_t)(seed + i);
+
+ return mbuf;
+}
+
+static int
+test_memif_dev_info(void)
+{
+ struct rte_eth_dev_info dev_info;
+ int ret;
+
+ ret = rte_eth_dev_info_get(memif_srv_port, &dev_info);
+ TEST_ASSERT(ret == 0, "dev_info_get failed: %s", rte_strerror(-ret));
+ TEST_ASSERT(strcmp(dev_info.driver_name, "net_memif") == 0,
+ "unexpected driver name: %s", dev_info.driver_name);
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_link_up(void)
+{
+ /* The pair must have connected during setup. */
+ TEST_ASSERT(wait_for_link() == 0,
+ "memif server/client did not reach link up");
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_stats_get(void)
+{
+ struct rte_eth_stats stats;
+
+ TEST_ASSERT(rte_eth_stats_get(memif_srv_port, &stats) == 0,
+ "stats_get failed for server port");
+ TEST_ASSERT(rte_eth_stats_get(memif_cli_port, &stats) == 0,
+ "stats_get failed for client port");
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_stats_reset(void)
+{
+ struct rte_eth_stats stats;
+
+ TEST_ASSERT(rte_eth_stats_reset(memif_srv_port) == 0,
+ "stats_reset failed");
+ TEST_ASSERT(rte_eth_stats_get(memif_srv_port, &stats) == 0,
+ "stats_get after reset failed");
+ TEST_ASSERT(stats.ipackets == 0 && stats.opackets == 0 &&
+ stats.ibytes == 0 && stats.obytes == 0,
+ "stats not zero after reset");
+ return TEST_SUCCESS;
+}
+
+/*
+ * Send a burst from the client and receive it on the server, verifying the
+ * payload is delivered intact. This exercises the Rx/Tx data path over the
+ * connected memif pair.
+ */
+static int
+test_memif_send_receive(void)
+{
+ struct rte_mbuf *tx[MAX_PKT_BURST];
+ struct rte_mbuf *rx[MAX_PKT_BURST];
+ const uint16_t nb = MAX_PKT_BURST / 2;
+ uint16_t nb_tx, nb_rx, got;
+ int i, tries;
+
+ TEST_ASSERT(wait_for_link() == 0, "link not up before send/receive");
+
+ for (i = 0; i < nb; i++) {
+ tx[i] = create_test_packet(PKT_LEN, (uint8_t)i);
+ if (tx[i] == NULL) {
+ while (--i >= 0)
+ rte_pktmbuf_free(tx[i]);
+ TEST_ASSERT(0, "failed to build test packet");
+ }
+ }
+
+ nb_tx = rte_eth_tx_burst(memif_cli_port, 0, tx, nb);
+ for (i = nb_tx; i < nb; i++)
+ rte_pktmbuf_free(tx[i]);
+ TEST_ASSERT(nb_tx > 0, "client transmitted no packets");
+
+ /* Receive may take a moment; poll a few times. */
+ got = 0;
+ for (tries = 0; tries < 10 && got < nb_tx; tries++) {
+ nb_rx = rte_eth_rx_burst(memif_srv_port, 0, rx, MAX_PKT_BURST);
+ for (i = 0; i < nb_rx; i++) {
+ /* Payload must match the position-dependent pattern. */
+ uint8_t *p = rte_pktmbuf_mtod_offset(rx[i], uint8_t *,
+ sizeof(struct rte_ether_hdr));
+ uint16_t plen = PKT_LEN -
+ sizeof(struct rte_ether_hdr);
+ uint16_t k;
+ int bad = rte_pktmbuf_pkt_len(rx[i]) != PKT_LEN;
+
+ for (k = 0; !bad && k < plen; k++)
+ if (p[k] != (uint8_t)(got + k))
+ bad = 1;
+ if (bad) {
+ for (k = i; k < nb_rx; k++)
+ rte_pktmbuf_free(rx[k]);
+ TEST_ASSERT(0,
+ "packet %u payload/length mismatch",
+ got);
+ }
+ rte_pktmbuf_free(rx[i]);
+ got++;
+ }
+ if (got < nb_tx)
+ usleep(10000);
+ }
+
+ TEST_ASSERT(got == nb_tx,
+ "received %u packets, expected %u", got, nb_tx);
+ return TEST_SUCCESS;
+}
+
+/*
+ * Single-segment loopback on the default-bsize pair, where bsize exceeds
+ * the mbuf data room so eth_memif_tx() takes the single-pool fast path.
+ */
+static int
+test_memif_fastpath(void)
+{
+ struct rte_mbuf *tx[MAX_PKT_BURST];
+ struct rte_mbuf *rx[MAX_PKT_BURST];
+ const uint16_t nb = MAX_PKT_BURST / 2;
+ uint16_t nb_tx, got;
+ int i, tries;
+
+ TEST_ASSERT(wait_for_link_pair(memif_fp_srv_port, memif_fp_cli_port)
+ == 0, "fast-path pair not up");
+
+ for (i = 0; i < nb; i++) {
+ tx[i] = create_test_packet(PKT_LEN, (uint8_t)i);
+ if (tx[i] == NULL) {
+ while (--i >= 0)
+ rte_pktmbuf_free(tx[i]);
+ TEST_ASSERT(0, "failed to build test packet");
+ }
+ }
+
+ nb_tx = rte_eth_tx_burst(memif_fp_cli_port, 0, tx, nb);
+ for (i = nb_tx; i < nb; i++)
+ rte_pktmbuf_free(tx[i]);
+ TEST_ASSERT(nb_tx > 0, "fast-path client transmitted no packets");
+
+ got = 0;
+ for (tries = 0; tries < 10 && got < nb_tx; tries++) {
+ uint16_t nb_rx = rte_eth_rx_burst(memif_fp_srv_port, 0, rx,
+ MAX_PKT_BURST);
+ for (i = 0; i < nb_rx; i++) {
+ uint8_t *p = rte_pktmbuf_mtod_offset(rx[i], uint8_t *,
+ sizeof(struct rte_ether_hdr));
+ uint16_t plen = PKT_LEN -
+ sizeof(struct rte_ether_hdr);
+ uint16_t k;
+ int bad = rte_pktmbuf_pkt_len(rx[i]) != PKT_LEN;
+
+ for (k = 0; !bad && k < plen; k++)
+ if (p[k] != (uint8_t)(got + k))
+ bad = 1;
+ if (bad) {
+ for (k = i; k < nb_rx; k++)
+ rte_pktmbuf_free(rx[k]);
+ TEST_ASSERT(0,
+ "fast-path packet %u payload/length mismatch",
+ got);
+ }
+ rte_pktmbuf_free(rx[i]);
+ got++;
+ }
+ if (got < nb_tx)
+ usleep(10000);
+ }
+
+ TEST_ASSERT(got == nb_tx, "fast-path received %u expected %u",
+ got, nb_tx);
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_stop_start(void)
+{
+ TEST_ASSERT(rte_eth_dev_stop(memif_cli_port) == 0,
+ "stop client failed");
+ TEST_ASSERT(rte_eth_dev_start(memif_cli_port) == 0,
+ "restart client failed");
+ /* After a restart the pair must reconnect. */
+ TEST_ASSERT(wait_for_link() == 0, "link did not recover after restart");
+ return TEST_SUCCESS;
+}
+
+/*
+ * A zero-length Tx burst must be accepted and return 0 without touching
+ * the (empty) mbuf array. Regression test for the empty-burst guard in
+ * eth_memif_tx().
+ */
+static int
+test_memif_tx_empty_burst(void)
+{
+ struct rte_mbuf *dummy = NULL;
+
+ TEST_ASSERT(wait_for_link() == 0, "link not up before empty burst");
+ TEST_ASSERT(rte_eth_tx_burst(memif_cli_port, 0, &dummy, 0) == 0,
+ "zero-length Tx burst did not return 0");
+ return TEST_SUCCESS;
+}
+
+/*
+ * Close a configured but never-started port. The queues were never
+ * allocated, so memif_disconnect() must skip the NULL queue entries
+ * rather than dereference them. Regression test for the NULL-queue guard
+ * in the disconnect-on-close fix. Uses a throwaway device so the suite
+ * pair stays intact.
+ */
+static int
+test_memif_close_without_stop(void)
+{
+ int sport, ret;
+ struct rte_eth_conf port_conf;
+
+ ret = rte_vdev_init("net_memif_test_tmps",
+ "role=server,id=1,socket-abstract=yes,socket=memif-autotest-tmp");
+ if (ret < 0) {
+ printf("could not create throwaway server: %s\n",
+ rte_strerror(-ret));
+ return TEST_FAILED;
+ }
+
+ sport = find_port("net_memif_test_tmps");
+ if (sport < 0) {
+ rte_vdev_uninit("net_memif_test_tmps");
+ TEST_ASSERT(0, "throwaway server port not found");
+ }
+
+ /* Configure but do not start: close is rejected on a started port. */
+ memset(&port_conf, 0, sizeof(port_conf));
+ ret = rte_eth_dev_configure(sport, 1, 1, &port_conf);
+ if (ret < 0) {
+ rte_vdev_uninit("net_memif_test_tmps");
+ TEST_ASSERT(0, "configure of throwaway server failed: %s",
+ rte_strerror(-ret));
+ }
+
+ /* Close without ever starting; must not crash on the NULL queues. */
+ ret = rte_eth_dev_close(sport);
+ rte_vdev_uninit("net_memif_test_tmps");
+ TEST_ASSERT(ret == 0, "close without stop failed: %s",
+ rte_strerror(-ret));
+
+ return TEST_SUCCESS;
+}
+
+/*
+ * A server that has been started and then stopped must refuse peers: a
+ * connection accepted while stopped would leave a live control channel on
+ * a stopped port. Regression test for the refuse-connection-on-stopped-
+ * device fix. Without it the stopped server reports link up here.
+ */
+static int
+test_memif_stopped_server_rejects_peer(void)
+{
+ int sport, cport, ret;
+
+ ret = rte_vdev_init("net_memif_test_tmp2s",
+ "role=server,id=3,socket-abstract=yes,socket=memif-autotest-tmp2");
+ if (ret < 0) {
+ printf("could not create throwaway server: %s\n",
+ rte_strerror(-ret));
+ return TEST_FAILED;
+ }
+ sport = find_port("net_memif_test_tmp2s");
+ if (sport < 0) {
+ rte_vdev_uninit("net_memif_test_tmp2s");
+ TEST_ASSERT(0, "throwaway server port not found");
+ }
+ if (configure_port(sport) < 0) {
+ rte_vdev_uninit("net_memif_test_tmp2s");
+ TEST_ASSERT(0, "throwaway server start failed");
+ }
+ if (rte_eth_dev_stop(sport) < 0) {
+ rte_eth_dev_close(sport);
+ rte_vdev_uninit("net_memif_test_tmp2s");
+ TEST_ASSERT(0, "throwaway server stop failed");
+ }
+
+ ret = rte_vdev_init("net_memif_test_tmp2c",
+ "role=client,id=3,socket-abstract=yes,socket=memif-autotest-tmp2");
+ if (ret < 0) {
+ rte_eth_dev_close(sport);
+ rte_vdev_uninit("net_memif_test_tmp2s");
+ TEST_ASSERT(0, "could not create throwaway client: %s",
+ rte_strerror(-ret));
+ }
+ cport = find_port("net_memif_test_tmp2c");
+ if (cport < 0 || configure_port(cport) < 0) {
+ rte_eth_dev_close(sport);
+ rte_vdev_uninit("net_memif_test_tmp2c");
+ rte_vdev_uninit("net_memif_test_tmp2s");
+ TEST_ASSERT(0, "throwaway client setup failed");
+ }
+
+ /* The stopped server must not come up (short window; it never will). */
+ ret = wait_for_link_pair_tries(sport, cport, REJECT_TRIES);
+
+ rte_eth_dev_stop(cport);
+ rte_eth_dev_close(cport);
+ rte_eth_dev_close(sport);
+ rte_vdev_uninit("net_memif_test_tmp2c");
+ rte_vdev_uninit("net_memif_test_tmp2s");
+
+ TEST_ASSERT(ret != 0, "stopped server accepted a peer");
+ return TEST_SUCCESS;
+}
+
+#define BIG_PKT_LEN 4000 /* > bsize (512) => chained descriptors */
+#define BIG_SEG_LEN 1000 /* each mbuf segment carries this many bytes */
+/* ~8 descriptors per packet; enough to wrap the 2^MEMIF_LOG2_RING ring */
+#define WRAP_PKTS ((1 << MEMIF_LOG2_RING) / 8 + 8)
+
+/*
+ * Build a multi-segment mbuf of pkt_len bytes, chaining as many pool mbufs
+ * as needed (BIG_SEG_LEN bytes each) with a position-dependent pattern.
+ */
+static struct rte_mbuf *
+create_big_packet(uint16_t pkt_len, uint8_t seed)
+{
+ struct rte_mbuf *head = NULL, *tail = NULL;
+ uint16_t done = 0;
+
+ while (done < pkt_len) {
+ struct rte_mbuf *seg = rte_pktmbuf_alloc(mp);
+ uint16_t seg_len = RTE_MIN((uint16_t)BIG_SEG_LEN,
+ (uint16_t)(pkt_len - done));
+ uint8_t *data;
+ uint16_t i;
+
+ if (seg == NULL) {
+ rte_pktmbuf_free(head);
+ return NULL;
+ }
+ data = (uint8_t *)rte_pktmbuf_append(seg, seg_len);
+ if (data == NULL) {
+ rte_pktmbuf_free(seg);
+ rte_pktmbuf_free(head);
+ return NULL;
+ }
+ for (i = 0; i < seg_len; i++)
+ data[i] = (uint8_t)(seed + done + i);
+
+ if (head == NULL) {
+ head = seg;
+ tail = seg;
+ } else {
+ tail->next = seg;
+ tail = seg;
+ head->nb_segs++;
+ head->pkt_len += seg_len;
+ }
+ done += seg_len;
+ }
+ return head;
+}
+
+/* Verify a received packet's payload, walking its segment chain. */
+static int
+check_big_packet(struct rte_mbuf *m, uint16_t pkt_len, uint8_t seed)
+{
+ uint16_t off = 0;
+ struct rte_mbuf *seg;
+
+ if (rte_pktmbuf_pkt_len(m) != pkt_len)
+ return -1;
+ for (seg = m; seg != NULL; seg = seg->next) {
+ const uint8_t *data = rte_pktmbuf_mtod(seg, const uint8_t *);
+ uint16_t i;
+
+ for (i = 0; i < rte_pktmbuf_data_len(seg); i++, off++)
+ if (data[i] != (uint8_t)(seed + off))
+ return -1;
+ }
+ return off == pkt_len ? 0 : -1;
+}
+
+/*
+ * Send multi-segment packets larger than the memif buffer size, repeated
+ * enough to wrap the ring, and verify each is delivered intact. Exercises
+ * the chained-descriptor Rx and Tx paths across a ring wrap.
+ */
+static int
+test_memif_multiseg(void)
+{
+ unsigned int sent = 0, recv = 0;
+ unsigned int iter;
+
+ TEST_ASSERT(wait_for_link() == 0, "link not up before multiseg test");
+
+ for (iter = 0; iter < WRAP_PKTS; iter++) {
+ struct rte_mbuf *tx, *rx[MAX_PKT_BURST];
+ uint16_t n, j;
+ int tries;
+
+ tx = create_big_packet(BIG_PKT_LEN, (uint8_t)iter);
+ TEST_ASSERT(tx != NULL, "failed to build big packet %u", iter);
+ if (tx->nb_segs <= 1) {
+ rte_pktmbuf_free(tx);
+ TEST_ASSERT(0, "test packet is not multi-segment");
+ }
+
+ if (rte_eth_tx_burst(memif_cli_port, 0, &tx, 1) != 1) {
+ rte_pktmbuf_free(tx);
+ continue;
+ }
+ sent++;
+
+ for (tries = 0; tries < 10; tries++) {
+ n = rte_eth_rx_burst(memif_srv_port, 0, rx,
+ MAX_PKT_BURST);
+ for (j = 0; j < n; j++) {
+ if (check_big_packet(rx[j], BIG_PKT_LEN,
+ (uint8_t)recv) != 0) {
+ uint16_t k;
+ for (k = j; k < n; k++)
+ rte_pktmbuf_free(rx[k]);
+ TEST_ASSERT(0,
+ "multi-seg payload corrupted at pkt %u",
+ recv);
+ }
+ rte_pktmbuf_free(rx[j]);
+ recv++;
+ }
+ if (recv >= sent)
+ break;
+ usleep(2000);
+ }
+ }
+
+ TEST_ASSERT(recv == sent,
+ "multi-seg: sent %u received %u", sent, recv);
+ return TEST_SUCCESS;
+}
+
+static void
+test_memif_cleanup(void)
+{
+ if (memif_srv_port >= 0) {
+ rte_eth_dev_stop(memif_srv_port);
+ rte_eth_dev_close(memif_srv_port);
+ }
+ if (memif_cli_port >= 0) {
+ rte_eth_dev_stop(memif_cli_port);
+ rte_eth_dev_close(memif_cli_port);
+ }
+ if (memif_fp_srv_port >= 0) {
+ rte_eth_dev_stop(memif_fp_srv_port);
+ rte_eth_dev_close(memif_fp_srv_port);
+ }
+ if (memif_fp_cli_port >= 0) {
+ rte_eth_dev_stop(memif_fp_cli_port);
+ rte_eth_dev_close(memif_fp_cli_port);
+ }
+ rte_vdev_uninit(MEMIF_SRV_NAME);
+ rte_vdev_uninit(MEMIF_CLI_NAME);
+ rte_vdev_uninit(MEMIF_FP_SRV_NAME);
+ rte_vdev_uninit(MEMIF_FP_CLI_NAME);
+ if (mp != NULL) {
+ rte_mempool_free(mp);
+ mp = NULL;
+ }
+ memif_srv_port = -1;
+ memif_cli_port = -1;
+ memif_fp_srv_port = -1;
+ memif_fp_cli_port = -1;
+}
+
+static int
+test_memif_setup(void)
+{
+ int ret;
+
+ mp = rte_pktmbuf_pool_create("memif_test_pool", NB_MBUF, 32, 0,
+ RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id());
+ if (mp == NULL) {
+ printf("Error: failed to create mempool: %s\n",
+ rte_strerror(rte_errno));
+ return -1;
+ }
+
+ ret = rte_vdev_init(MEMIF_SRV_NAME, MEMIF_SRV_ARGS);
+ if (ret < 0) {
+ printf("Error: failed to create memif server: %s\n",
+ rte_strerror(-ret));
+ rte_mempool_free(mp);
+ mp = NULL;
+ return TEST_FAILED;
+ }
+
+ ret = rte_vdev_init(MEMIF_CLI_NAME, MEMIF_CLI_ARGS);
+ if (ret < 0) {
+ printf("Error: failed to create memif client: %s\n",
+ rte_strerror(-ret));
+ rte_vdev_uninit(MEMIF_SRV_NAME);
+ rte_mempool_free(mp);
+ mp = NULL;
+ return -1;
+ }
+
+ memif_srv_port = find_port(MEMIF_SRV_NAME);
+ memif_cli_port = find_port(MEMIF_CLI_NAME);
+ if (memif_srv_port < 0 || memif_cli_port < 0) {
+ printf("Error: failed to find memif ports\n");
+ test_memif_cleanup();
+ return -1;
+ }
+
+ if (configure_port(memif_srv_port) < 0 ||
+ configure_port(memif_cli_port) < 0) {
+ test_memif_cleanup();
+ return -1;
+ }
+
+ /* Second pair with default bsize to exercise the Tx fast path. */
+ if (rte_vdev_init(MEMIF_FP_SRV_NAME, MEMIF_FP_SRV_ARGS) < 0 ||
+ rte_vdev_init(MEMIF_FP_CLI_NAME, MEMIF_FP_CLI_ARGS) < 0) {
+ printf("Error: failed to create fast-path memif pair\n");
+ test_memif_cleanup();
+ return TEST_FAILED;
+ }
+
+ memif_fp_srv_port = find_port(MEMIF_FP_SRV_NAME);
+ memif_fp_cli_port = find_port(MEMIF_FP_CLI_NAME);
+ if (memif_fp_srv_port < 0 || memif_fp_cli_port < 0) {
+ printf("Error: failed to find fast-path memif ports\n");
+ test_memif_cleanup();
+ return TEST_FAILED;
+ }
+
+ if (configure_port(memif_fp_srv_port) < 0 ||
+ configure_port(memif_fp_cli_port) < 0) {
+ test_memif_cleanup();
+ return -1;
+ }
+
+ return 0;
+}
+
+static struct unit_test_suite test_pmd_memif_suite = {
+ .setup = test_memif_setup,
+ .teardown = test_memif_cleanup,
+ .suite_name = "memif PMD Unit Test Suite",
+ .unit_test_cases = {
+ TEST_CASE(test_memif_dev_info),
+ TEST_CASE(test_memif_link_up),
+ TEST_CASE(test_memif_stats_get),
+ TEST_CASE(test_memif_send_receive),
+ TEST_CASE(test_memif_fastpath),
+ TEST_CASE(test_memif_multiseg),
+ TEST_CASE(test_memif_stats_reset),
+ TEST_CASE(test_memif_stop_start),
+ TEST_CASE(test_memif_tx_empty_burst),
+ TEST_CASE(test_memif_close_without_stop),
+ TEST_CASE(test_memif_stopped_server_rejects_peer),
+ TEST_CASES_END()
+ }
+};
+
+static int
+test_pmd_memif(void)
+{
+ return unit_test_suite_runner(&test_pmd_memif_suite);
+}
+
+#endif /* RTE_EXEC_ENV_LINUX */
+
+REGISTER_FAST_TEST(memif_pmd_autotest, NOHUGE_OK, ASAN_OK, test_pmd_memif);
--
2.43.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/9] net/memif: lifecycle fixes and unit test
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (8 preceding siblings ...)
2026-10-16 19:58 ` [PATCH v4 9/9] test/memif: add memif PMD unit test Sriram Yagnaraman
@ 2026-10-17 21:17 ` Stephen Hemminger
9 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2026-10-17 21:17 UTC (permalink / raw)
To: Sriram Yagnaraman; +Cc: dev, Jakub Grajciar, stable
On Fri, 16 Oct 2026 21:58:48 +0200
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> wrote:
> This series fixes a set of error-path and lifecycle bugs in the memif
> PMD and adds a unit test for the driver.
>
> The first eight patches are fixes:
>
> - guard eth_memif_tx() against an empty burst (nb_pkts == 0);
> - close file descriptor 0 instead of leaking it;
> - do not close the EAL-owned memseg fd in zero-copy mode;
> - free the MAC address allocated during probe on failure;
> - unwind a partially-created device on create failure;
> - refuse connections on a stopped server so it does not come up on a
> stopped port;
> - disconnect on close so a stopped server's control channel is not
> leaked, and make memif_disconnect() safe on a never-started device;
> - fix eth_memif_tx_zc() returning 65535 on an invalid interrupt fd.
>
> The last patch adds a memif PMD unit test (app/test, memif_pmd_autotest):
> a server/client vdev pair over a loopback socket covering dev_info, link
> state, stats, single-segment loopback (slow and fast path), multi-segment
> loopback across a ring wrap, stop/start reconnect, empty Tx burst, close
> without stop, and a stopped server refusing a peer.
>
> Verified: memif_pmd_autotest passes 11/11 (~1.4s); devtools/test-memif.sh
> two-process forwarding OK; each commit builds with -Dwerror=true;
> check-git-log and checkpatch are clean.
>
> Thanks to Stephen Hemminger and the ai-code-review bot for the reviews
> that shaped this series.
Applied to next-net.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 9/9] test/memif: add memif PMD unit test
2026-10-16 19:58 ` [PATCH v4 9/9] test/memif: add memif PMD unit test Sriram Yagnaraman
@ 2026-10-25 17:00 ` Stephen Hemminger
2026-10-27 10:18 ` Sriram Yagnaraman
0 siblings, 1 reply; 13+ messages in thread
From: Stephen Hemminger @ 2026-10-25 17:00 UTC (permalink / raw)
To: Sriram Yagnaraman; +Cc: dev, Jakub Grajciar, stable
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* RE: [PATCH v4 9/9] test/memif: add memif PMD unit test
2026-10-25 17:00 ` Stephen Hemminger
@ 2026-10-27 10:18 ` Sriram Yagnaraman
0 siblings, 0 replies; 13+ messages in thread
From: Sriram Yagnaraman @ 2026-10-27 10:18 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev@dpdk.org, Jakub Grajciar, stable@dpdk.org
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Sunday, 25 October 2026 18:01
> To: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> Cc: dev@dpdk.org; Jakub Grajciar <jgrajcia@cisco.com>; stable@dpdk.org
> Subject: Re: [PATCH v4 9/9] test/memif: add memif PMD unit test
>
> 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://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fclaude.
> ai%2Fcode%2Fsession_01FA4k3HfYcHiM1tu71SfXb1&data=05%7C02%7Csrira
> m.yagnaraman%40ericsson.com%7Cf1e8aaf65f3745c8e82208df32b98aa7%7
> C92e84cebfbfd47abbe52080c6b87953f%7C0%7C0%7C639285444622358195%
> 7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuM
> DAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7
> C%7C&sdata=iKB%2BMRPw352jKwZQGBZlRGaZa07PINx5QR50NJ4aduM%3D&
> reserved=0
I am cheap and don't have a Claude subscription yet, so could not access the above session.
But I used my corporate subscription (incidentally using Claude Opus as well 😊) to analyze and review the patch
The analysis is spot on, on the root cause. I was able to reproduce the SIGSEGV running the test in a loop x100 times.
With your patch I couldn't reproduce it.
I got AI to make a variant of the fix here; you are welcome to use either version to post a proper patch.
From 47cbd5064033eca40a14301b99c4cace2d60fd71 Mon Sep 17 00:00:00 2001
From: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
Date: Tue, 27 Oct 2026 10:25:00 +0100
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 was active the application side
only marked the callback pending and carried on freeing queues, regions
and finally the port; the handler then dereferenced dev->data on a
released ethdev and crashed. 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.
Detach the control channel under cc_lock, then, when called from an
application thread, wait for the handler with
rte_intr_callback_unregister_sync() before tearing anything down. The
handler drops its own callback once it sees the channel detached
(pmd->cc != cc), so the EOF that keeps the fd readable does not stall
the wait. When called from the interrupt thread the handler cannot wait
for itself, so it hands the free to the pending unregister callback.
The teardown is split out into memif_cc_detach(), which has a single
"call without cc_lock held" contract; once it returns the handler can no
longer run for the device, so the queue and region teardown in
memif_disconnect() needs no further locking.
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/memif_socket.c | 146 +++++++++++++++++++++----------
1 file changed, 98 insertions(+), 48 deletions(-)
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 971964133c..70529b657d 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -572,84 +572,117 @@ memif_msg_enq_connected(struct rte_eth_dev *dev)
}
static void
-memif_intr_unregister_handler(const 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;
- RTE_SET_USED(intr_handle);
- /* close control channel fd */
rte_intr_fd_close(cc->intr_handle);
- /* clear message queue */
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);
}
-void
-memif_disconnect(struct rte_eth_dev *dev)
+/*
+ * 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(const struct rte_intr_handle *intr_handle, void *arg)
{
- struct pmd_internals *pmd = dev->data->dev_private;
+ RTE_SET_USED(intr_handle);
+ memif_cc_free(arg);
+}
+
+/*
+ * Detach the control channel from the device and free it, synchronising
+ * with memif_intr_handler() which may be running in the interrupt thread.
+ * Must be called WITHOUT pmd->cc_lock held (the handler takes it). On
+ * return the handler can no longer run for this device.
+ */
+static void
+memif_cc_detach(struct pmd_internals *pmd)
+{
+ struct memif_control_channel *cc;
struct memif_msg_queue_elt *elt, *next;
- struct memif_queue *mq;
- struct rte_intr_handle *ih;
- int i;
int ret;
- 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);
+ if (TAILQ_FIRST(&cc->msg_queue) != NULL)
+ MIF_LOG(WARNING, "Unexpected message(s) in message queue.");
+ }
+ rte_spinlock_unlock(&pmd->cc_lock);
- /* 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 (cc == NULL)
+ return;
- ih = pmd->cc->intr_handle;
- if (rte_intr_fd_get(ih) >= 0) {
- ret = rte_intr_callback_unregister(ih,
- memif_intr_handler,
- pmd->cc);
- /*
- * If callback is active (disconnecting based on
- * received control message).
- */
- 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;
- if (ret <= 0)
- MIF_LOG(WARNING,
- "Failed to unregister control channel callback.");
+ if (rte_thread_is_intr()) {
+ /*
+ * In the interrupt thread (possibly memif_intr_handler()
+ * itself): cannot wait for the handler. If it is active, hand
+ * the free to the pending-unregister callback.
+ */
+ 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);
+ } else {
+ /*
+ * In an application thread (dev_stop/dev_close): the handler
+ * may be mid-flight and will drop itself once it sees pmd->cc
+ * detached. Wait for it, then free (-ENOENT: already dropped).
+ */
+ ret = rte_intr_callback_unregister_sync(cc->intr_handle,
+ memif_intr_handler, cc);
+ if (ret > 0 || ret == -ENOENT) {
+ memif_cc_free(cc);
+ ret = 1;
}
}
+ if (ret <= 0)
+ MIF_LOG(WARNING, "Failed to unregister control channel callback.");
+}
+
+void
+memif_disconnect(struct rte_eth_dev *dev)
+{
+ struct pmd_internals *pmd = dev->data->dev_private;
+ struct memif_queue *mq;
+ int i;
+
+ 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_unlock(&pmd->cc_lock);
+ /* Tear down the control channel, synchronising with the handler. */
+ memif_cc_detach(pmd);
+
+ /*
+ * The handler can no longer run for this device, so the queues and
+ * regions can be freed without holding cc_lock.
+ */
/* unconfig interrupts */
for (i = 0; i < pmd->cfg.num_c2s_rings; i++) {
if (pmd->role == MEMIF_ROLE_CLIENT) {
@@ -868,6 +901,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);
@@ -898,6 +933,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.7
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-27 10:18 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 19:58 [PATCH v4 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 1/9] net/memif: guard against empty Tx burst Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 2/9] net/memif: close file descriptor zero Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 3/9] net/memif: do not close EAL-owned memseg file descriptor Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 4/9] net/memif: fix MAC address leak on probe failure Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 5/9] net/memif: unwind partial device on create failure Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 6/9] net/memif: refuse connection on stopped device Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 7/9] net/memif: disconnect on device close Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 8/9] net/memif: fix bogus Tx count on invalid interrupt FD Sriram Yagnaraman
2026-10-16 19:58 ` [PATCH v4 9/9] test/memif: add memif PMD unit test Sriram Yagnaraman
2026-10-25 17:00 ` Stephen Hemminger
2026-10-27 10:18 ` Sriram Yagnaraman
2026-10-17 21:17 ` [PATCH v4 0/9] net/memif: lifecycle fixes and " Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox