Re: [PATCH 0/6] net/memif: lifecycle fixes and unit test

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 14 Oct 2026 11:12:51 +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 five patches are fixes:
> 
>  - guard eth_memif_tx() against an empty burst so a zero-count call
>    returns before touching the burst or the ring;
>  - close file descriptor 0 instead of leaking it (the close path
>    tested for fd > 0 instead of fd >= 0);
>  - free the MAC address allocated during probe when a later step
>    fails, instead of leaking it;
>  - unwind a partially-created device on failure in memif_create(),
>    releasing the port and the process-private data rather than
>    returning with them registered and leaked;
>  - disconnect the device on close so queues and shared regions are
>    torn down, and make memif_disconnect() safe to call on a device
>    that was configured but never started.
> 
> The last patch adds a memif PMD unit test (app/test, memif_pmd_autotest)
> that stands up a server/client vdev pair over a loopback socket and
> covers dev_info, link state, stats, single- and multi-segment loopback,
> stop/start reconnect, empty Tx burst, and close without stop.
> 
> Verified: memif_pmd_autotest passes 9/9; check-git-log and checkpatch
> are clean across the series.


After walking through code when using bonding etc, sometimes apps
send 0 packets as keepalive time call.

AI did find some other things to address..

Series applies cleanly to main (2a58f6e) and builds at every commit
with -Dwerror=true. Not run. No errors; warnings below, so no
Reviewed-by yet.


[PATCH 1/6] net/memif: guard against empty Tx burst

Warning: wrong Fixes tag. The bufs[0]->pool read in the single-pool
check came in with the Tx fast path, not the original driver:

  Fixes: b5613c8f9d0a ("net/memif: add a Tx fast path")

Info: put the check after the ring == NULL block, not before it. An
empty burst in a secondary process then still triggers the region
request through rte_eth_link_get(), as it does in zero-copy mode.


[PATCH 2/6] net/memif: close file descriptor zero

Warning: the zero-copy branch in memif_free_regions() closes an fd
the driver does not own. For i > 0 with ETH_MEMIF_FLAG_ZERO_COPY,
r->fd came from rte_memseg_get_fd() in memif_region_init_zc(), and
rte_memory.h says that fd is EAL internal and read-only. Every
zero-copy client disconnect closes EAL's hugepage fd; the next
connect passes the peer a stale (possibly reused) fd number.
Pre-existing, but this patch edits that exact line. Fix it in a
separate patch in this series instead of making it ">= 0":

	if (i > 0 && (pmd->flags & ETH_MEMIF_FLAG_ZERO_COPY)) {
		/* memseg fd is owned by EAL, do not close */
		r->addr = NULL;
		r->fd = -1;
	}

  Fixes: 43b815d88188 ("net/memif: support zero-copy slave")


[PATCH 3/6] net/memif: fix MAC address leak on probe failure

Info: the secondary-process "return -1" -> "return -ENOMEM" change
is not in the commit message. Mention it or drop it.


[PATCH 4/6] net/memif: unwind partial device on create failure

Info: the process_private leak came from multi-process support.
Add a second tag:

  Fixes: c41a04958b09 ("net/memif: support multi-process")


[PATCH 5/6] net/memif: disconnect on device close

Warning: the commit message names the wrong trigger.
rte_eth_dev_close() rejects a started port, so close never runs
"without a preceding dev_stop" on a started device. The real leak:
memif_msg_receive_init() binds pmd->cc on a server without checking
dev_started, so a server port that is stopped or never started still
accepts a peer, and pmd->cc is live at close. Say that; it is what
stable maintainers need to judge the backport.


[PATCH 6/6] test/memif: add memif PMD unit test

Warning: add the test to MAINTAINERS under Memif PMD, as done for
the other PMD tests:

  F: app/test/test_pmd_memif.c

Warning: test_memif_close_without_stop does not cover the fix it
claims to. The throwaway server never has a peer, so pmd->cc is
NULL and memif_disconnect() skips the control channel; only the
NULL queue skip is exercised. Connect a client to the unstarted
server before closing it so ASan catches a cc leak:

	/* server: configure + queue setup, do NOT start */
	ret = rte_vdev_init("net_memif_test_tmpc",
		"role=client,id=1,socket-abstract=yes,"
		"socket=memif-autotest-tmp");
	cport = find_port("net_memif_test_tmpc");
	TEST_ASSERT(configure_port(cport) == 0, "client start failed");
	usleep(CONNECT_DELAY_US);	/* server accepts, binds cc */
	ret = rte_eth_dev_close(port);	/* server, cc live */

Warning: test_memif_setup() returns TEST_SKIPPED when the server
fails to probe. memif on an abstract socket needs no privilege or
hardware, so a probe regression would be reported as a skip. Return
TEST_FAILED there, and in test_memif_close_without_stop().

Info: the Tx fast path is never exercised. Both ports use bsize=512,
which is below the mbuf data room (2048), so eth_memif_tx() always
takes the slow path and the single-pool copy loop is untested. Add a
single-segment loopback on a pair using the default bsize.

Info: stale comments in the multiseg test. BIG_PKT_LEN says
"> default pkt_buffer_size (2048)" but the pair runs bsize=512.
WRAP_PKTS is derived from RING_SIZE, which memif ignores
(nb_tx_desc is __rte_unused); the memif ring is rsize, default
2^10. It wraps only because each packet takes 8 descriptors. Make
it explicit:

	#define MEMIF_LOG2_RING	8
	/* add ",rsize=8" to both MEMIF_*_ARGS */
	#define WRAP_PKTS	((1 << MEMIF_LOG2_RING) / 8 + 8)

Info: test_memif_send_receive checks only the first payload byte,
while the commit message says payload is verified. Check it all:

	plen = PKT_LEN - sizeof(struct rte_ether_hdr);
	for (k = 0; k < plen; k++)
		if (p[k] != (uint8_t)(got + k))
			break;


Not in this series, found while reading the Tx path:

eth_memif_tx_zc() returns uint16_t but does "return -1" when the
interrupt fd is invalid, so the caller sees 65535 packets sent after
ring->head has already been published. It is reachable:
memif_init_queues() only warns and continues with fd -1 when
eventfd() fails. Skip the kick instead, as eth_memif_tx() does:

	if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0 &&
	    rte_intr_fd_get(mq->intr_handle) >= 0) {
		uint64_t a = 1;
		ssize_t size = write(rte_intr_fd_get(mq->intr_handle),
				     &a, sizeof(a));
		...
	}

  Fixes: aedd054c5c2a ("drivers: check interrupt file descriptor validity")
  Cc: stable@dpdk.org