All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.