From: Stephen Hemminger <stephen@networkplumber.org>
To: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
Cc: <dev@dpdk.org>, Jakub Grajciar <jgrajcia@cisco.com>, <stable@dpdk.org>
Subject: Re: [PATCH v3 0/9] net/memif: lifecycle fixes and unit test
Date: Fri, 16 Oct 2026 10:05:23 -0700 [thread overview]
Message-ID: <20261016100523.74067d8a@phoenix.local> (raw)
In-Reply-To: <20261016141557.3081522-1-sriram.yagnaraman@ericsson.com>
On Fri, 16 Oct 2026 16:15: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; 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.
>
> 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. (v2 patch 6/7
> described this as never-started, which was wrong.)
> - Patch 7 (disconnect on close): reword to the real trigger and keep
> the close-time disconnect as a backstop.
> - 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 (the NULL-queue skip); verify the full fast-path payload.
>
> v2:
> - P1: correct Fixes tag (Tx fast path, not original driver); move the
> empty-burst check after the ring == NULL block.
> - P2: split the memseg-fd change into its own patch rather than
> widening the check to ">= 0".
> - 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; return TEST_FAILED (not SKIPPED) on probe
> failure; add a default-bsize pair to cover the Tx fast path; set
> ring size via rsize and fix the stale comments.
>
> 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 | 751 ++++++++++++++++++++++++++++++
> drivers/net/memif/memif_socket.c | 10 +-
> drivers/net/memif/rte_eth_memif.c | 76 ++-
> 5 files changed, 812 insertions(+), 27 deletions(-)
> create mode 100644 app/test/test_pmd_memif.c
>
Still see warnings from AI
v3 applies to main (26.11-rc1) and every commit builds with
-Dwerror=true. Not run. The v2 comments are addressed. 6/9 fixes the
stopped-server case at the source, and the close test now covers what
its comment says.
[PATCH v3 7/9] net/memif: disconnect on device close
Info: the commit message describes the tree before 6/9. With 6/9
applied first, dev_stop does set ETH_MEMIF_FLAG_DISABLED, so "dev_stop
does not set it again ... memif_msg_receive_init() still accepts a
peer" is no longer true at this commit. Suggest:
Before the previous patch, a server that was started and then
stopped still accepted a peer and held a live pmd->cc at close.
Call memif_disconnect() from close as a backstop for an init that
races with stop.
[PATCH v3 9/9] test/memif: add memif PMD unit test
Warning: test_memif_stopped_server_rejects_peer() passes only by
timing out, so it always burns the full wait_for_link_pair() window
(CONNECT_TRIES * CONNECT_DELAY_US = 5s). memif_pmd_autotest is a fast
test with a 10s meson timeout, and that budget also has to cover EAL
init and the other handshakes, under ASan too. Use a short window for
the negative check:
static int
wait_for_link_pair_tries(int p0, int p1, int tries)
...
#define REJECT_TRIES 10 /* 1s */
ret = wait_for_link_pair_tries(sport, cport, REJECT_TRIES);
Info: the commit message does not list the fast-path loopback or the
stopped-server test.
next prev parent reply other threads:[~2026-10-16 17:05 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-16 14:15 [PATCH v3 0/9] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 1/9] net/memif: guard against empty Tx burst Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 2/9] net/memif: close file descriptor zero Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 3/9] net/memif: do not close EAL-owned memseg file descriptor Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 4/9] net/memif: fix MAC address leak on probe failure Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 5/9] net/memif: unwind partial device on create failure Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 6/9] net/memif: refuse connection on stopped device Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 7/9] net/memif: disconnect on device close Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 8/9] net/memif: fix bogus Tx count on invalid interrupt FD Sriram Yagnaraman
2026-10-16 14:15 ` [PATCH v3 9/9] test/memif: add memif PMD unit test Sriram Yagnaraman
2026-10-16 17:05 ` Stephen Hemminger [this message]
2026-10-16 19:59 ` [PATCH v3 0/9] net/memif: lifecycle fixes and " Sriram Yagnaraman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261016100523.74067d8a@phoenix.local \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=jgrajcia@cisco.com \
--cc=sriram.yagnaraman@ericsson.com \
--cc=stable@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox