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

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 15 Oct 2026 16:39:37 +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 seven 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;
>  - 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, and
> close without stop.
> 
> Verified: memif_pmd_autotest passes 10/10; 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 v1 review
> that prompted all of the v2 changes below.

More semi-automated AI review feedback

2/8 and 6/8 conflict with d9191f684c ("interrupts: add helpers to
close FD") on main, which already converted the queue fd closes in
memif_disconnect() to rte_intr_fd_close(). Rebase, keeping
rte_intr_fd_close() and the new mq == NULL checks. Resolved that way,
every commit builds with -Dwerror=true. Not run.

The v1 comments are addressed. One of them was wrong, and that error
went into 6/8 and 8/8; my mistake. Details below.


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

Warning: the reachable case is a server that was started and then
stopped, not one that was never started. memif_msg_receive_init()
skips any server with ETH_MEMIF_FLAG_DISABLED set. memif_create() sets
that flag and only memif_connect_server() clears it, at dev_start.
dev_stop never sets it again, so:

  never started:       peer rejected, pmd->cc is NULL at close
  started, stopped:    peer accepted, pmd->cc bound, and on CONNECT
                       memif_connect() sets CONNECTED and link up on
                       a stopped port

The second case is the bug. Fix it at the source in a separate patch
by refusing peers while stopped:

	static int
	memif_dev_stop(struct rte_eth_dev *dev)
	{
		...
		pmd->flags |= ETH_MEMIF_FLAG_DISABLED;
		memif_disconnect(dev);

  Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")

Keep the memif_disconnect() call in close as a backstop for an init
racing with stop, and change the commit message to "started and then
stopped".


[PATCH v2 8/8] test/memif: add memif PMD unit test

Warning: test_memif_close_without_stop() still never reaches the cc
teardown. The throwaway server is never started, so the client's init
is rejected and pmd->cc stays NULL. I suggested that setup in v1, and
it was wrong. What the test does cover is close on a configured but
never-started port, i.e. the mq == NULL skip. Reword its comment to
say so.

Once the DISABLED-on-stop fix is in, test that instead: start the
server, stop it, start a client, and check the server stays down:

	TEST_ASSERT(configure_port(sport) == 0, "server start failed");
	TEST_ASSERT(rte_eth_dev_stop(sport) == 0, "server stop failed");
	/* create and start the client as now */
	TEST_ASSERT(wait_for_link_pair(sport, cport) != 0,
		    "stopped server accepted a peer");

Without that fix, this check fails today: the stopped server reports
link up.

Info: test_memif_fastpath() checks only the length. Verify the payload
as test_memif_send_receive() now does.