* [PATCH 6/6] test/memif: add memif PMD unit test
2026-10-14 9:12 [PATCH 0/6] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (4 preceding siblings ...)
2026-10-14 9:12 ` [PATCH 5/6] net/memif: disconnect on device close Sriram Yagnaraman
@ 2026-10-14 9:12 ` Sriram Yagnaraman
2026-10-15 3:09 ` [PATCH 0/6] net/memif: lifecycle fixes and " Stephen Hemminger
6 siblings, 0 replies; 9+ messages in thread
From: Sriram Yagnaraman @ 2026-10-14 9:12 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Sriram Yagnaraman, Jakub Grajciar, stable
Add a memif_pmd_autotest unit test, modelled on the TAP PMD test. memif
differs from TAP in that a port is only usable once a peer connects, so
the test creates a server and a client memif port in the same process,
connected over an abstract-namespace socket (the loopback setup from the
memif documentation), and waits for the asynchronous control-channel
handshake to bring both ports up.
The suite checks dev_info reports the memif driver, that the pair reaches
link up, statistics get and reset, a client-to-server loopback burst with
payload verification, multi-segment loopback across a ring wrap with
payload verification (general coverage for the chained-descriptor Rx and
Tx paths), that the pair reconnects after a port restart, that a
zero-length Tx burst is handled, and that a configured-but-not-started
port can be closed without crashing. It is a fast test, runs without
hugepages and under ASan.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
app/test/meson.build | 1 +
app/test/test_pmd_memif.c | 574 ++++++++++++++++++++++++++++++++++++++
2 files changed, 575 insertions(+)
create mode 100644 app/test/test_pmd_memif.c
diff --git a/app/test/meson.build b/app/test/meson.build
index 9805cf5bee..5ef1ced7ce 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -148,6 +148,7 @@ source_file_deps = {
'test_pflock.c': [],
'test_pie.c': ['sched'],
'test_pmd_af_packet.c': ['net_af_packet', 'ethdev', 'bus_vdev'],
+ 'test_pmd_memif.c': ['ethdev', 'net_memif', 'bus_vdev'],
'test_pmd_pcap.c': ['net_pcap', 'ethdev', 'bus_vdev'] + packet_burst_generator_deps,
'test_pmd_perf.c': ['ethdev', 'net'] + packet_burst_generator_deps,
'test_pmd_ring.c': ['net_ring', 'ethdev', 'bus_vdev'],
diff --git a/app/test/test_pmd_memif.c b/app/test/test_pmd_memif.c
new file mode 100644
index 0000000000..6d5e84360e
--- /dev/null
+++ b/app/test/test_pmd_memif.c
@@ -0,0 +1,574 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Ericsson AB
+ */
+
+/*
+ * Basic test of memif PMD functionality.
+ * Based off of the TAP PMD test (test_pmd_tap.c).
+ *
+ * memif connects two endpoints over shared memory. Unlike a NIC or TAP
+ * device, a memif port is only usable once its peer connects, so the test
+ * creates a server and a client memif port in this one process, connected
+ * over an abstract-namespace socket (the loopback setup documented in
+ * doc/guides/nics/memif.rst). The control-channel handshake is asynchronous,
+ * so the test waits for both ports to report link up before exercising the
+ * data path.
+ */
+
+#include "test.h"
+
+#include <stdio.h>
+
+#ifndef RTE_EXEC_ENV_LINUX
+
+/* memif PMD is only available on Linux */
+static int
+test_pmd_memif(void)
+{
+ printf("memif PMD not supported on this platform, skipping test\n");
+ return TEST_SKIPPED;
+}
+
+#else /* RTE_EXEC_ENV_LINUX */
+
+#include <string.h>
+#include <unistd.h>
+
+#include <rte_ethdev.h>
+#include <rte_bus_vdev.h>
+#include <rte_mbuf.h>
+#include <rte_ether.h>
+
+#define SOCKET0 0
+#define RING_SIZE 256
+#define NB_MBUF 4096
+#define MAX_PKT_BURST 32
+#define PKT_LEN 64
+
+/*
+ * Same socket name and id on both ports makes them a connected pair.
+ * A small bsize relative to the mbuf data room forces chained-descriptor
+ * Rx and Tx.
+ */
+#define MEMIF_SRV_NAME "net_memif_test_srv"
+#define MEMIF_CLI_NAME "net_memif_test_cli"
+#define MEMIF_SRV_ARGS "role=server,id=0,bsize=512,socket-abstract=yes,socket=memif-autotest"
+#define MEMIF_CLI_ARGS "role=client,id=0,bsize=512,socket-abstract=yes,socket=memif-autotest"
+
+/* Time to wait for the asynchronous control-channel handshake. */
+#define CONNECT_TRIES 50
+#define CONNECT_DELAY_US 100000
+
+static struct rte_mempool *mp;
+static int memif_srv_port = -1;
+static int memif_cli_port = -1;
+
+static int
+find_port(const char *frag)
+{
+ uint16_t port_id;
+ char name[RTE_ETH_NAME_MAX_LEN];
+
+ RTE_ETH_FOREACH_DEV(port_id) {
+ if (rte_eth_dev_get_name_by_port(port_id, name) == 0 &&
+ strstr(name, frag) != NULL)
+ return port_id;
+ }
+ return -1;
+}
+
+static int
+configure_port(int port)
+{
+ struct rte_eth_conf port_conf;
+ int ret;
+
+ memset(&port_conf, 0, sizeof(port_conf));
+
+ ret = rte_eth_dev_configure(port, 1, 1, &port_conf);
+ if (ret < 0) {
+ printf("Configure failed for port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ ret = rte_eth_tx_queue_setup(port, 0, RING_SIZE, SOCKET0, NULL);
+ if (ret < 0) {
+ printf("TX queue setup failed for port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ ret = rte_eth_rx_queue_setup(port, 0, RING_SIZE, SOCKET0, NULL, mp);
+ if (ret < 0) {
+ printf("RX queue setup failed for port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ ret = rte_eth_dev_start(port);
+ if (ret < 0) {
+ printf("Error starting port %d: %s\n",
+ port, rte_strerror(-ret));
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Wait until both ports report link up, i.e. the peers have connected. */
+static int
+wait_for_link(void)
+{
+ struct rte_eth_link link_srv, link_cli;
+ int i;
+
+ for (i = 0; i < CONNECT_TRIES; i++) {
+ if (rte_eth_link_get_nowait(memif_srv_port, &link_srv) == 0 &&
+ rte_eth_link_get_nowait(memif_cli_port, &link_cli) == 0 &&
+ link_srv.link_status == RTE_ETH_LINK_UP &&
+ link_cli.link_status == RTE_ETH_LINK_UP)
+ return 0;
+ usleep(CONNECT_DELAY_US);
+ }
+ return -1;
+}
+
+static struct rte_mbuf *
+create_test_packet(uint16_t pkt_len, uint8_t seed)
+{
+ struct rte_mbuf *mbuf;
+ struct rte_ether_hdr *eth_hdr;
+ uint8_t *payload;
+ uint16_t i;
+
+ mbuf = rte_pktmbuf_alloc(mp);
+ if (mbuf == NULL)
+ return NULL;
+
+ if (pkt_len < RTE_ETHER_MIN_LEN)
+ pkt_len = RTE_ETHER_MIN_LEN;
+
+ eth_hdr = (struct rte_ether_hdr *)rte_pktmbuf_append(mbuf, pkt_len);
+ if (eth_hdr == NULL) {
+ rte_pktmbuf_free(mbuf);
+ return NULL;
+ }
+
+ memset(ð_hdr->dst_addr, 0xFF, RTE_ETHER_ADDR_LEN);
+ memset(ð_hdr->src_addr, 0x02, RTE_ETHER_ADDR_LEN);
+ eth_hdr->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
+
+ payload = (uint8_t *)(eth_hdr + 1);
+ for (i = 0; i < pkt_len - sizeof(*eth_hdr); i++)
+ payload[i] = (uint8_t)(seed + i);
+
+ return mbuf;
+}
+
+static int
+test_memif_dev_info(void)
+{
+ struct rte_eth_dev_info dev_info;
+ int ret;
+
+ ret = rte_eth_dev_info_get(memif_srv_port, &dev_info);
+ TEST_ASSERT(ret == 0, "dev_info_get failed: %s", rte_strerror(-ret));
+ TEST_ASSERT(strcmp(dev_info.driver_name, "net_memif") == 0,
+ "unexpected driver name: %s", dev_info.driver_name);
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_link_up(void)
+{
+ /* The pair must have connected during setup. */
+ TEST_ASSERT(wait_for_link() == 0,
+ "memif server/client did not reach link up");
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_stats_get(void)
+{
+ struct rte_eth_stats stats;
+
+ TEST_ASSERT(rte_eth_stats_get(memif_srv_port, &stats) == 0,
+ "stats_get failed for server port");
+ TEST_ASSERT(rte_eth_stats_get(memif_cli_port, &stats) == 0,
+ "stats_get failed for client port");
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_stats_reset(void)
+{
+ struct rte_eth_stats stats;
+
+ TEST_ASSERT(rte_eth_stats_reset(memif_srv_port) == 0,
+ "stats_reset failed");
+ TEST_ASSERT(rte_eth_stats_get(memif_srv_port, &stats) == 0,
+ "stats_get after reset failed");
+ TEST_ASSERT(stats.ipackets == 0 && stats.opackets == 0 &&
+ stats.ibytes == 0 && stats.obytes == 0,
+ "stats not zero after reset");
+ return TEST_SUCCESS;
+}
+
+/*
+ * Send a burst from the client and receive it on the server, verifying the
+ * payload is delivered intact. This exercises the Rx/Tx data path over the
+ * connected memif pair.
+ */
+static int
+test_memif_send_receive(void)
+{
+ struct rte_mbuf *tx[MAX_PKT_BURST];
+ struct rte_mbuf *rx[MAX_PKT_BURST];
+ const uint16_t nb = MAX_PKT_BURST / 2;
+ uint16_t nb_tx, nb_rx, got;
+ int i, tries;
+
+ TEST_ASSERT(wait_for_link() == 0, "link not up before send/receive");
+
+ for (i = 0; i < nb; i++) {
+ tx[i] = create_test_packet(PKT_LEN, (uint8_t)i);
+ if (tx[i] == NULL) {
+ while (--i >= 0)
+ rte_pktmbuf_free(tx[i]);
+ TEST_ASSERT(0, "failed to build test packet");
+ }
+ }
+
+ nb_tx = rte_eth_tx_burst(memif_cli_port, 0, tx, nb);
+ for (i = nb_tx; i < nb; i++)
+ rte_pktmbuf_free(tx[i]);
+ TEST_ASSERT(nb_tx > 0, "client transmitted no packets");
+
+ /* Receive may take a moment; poll a few times. */
+ got = 0;
+ for (tries = 0; tries < 10 && got < nb_tx; tries++) {
+ nb_rx = rte_eth_rx_burst(memif_srv_port, 0, rx, MAX_PKT_BURST);
+ for (i = 0; i < nb_rx; i++) {
+ /* First payload byte must match the seed we sent. */
+ uint8_t *p = rte_pktmbuf_mtod_offset(rx[i], uint8_t *,
+ sizeof(struct rte_ether_hdr));
+ if (rte_pktmbuf_pkt_len(rx[i]) != PKT_LEN ||
+ *p != (uint8_t)got) {
+ uint16_t k;
+ unsigned int bad_len =
+ rte_pktmbuf_pkt_len(rx[i]);
+ uint8_t bad_p = *p;
+ for (k = i; k < nb_rx; k++)
+ rte_pktmbuf_free(rx[k]);
+ TEST_ASSERT(bad_len == PKT_LEN,
+ "received packet has wrong length %u",
+ bad_len);
+ TEST_ASSERT(0,
+ "payload mismatch: got 0x%02x expected 0x%02x",
+ bad_p, (uint8_t)got);
+ }
+ rte_pktmbuf_free(rx[i]);
+ got++;
+ }
+ if (got < nb_tx)
+ usleep(10000);
+ }
+
+ TEST_ASSERT(got == nb_tx,
+ "received %u packets, expected %u", got, nb_tx);
+ return TEST_SUCCESS;
+}
+
+static int
+test_memif_stop_start(void)
+{
+ TEST_ASSERT(rte_eth_dev_stop(memif_cli_port) == 0,
+ "stop client failed");
+ TEST_ASSERT(rte_eth_dev_start(memif_cli_port) == 0,
+ "restart client failed");
+ /* After a restart the pair must reconnect. */
+ TEST_ASSERT(wait_for_link() == 0, "link did not recover after restart");
+ return TEST_SUCCESS;
+}
+
+/*
+ * A zero-length Tx burst must be accepted and return 0 without touching
+ * the (empty) mbuf array. Regression test for the empty-burst guard in
+ * eth_memif_tx().
+ */
+static int
+test_memif_tx_empty_burst(void)
+{
+ struct rte_mbuf *dummy = NULL;
+
+ TEST_ASSERT(wait_for_link() == 0, "link not up before empty burst");
+ TEST_ASSERT(rte_eth_tx_burst(memif_cli_port, 0, &dummy, 0) == 0,
+ "zero-length Tx burst did not return 0");
+ return TEST_SUCCESS;
+}
+
+/*
+ * A port may be closed without a preceding stop; the driver must tear the
+ * control channel down rather than leak it. Regression test for the
+ * disconnect-on-close fix. Uses a throwaway device so the suite's own
+ * server/client pair is left intact.
+ */
+static int
+test_memif_close_without_stop(void)
+{
+ int port;
+ int ret;
+ struct rte_eth_conf port_conf;
+
+ ret = rte_vdev_init("net_memif_test_tmp",
+ "role=server,id=1,socket-abstract=yes,socket=memif-autotest-tmp");
+ if (ret < 0) {
+ printf("could not create throwaway memif device: %s\n",
+ rte_strerror(-ret));
+ return TEST_SKIPPED;
+ }
+
+ port = find_port("net_memif_test_tmp");
+ TEST_ASSERT(port >= 0, "throwaway memif port not found");
+
+ /*
+ * Configure (but do not start) so the device is closable: the ethdev
+ * layer rejects close on a started port. This exercises dev_close on a
+ * device that never connected, which must still tear down cleanly.
+ */
+ memset(&port_conf, 0, sizeof(port_conf));
+ ret = rte_eth_dev_configure(port, 1, 1, &port_conf);
+ if (ret < 0) {
+ rte_vdev_uninit("net_memif_test_tmp");
+ TEST_ASSERT(0, "configure of throwaway port failed: %s",
+ rte_strerror(-ret));
+ }
+
+ /* Close without ever starting; must not crash or leak. */
+ ret = rte_eth_dev_close(port);
+ TEST_ASSERT(ret == 0, "close without stop failed: %s",
+ rte_strerror(-ret));
+
+ rte_vdev_uninit("net_memif_test_tmp");
+ return TEST_SUCCESS;
+}
+
+#define BIG_PKT_LEN 4000 /* > default pkt_buffer_size (2048) => chained */
+#define WRAP_PKTS (RING_SIZE + 8) /* enough to wrap the Tx ring */
+#define BIG_SEG_LEN 1000 /* each mbuf segment carries this many bytes */
+
+/*
+ * Build a multi-segment mbuf of pkt_len bytes, chaining as many pool mbufs
+ * as needed (BIG_SEG_LEN bytes each) with a position-dependent pattern.
+ */
+static struct rte_mbuf *
+create_big_packet(uint16_t pkt_len, uint8_t seed)
+{
+ struct rte_mbuf *head = NULL, *tail = NULL;
+ uint16_t done = 0;
+
+ while (done < pkt_len) {
+ struct rte_mbuf *seg = rte_pktmbuf_alloc(mp);
+ uint16_t seg_len = RTE_MIN((uint16_t)BIG_SEG_LEN,
+ (uint16_t)(pkt_len - done));
+ uint8_t *data;
+ uint16_t i;
+
+ if (seg == NULL) {
+ rte_pktmbuf_free(head);
+ return NULL;
+ }
+ data = (uint8_t *)rte_pktmbuf_append(seg, seg_len);
+ for (i = 0; i < seg_len; i++)
+ data[i] = (uint8_t)(seed + done + i);
+
+ if (head == NULL) {
+ head = seg;
+ tail = seg;
+ } else {
+ tail->next = seg;
+ tail = seg;
+ head->nb_segs++;
+ head->pkt_len += seg_len;
+ }
+ done += seg_len;
+ }
+ return head;
+}
+
+/* Verify a received packet's payload, walking its segment chain. */
+static int
+check_big_packet(struct rte_mbuf *m, uint16_t pkt_len, uint8_t seed)
+{
+ uint16_t off = 0;
+ struct rte_mbuf *seg;
+
+ if (rte_pktmbuf_pkt_len(m) != pkt_len)
+ return -1;
+ for (seg = m; seg != NULL; seg = seg->next) {
+ const uint8_t *data = rte_pktmbuf_mtod(seg, const uint8_t *);
+ uint16_t i;
+
+ for (i = 0; i < rte_pktmbuf_data_len(seg); i++, off++)
+ if (data[i] != (uint8_t)(seed + off))
+ return -1;
+ }
+ return off == pkt_len ? 0 : -1;
+}
+
+/*
+ * Send multi-segment packets larger than the memif buffer size, repeated
+ * enough to wrap the ring, and verify each is delivered intact. Exercises
+ * the chained-descriptor Rx and Tx paths across a ring wrap.
+ */
+static int
+test_memif_multiseg(void)
+{
+ unsigned int sent = 0, recv = 0;
+ unsigned int iter;
+
+ TEST_ASSERT(wait_for_link() == 0, "link not up before multiseg test");
+
+ for (iter = 0; iter < WRAP_PKTS; iter++) {
+ struct rte_mbuf *tx, *rx[MAX_PKT_BURST];
+ uint16_t n, j;
+ int tries;
+
+ tx = create_big_packet(BIG_PKT_LEN, (uint8_t)iter);
+ TEST_ASSERT(tx != NULL, "failed to build big packet %u", iter);
+ if (tx->nb_segs <= 1) {
+ rte_pktmbuf_free(tx);
+ TEST_ASSERT(0, "test packet is not multi-segment");
+ }
+
+ if (rte_eth_tx_burst(memif_cli_port, 0, &tx, 1) != 1) {
+ rte_pktmbuf_free(tx);
+ continue;
+ }
+ sent++;
+
+ for (tries = 0; tries < 10; tries++) {
+ n = rte_eth_rx_burst(memif_srv_port, 0, rx,
+ MAX_PKT_BURST);
+ for (j = 0; j < n; j++) {
+ if (check_big_packet(rx[j], BIG_PKT_LEN,
+ (uint8_t)recv) != 0) {
+ uint16_t k;
+ for (k = j; k < n; k++)
+ rte_pktmbuf_free(rx[k]);
+ TEST_ASSERT(0,
+ "multi-seg payload corrupted at pkt %u",
+ recv);
+ }
+ rte_pktmbuf_free(rx[j]);
+ recv++;
+ }
+ if (recv >= sent)
+ break;
+ usleep(2000);
+ }
+ }
+
+ TEST_ASSERT(recv == sent,
+ "multi-seg: sent %u received %u", sent, recv);
+ return TEST_SUCCESS;
+}
+
+static void
+test_memif_cleanup(void)
+{
+ if (memif_srv_port >= 0) {
+ rte_eth_dev_stop(memif_srv_port);
+ rte_eth_dev_close(memif_srv_port);
+ }
+ if (memif_cli_port >= 0) {
+ rte_eth_dev_stop(memif_cli_port);
+ rte_eth_dev_close(memif_cli_port);
+ }
+ rte_vdev_uninit(MEMIF_SRV_NAME);
+ rte_vdev_uninit(MEMIF_CLI_NAME);
+ if (mp != NULL) {
+ rte_mempool_free(mp);
+ mp = NULL;
+ }
+ memif_srv_port = -1;
+ memif_cli_port = -1;
+}
+
+static int
+test_memif_setup(void)
+{
+ int ret;
+
+ mp = rte_pktmbuf_pool_create("memif_test_pool", NB_MBUF, 32, 0,
+ RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id());
+ if (mp == NULL) {
+ printf("Error: failed to create mempool: %s\n",
+ rte_strerror(rte_errno));
+ return -1;
+ }
+
+ ret = rte_vdev_init(MEMIF_SRV_NAME, MEMIF_SRV_ARGS);
+ if (ret < 0) {
+ printf("Error: failed to create memif server: %s\n",
+ rte_strerror(-ret));
+ rte_mempool_free(mp);
+ mp = NULL;
+ return TEST_SKIPPED;
+ }
+
+ ret = rte_vdev_init(MEMIF_CLI_NAME, MEMIF_CLI_ARGS);
+ if (ret < 0) {
+ printf("Error: failed to create memif client: %s\n",
+ rte_strerror(-ret));
+ rte_vdev_uninit(MEMIF_SRV_NAME);
+ rte_mempool_free(mp);
+ mp = NULL;
+ return -1;
+ }
+
+ memif_srv_port = find_port(MEMIF_SRV_NAME);
+ memif_cli_port = find_port(MEMIF_CLI_NAME);
+ if (memif_srv_port < 0 || memif_cli_port < 0) {
+ printf("Error: failed to find memif ports\n");
+ test_memif_cleanup();
+ return -1;
+ }
+
+ if (configure_port(memif_srv_port) < 0 ||
+ configure_port(memif_cli_port) < 0) {
+ test_memif_cleanup();
+ return -1;
+ }
+
+ return 0;
+}
+
+static struct unit_test_suite test_pmd_memif_suite = {
+ .setup = test_memif_setup,
+ .teardown = test_memif_cleanup,
+ .suite_name = "memif PMD Unit Test Suite",
+ .unit_test_cases = {
+ TEST_CASE(test_memif_dev_info),
+ TEST_CASE(test_memif_link_up),
+ TEST_CASE(test_memif_stats_get),
+ TEST_CASE(test_memif_send_receive),
+ TEST_CASE(test_memif_multiseg),
+ TEST_CASE(test_memif_stats_reset),
+ TEST_CASE(test_memif_stop_start),
+ TEST_CASE(test_memif_tx_empty_burst),
+ TEST_CASE(test_memif_close_without_stop),
+ TEST_CASES_END()
+ }
+};
+
+static int
+test_pmd_memif(void)
+{
+ return unit_test_suite_runner(&test_pmd_memif_suite);
+}
+
+#endif /* RTE_EXEC_ENV_LINUX */
+
+REGISTER_FAST_TEST(memif_pmd_autotest, NOHUGE_OK, ASAN_OK, test_pmd_memif);
--
2.43.7
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] net/memif: lifecycle fixes and unit test
2026-10-14 9:12 [PATCH 0/6] net/memif: lifecycle fixes and unit test Sriram Yagnaraman
` (5 preceding siblings ...)
2026-10-14 9:12 ` [PATCH 6/6] test/memif: add memif PMD unit test Sriram Yagnaraman
@ 2026-10-15 3:09 ` Stephen Hemminger
6 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2026-10-15 3:09 UTC (permalink / raw)
To: Sriram Yagnaraman; +Cc: dev, Jakub Grajciar, stable
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
^ permalink raw reply [flat|nested] 9+ messages in thread