DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
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.

  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