DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [DPDK/other Bug 952] unit tests fail when machine has more than 128 cores
From: bugzilla @ 2026-06-16 12:34 UTC (permalink / raw)
  To: dev
In-Reply-To: <bug-952-3@http.bugs.dpdk.org/>

http://bugs.dpdk.org/show_bug.cgi?id=952

Thomas Monjalon (thomas@monjalon.net) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #1 from Thomas Monjalon (thomas@monjalon.net) ---
Resolved in https://dpdk.org/id/c9eb695f16
test/atomic: scale test based on core count

-- 
You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* Re: [RFC 0/4] alternative capture mechanism
From: Bruce Richardson @ 2026-06-16 12:37 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev
In-Reply-To: <20260609210540.768074-1-stephen@networkplumber.org>

On Tue, Jun 09, 2026 at 02:02:01PM -0700, Stephen Hemminger wrote:
> This is an RFC for an alternative way to capture packets from a DPDK
> application. I did brief demo of similar mechanism at DPDK summit but
> this is more complete. Capture runs in the primary process and is driven
> entirely over telemetry; no secondary process is involved.
> 
> A client asks the application to start capturing and passes it a file
> descriptor to write to. The application writes pcapng to that descriptor.
> A Wireshark extcap script is the intended front end, but the control path
> is just telemetry and the output is just a pipe, so other front ends are
> possible.
> 
>   1/4  telemetry: let a command receive file descriptors from the client
>   2/4  capture: the library
>   3/4  test: functional test
>   4/4  app: the Wireshark extcap script and its documentation
> 
> Setup and usage are in doc/guides/tools/wireshark_extcap.rst.
> 
> Primary process only for now; secondary-process capture is possible as
> follow-on. Posting as RFC to get feedback on the approach.
> 
> The extcap script is dual licensed (BSD-3-Clause OR GPL-2.0-or-later) as
> it may be more useful in the Wireshark tree.
> 

One concern I have though - does this cause system-calls to be made in the
fast-path because we are writting to a passed in FD? For performance
reasons, would it not be better to use a memory buffer for this, thereby
avoiding syscalls? For example, rather than passing in an FD to telemetry,
we could pass in a key to be passed to shmget (going old-school!), or
name parameter for shm_open. Thereafter with the memory buffer we can use a
circular ring or similar to pass the data from app to client.

/Bruce

> Stephen Hemminger (4):
>   telemetry: allow commands to receive file descriptors
>   capture: infrastructure wireshark packet capture
>   test: add test for capture hooks
>   usertools/dpdk-wireshark-extcap.py: script for external capture
> 
>  MAINTAINERS                            |   4 +
>  app/test/meson.build                   |   1 +
>  app/test/test_capture.c                | 365 +++++++++++
>  doc/guides/rel_notes/release_26_07.rst |  12 +
>  doc/guides/tools/index.rst             |   1 +
>  doc/guides/tools/wireshark_extcap.rst  | 155 +++++
>  lib/capture/capture.c                  | 821 +++++++++++++++++++++++++
>  lib/capture/capture_impl.h             |  56 ++
>  lib/capture/filter.c                   | 108 ++++
>  lib/capture/meson.build                |  19 +
>  lib/meson.build                        |   1 +
>  lib/telemetry/rte_telemetry.h          |  66 ++
>  lib/telemetry/telemetry.c              | 115 +++-
>  usertools/dpdk-wireshark-extcap.py     | 274 +++++++++
>  14 files changed, 1986 insertions(+), 12 deletions(-)
>  create mode 100644 app/test/test_capture.c
>  create mode 100644 doc/guides/tools/wireshark_extcap.rst
>  create mode 100644 lib/capture/capture.c
>  create mode 100644 lib/capture/capture_impl.h
>  create mode 100644 lib/capture/filter.c
>  create mode 100644 lib/capture/meson.build
>  create mode 100755 usertools/dpdk-wireshark-extcap.py
> 
> -- 
> 2.53.0
> 

^ permalink raw reply

* [v4] net/cksum: compute raw cksum for several segments
From: Su Sai @ 2026-06-16 12:38 UTC (permalink / raw)
  To: stephen; +Cc: dev, spiderdetective.ss
In-Reply-To: <20260608100202.0deac83d@phoenix.local>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 7012 bytes --]

The rte_raw_cksum_mbuf function is used to compute
the raw checksum of a packet.
If the packet payload stored in multi mbuf, the function
will goto the hard case. In hard case,
the variable 'tmp' is a type of uint32_t,
so rte_bswap16 will drop high 16 bit.
Meanwhile, the variable 'sum' is a type of uint32_t,
so 'sum += tmp' will drop the carry when overflow.
Both drop will make cksum incorrect.
This commit fixes the above bug.

Signed-off-by: Su Sai <spiderdetective.ss@gmail.com>
---
 .mailmap              |   1 +
 app/test/test_cksum.c | 102 ++++++++++++++++++++++++++++++++++++++++++
 lib/net/rte_cksum.h   |  27 +++++++++--
 3 files changed, 126 insertions(+), 4 deletions(-)

diff --git a/.mailmap b/.mailmap
index 4001e5fb0e..bcf73cb902 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1630,6 +1630,7 @@ Sylvia Grundwürmer <sylvia.grundwuermer@b-plus.com>
 Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
 Sylwia Wnuczko <sylwia.wnuczko@intel.com>
 Szymon Sliwa <szs@semihalf.com>
+Su Sai <spiderdetective.ss@gmail.com> <susai.ss@bytedance.com>
 Szymon T Cudzilo <szymon.t.cudzilo@intel.com>
 Tadhg Kearney <tadhg.kearney@intel.com>
 Taekyung Kim <kim.tae.kyung@navercorp.com>
diff --git a/app/test/test_cksum.c b/app/test/test_cksum.c
index ea443382a1..5bd9723fbd 100644
--- a/app/test/test_cksum.c
+++ b/app/test/test_cksum.c
@@ -85,6 +85,42 @@ static const char test_cksum_ipv4_opts_udp[] = {
 	0x00, 0x35, 0x00, 0x09, 0x89, 0x6f, 0x78,
 };
 
+/*
+ * generated in scapy with
+ * Ether()/IP()/TCP(options=[NOP,NOP,Timestamps])/os.urandom(113))
+ */
+static const char test_cksum_ipv4_tcp_multi_segs[] = {
+	0x00, 0x16, 0x3e, 0x0b, 0x6b, 0xd2, 0xee, 0xff,
+	0xff, 0xff, 0xff, 0xff, 0x08, 0x00, 0x45, 0x00,
+	0x00, 0xa5, 0x46, 0x10, 0x40, 0x00, 0x40, 0x06,
+	0x80, 0xb5, 0xc0, 0xa8, 0xf9, 0x1d, 0xc0, 0xa8,
+	0xf9, 0x1e, 0xdc, 0xa2, 0x14, 0x51, 0xbb, 0x8f,
+	0xa0, 0x00, 0xe4, 0x7c, 0xe4, 0xb8, 0x80, 0x10,
+	0x02, 0x00, 0x4b, 0xc1, 0x00, 0x00, 0x01, 0x01,
+	0x08, 0x0a, 0x90, 0x60, 0xf4, 0xff, 0x03, 0xc5,
+	0xb4, 0x19, 0x77, 0x34, 0xd4, 0xdc, 0x84, 0x86,
+	0xff, 0x44, 0x09, 0x63, 0x36, 0x2e, 0x26, 0x9b,
+	0x90, 0x70, 0xf2, 0xed, 0xc8, 0x5b, 0x87, 0xaa,
+	0xb4, 0x67, 0x6b, 0x32, 0x3d, 0xc4, 0xbf, 0x15,
+	0xa9, 0x16, 0x6c, 0x2a, 0x9d, 0xb2, 0xb7, 0x6b,
+	0x58, 0x44, 0x58, 0x12, 0x4b, 0x8f, 0xe5, 0x12,
+	0x11, 0x90, 0x94, 0x68, 0x37, 0xad, 0x0a, 0x9b,
+	0xd6, 0x79, 0xf2, 0xb7, 0x31, 0xcf, 0x44, 0x22,
+	0xc8, 0x99, 0x3f, 0xe5, 0xe7, 0xac, 0xc7, 0x0b,
+	0x86, 0xdf, 0xda, 0xed, 0x0a, 0x0f, 0x86, 0xd7,
+	0x48, 0xe2, 0xf1, 0xc2, 0x43, 0xed, 0x47, 0x3a,
+	0xea, 0x25, 0x2d, 0xd6, 0x60, 0x38, 0x30, 0x07,
+	0x28, 0xdd, 0x1f, 0x0c, 0xdd, 0x7b, 0x7c, 0xd9,
+	0x35, 0x9d, 0x14, 0xaa, 0xc6, 0x35, 0xd1, 0x03,
+	0x38, 0xb1, 0xf5,
+};
+
+static const uint8_t test_cksum_ipv4_tcp_multi_segs_len[] = {
+	66,  /* the first seg contains all headers, including L2 to L4 */
+	61,  /* the second seg length is odd, test byte order independent */
+	52,  /* three segs are sufficient to test the most complex scenarios */
+};
+
 /* test l3/l4 checksum api */
 static int
 test_l4_cksum(struct rte_mempool *pktmbuf_pool, const char *pktdata, size_t len)
@@ -223,6 +259,66 @@ test_l4_cksum(struct rte_mempool *pktmbuf_pool, const char *pktdata, size_t len)
 	return -1;
 }
 
+/* test l4 checksum api for a packet with multiple mbufs */
+static int
+test_l4_cksum_multi_mbufs(struct rte_mempool *pktmbuf_pool, const char *pktdata, size_t len,
+			     const uint8_t *segs, size_t segs_len)
+{
+	struct rte_mbuf *m[NB_MBUF] = {0};
+	struct rte_mbuf *m_hdr = NULL;
+	struct rte_net_hdr_lens hdr_lens;
+	size_t i, off = 0;
+	uint32_t packet_type, l3;
+	void *l3_hdr;
+	char *data;
+
+	for (i = 0; i < segs_len; i++) {
+		m[i] = rte_pktmbuf_alloc(pktmbuf_pool);
+		if (m[i] == NULL)
+			GOTO_FAIL("Cannot allocate mbuf");
+
+		data = rte_pktmbuf_append(m[i], segs[i]);
+		if (data == NULL)
+			GOTO_FAIL("Cannot append data");
+
+		memcpy(data, pktdata + off, segs[i]);
+		off += segs[i];
+
+		if (m_hdr) {
+			if (rte_pktmbuf_chain(m_hdr, m[i]))
+				GOTO_FAIL("Cannot chain mbuf");
+		} else {
+			m_hdr = m[i];
+		}
+	}
+
+	if (off != len)
+		GOTO_FAIL("Invalid segs");
+
+	packet_type = rte_net_get_ptype(m_hdr, &hdr_lens, RTE_PTYPE_ALL_MASK);
+	l3 = packet_type & RTE_PTYPE_L3_MASK;
+
+	l3_hdr = rte_pktmbuf_mtod_offset(m_hdr, void *, hdr_lens.l2_len);
+	off = hdr_lens.l2_len + hdr_lens.l3_len;
+
+	if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
+		if (rte_ipv4_udptcp_cksum_mbuf_verify(m_hdr, l3_hdr, off) != 0)
+			GOTO_FAIL("Invalid L4 checksum verification for multiple mbufs");
+	} else if (l3 == RTE_PTYPE_L3_IPV6 || l3 == RTE_PTYPE_L3_IPV6_EXT) {
+		if (rte_ipv6_udptcp_cksum_mbuf_verify(m_hdr, l3_hdr, off) != 0)
+			GOTO_FAIL("Invalid L4 checksum verification for multiple mbufs");
+	}
+
+	rte_pktmbuf_free_bulk(m, segs_len);
+
+	return 0;
+
+fail:
+	rte_pktmbuf_free_bulk(m, segs_len);
+
+	return -1;
+}
+
 static int
 test_cksum(void)
 {
@@ -256,6 +352,12 @@ test_cksum(void)
 			  sizeof(test_cksum_ipv4_opts_udp)) < 0)
 		GOTO_FAIL("checksum error on ipv4_opts_udp");
 
+	if (test_l4_cksum_multi_mbufs(pktmbuf_pool, test_cksum_ipv4_tcp_multi_segs,
+			  sizeof(test_cksum_ipv4_tcp_multi_segs),
+			  test_cksum_ipv4_tcp_multi_segs_len,
+			  sizeof(test_cksum_ipv4_tcp_multi_segs_len)) < 0)
+		GOTO_FAIL("checksum error on multi mbufs check");
+
 	rte_mempool_free(pktmbuf_pool);
 
 	return 0;
diff --git a/lib/net/rte_cksum.h b/lib/net/rte_cksum.h
index a8e8927952..679ba82eb6 100644
--- a/lib/net/rte_cksum.h
+++ b/lib/net/rte_cksum.h
@@ -80,6 +80,25 @@ __rte_raw_cksum_reduce(uint32_t sum)
 	return (uint16_t)sum;
 }
 
+/**
+ * @internal Reduce a sum to the non-complemented checksum.
+ * Helper routine for the rte_raw_cksum_mbuf().
+ *
+ * @param sum
+ *   Value of the sum.
+ * @return
+ *   The non-complemented checksum.
+ */
+static inline uint16_t
+__rte_raw_cksum_reduce_u64(uint64_t sum)
+{
+	uint32_t tmp;
+
+	tmp = __rte_raw_cksum_reduce((uint32_t)sum);
+	tmp += __rte_raw_cksum_reduce((uint32_t)(sum >> 32));
+	return __rte_raw_cksum_reduce(tmp);
+}
+
 /**
  * Process the non-complemented checksum of a buffer.
  *
@@ -119,8 +138,8 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len,
 {
 	const struct rte_mbuf *seg;
 	const char *buf;
-	uint32_t sum, tmp;
-	uint32_t seglen, done;
+	uint32_t seglen, done, tmp;
+	uint64_t sum;
 
 	/* easy case: all data in the first segment */
 	if (off + len <= rte_pktmbuf_data_len(m)) {
@@ -157,7 +176,7 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len,
 	for (;;) {
 		tmp = __rte_raw_cksum(buf, seglen, 0);
 		if (done & 1)
-			tmp = rte_bswap16((uint16_t)tmp);
+			tmp = rte_bswap32(tmp);
 		sum += tmp;
 		done += seglen;
 		if (done == len)
@@ -169,7 +188,7 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len,
 			seglen = len - done;
 	}
 
-	*cksum = __rte_raw_cksum_reduce(sum);
+	*cksum = __rte_raw_cksum_reduce_u64(sum);
 	return 0;
 }
 
-- 
2.20.1


^ permalink raw reply related

* Re: [v3] net/cksum: compute raw cksum for several segments
From: su sai @ 2026-06-16 12:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev
In-Reply-To: <20260608100202.0deac83d@phoenix.local>

[-- Attachment #1: Type: text/plain, Size: 9570 bytes --]

Hi Stephen, I've revised the patch per your feedback and sent out v4:
net/cksum: compute raw cksum for several segments. Your review is
appreciated.

On Tue, Jun 9, 2026 at 1:02 AM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Mon,  4 Aug 2025 11:54:30 +0800
> Su Sai <spiderdetective.ss@gmail.com> wrote:
>
> > The rte_raw_cksum_mbuf function is used to compute
> > the raw checksum of a packet.
> > If the packet payload stored in multi mbuf, the function
> > will goto the hard case. In hard case,
> > the variable 'tmp' is a type of uint32_t,
> > so rte_bswap16 will drop high 16 bit.
> > Meanwhile, the variable 'sum' is a type of uint32_t,
> > so 'sum += tmp' will drop the carry when overflow.
> > Both drop will make cksum incorrect.
> > This commit fixes the above bug.
> >
> > Signed-off-by: Su Sai <spiderdetective.ss@gmail.com>
> > ---
> >  .mailmap              |   1 +
> >  app/test/test_cksum.c | 106 ++++++++++++++++++++++++++++++++++++++++++
> >  lib/net/rte_cksum.h   |  27 +++++++++--
> >  3 files changed, 130 insertions(+), 4 deletions(-)
> >
> > diff --git a/.mailmap b/.mailmap
> > index 34a99f93a1..1da1d9f8e1 100644
> > --- a/.mailmap
> > +++ b/.mailmap
> > @@ -1552,6 +1552,7 @@ Sunil Kumar Kori <skori@marvell.com> <
> sunil.kori@nxp.com>
> >  Sunil Pai G <sunil.pai.g@intel.com>
> >  Sunil Uttarwar <sunilprakashrao.uttarwar@amd.com>
> >  Sun Jiajia <sunx.jiajia@intel.com>
> > +Su Sai <spiderdetective.ss@gmail.com> <susai.ss@bytedance.com>
> >  Sunyang Wu <sunyang.wu@jaguarmicro.com>
> >  Surabhi Boob <surabhi.boob@intel.com>
> >  Suyang Ju <sju@paloaltonetworks.com>
> > diff --git a/app/test/test_cksum.c b/app/test/test_cksum.c
> > index f2ab5af5a7..fb2e3cf9e6 100644
> > --- a/app/test/test_cksum.c
> > +++ b/app/test/test_cksum.c
> > @@ -85,6 +85,42 @@ static const char test_cksum_ipv4_opts_udp[] = {
> >       0x00, 0x35, 0x00, 0x09, 0x89, 0x6f, 0x78,
> >  };
> >
> > +/*
> > + * generated in scapy with
> > + * Ether()/IP()/TCP(options=[NOP,NOP,Timestamps])/os.urandom(113))
> > + */
> > +static const char test_cksum_ipv4_tcp_multi_segs[] = {
> > +     0x00, 0x16, 0x3e, 0x0b, 0x6b, 0xd2, 0xee, 0xff,
> > +     0xff, 0xff, 0xff, 0xff, 0x08, 0x00, 0x45, 0x00,
> > +     0x00, 0xa5, 0x46, 0x10, 0x40, 0x00, 0x40, 0x06,
> > +     0x80, 0xb5, 0xc0, 0xa8, 0xf9, 0x1d, 0xc0, 0xa8,
> > +     0xf9, 0x1e, 0xdc, 0xa2, 0x14, 0x51, 0xbb, 0x8f,
> > +     0xa0, 0x00, 0xe4, 0x7c, 0xe4, 0xb8, 0x80, 0x10,
> > +     0x02, 0x00, 0x4b, 0xc1, 0x00, 0x00, 0x01, 0x01,
> > +     0x08, 0x0a, 0x90, 0x60, 0xf4, 0xff, 0x03, 0xc5,
> > +     0xb4, 0x19, 0x77, 0x34, 0xd4, 0xdc, 0x84, 0x86,
> > +     0xff, 0x44, 0x09, 0x63, 0x36, 0x2e, 0x26, 0x9b,
> > +     0x90, 0x70, 0xf2, 0xed, 0xc8, 0x5b, 0x87, 0xaa,
> > +     0xb4, 0x67, 0x6b, 0x32, 0x3d, 0xc4, 0xbf, 0x15,
> > +     0xa9, 0x16, 0x6c, 0x2a, 0x9d, 0xb2, 0xb7, 0x6b,
> > +     0x58, 0x44, 0x58, 0x12, 0x4b, 0x8f, 0xe5, 0x12,
> > +     0x11, 0x90, 0x94, 0x68, 0x37, 0xad, 0x0a, 0x9b,
> > +     0xd6, 0x79, 0xf2, 0xb7, 0x31, 0xcf, 0x44, 0x22,
> > +     0xc8, 0x99, 0x3f, 0xe5, 0xe7, 0xac, 0xc7, 0x0b,
> > +     0x86, 0xdf, 0xda, 0xed, 0x0a, 0x0f, 0x86, 0xd7,
> > +     0x48, 0xe2, 0xf1, 0xc2, 0x43, 0xed, 0x47, 0x3a,
> > +     0xea, 0x25, 0x2d, 0xd6, 0x60, 0x38, 0x30, 0x07,
> > +     0x28, 0xdd, 0x1f, 0x0c, 0xdd, 0x7b, 0x7c, 0xd9,
> > +     0x35, 0x9d, 0x14, 0xaa, 0xc6, 0x35, 0xd1, 0x03,
> > +     0x38, 0xb1, 0xf5,
> > +};
> > +
> > +static const uint8_t test_cksum_ipv4_tcp_multi_segs_len[] = {
> > +     66,  /* the first seg contains all headers, including L2 to L4 */
> > +     61,  /* the second seg length is odd, test byte order independent
> */
> > +     52,  /* three segs are sufficient to test the most complex
> scenarios */
> > +};
> > +
> >  /* test l3/l4 checksum api */
> >  static int
> >  test_l4_cksum(struct rte_mempool *pktmbuf_pool, const char *pktdata,
> size_t len)
> > @@ -223,6 +259,70 @@ test_l4_cksum(struct rte_mempool *pktmbuf_pool,
> const char *pktdata, size_t len)
> >       return -1;
> >  }
> >
> > +/* test l4 checksum api for a packet with multiple mbufs */
> > +static int
> > +test_l4_cksum_multi_mbufs(struct rte_mempool *pktmbuf_pool, const char
> *pktdata, size_t len,
> > +                          const uint8_t *segs, size_t segs_len)
> > +{
> > +     struct rte_mbuf *m[NB_MBUF] = {0};
> > +     struct rte_mbuf *m_hdr = NULL;
> > +     struct rte_net_hdr_lens hdr_lens;
> > +     size_t i, off = 0;
> > +     uint32_t packet_type, l3;
> > +     void *l3_hdr;
> > +     char *data;
> > +
> > +     for (i = 0; i < segs_len; i++) {
> > +             m[i] = rte_pktmbuf_alloc(pktmbuf_pool);
> > +             if (m[i] == NULL)
> > +                     GOTO_FAIL("Cannot allocate mbuf");
> > +
> > +             data = rte_pktmbuf_append(m[i], segs[i]);
> > +             if (data == NULL)
> > +                     GOTO_FAIL("Cannot append data");
> > +
> > +             rte_memcpy(data, pktdata + off, segs[i]);
>
> Tests (except rte_memcpy test) should not use rte_memcpy, instead use
> regular memcpy which has better coverage from analyzers.
>
> > +             off += segs[i];
> > +
> > +             if (m_hdr) {
> > +                     if (rte_pktmbuf_chain(m_hdr, m[i]))
> > +                             GOTO_FAIL("Cannot chain mbuf");
> > +             } else {
> > +                     m_hdr = m[i];
> > +             }
> > +     }
> > +
> > +     if (off != len)
> > +             GOTO_FAIL("Invalid segs");
> > +
> > +     packet_type = rte_net_get_ptype(m_hdr, &hdr_lens,
> RTE_PTYPE_ALL_MASK);
> > +     l3 = packet_type & RTE_PTYPE_L3_MASK;
> > +
> > +     l3_hdr = rte_pktmbuf_mtod_offset(m_hdr, void *, hdr_lens.l2_len);
> > +     off = hdr_lens.l2_len + hdr_lens.l3_len;
> > +
> > +     if (l3 == RTE_PTYPE_L3_IPV4 || l3 == RTE_PTYPE_L3_IPV4_EXT) {
> > +             if (rte_ipv4_udptcp_cksum_mbuf_verify(m_hdr, l3_hdr, off)
> != 0)
> > +                     GOTO_FAIL("Invalid L4 checksum verification for
> multiple mbufs");
> > +     } else if (l3 == RTE_PTYPE_L3_IPV6 || l3 == RTE_PTYPE_L3_IPV6_EXT)
> {
> > +             if (rte_ipv6_udptcp_cksum_mbuf_verify(m_hdr, l3_hdr, off)
> != 0)
> > +                     GOTO_FAIL("Invalid L4 checksum verification for
> multiple mbufs");
> > +     }
> > +
> > +     for (i = 0; i < segs_len; i++)
> > +             rte_pktmbuf_free(m[i]);
>
> Can avoid the loop here and elsewhere by using rte_pktmbuf_free_bulk()
>
> > +     return 0;
> > +
> > +fail:
> > +     for (i = 0; i < segs_len; i++) {
> > +             if (m[i])
> > +                     rte_pktmbuf_free(m[i]);
> > +     }
>
> Freebulk will work here
>
> > +     return -1;
> > +}
> > +
> >  static int
> >  test_cksum(void)
> >  {
> > @@ -256,6 +356,12 @@ test_cksum(void)
> >                         sizeof(test_cksum_ipv4_opts_udp)) < 0)
> >               GOTO_FAIL("checksum error on ipv4_opts_udp");
> >
> > +     if (test_l4_cksum_multi_mbufs(pktmbuf_pool,
> test_cksum_ipv4_tcp_multi_segs,
> > +                       sizeof(test_cksum_ipv4_tcp_multi_segs),
> > +                       test_cksum_ipv4_tcp_multi_segs_len,
> > +                       sizeof(test_cksum_ipv4_tcp_multi_segs_len)) < 0)
> > +             GOTO_FAIL("checksum error on multi mbufs check");
> > +
> >       rte_mempool_free(pktmbuf_pool);
> >
> >       return 0;
> > diff --git a/lib/net/rte_cksum.h b/lib/net/rte_cksum.h
> > index a8e8927952..679ba82eb6 100644
> > --- a/lib/net/rte_cksum.h
> > +++ b/lib/net/rte_cksum.h
> > @@ -80,6 +80,25 @@ __rte_raw_cksum_reduce(uint32_t sum)
> >       return (uint16_t)sum;
> >  }
> >
> > +/**
> > + * @internal Reduce a sum to the non-complemented checksum.
> > + * Helper routine for the rte_raw_cksum_mbuf().
> > + *
> > + * @param sum
> > + *   Value of the sum.
> > + * @return
> > + *   The non-complemented checksum.
> > + */
> > +static inline uint16_t
> > +__rte_raw_cksum_reduce_u64(uint64_t sum)
> > +{
> > +     uint32_t tmp;
> > +
> > +     tmp = __rte_raw_cksum_reduce((uint32_t)sum);
> > +     tmp += __rte_raw_cksum_reduce((uint32_t)(sum >> 32));
> > +     return __rte_raw_cksum_reduce(tmp);
> > +}
> > +
> >  /**
> >   * Process the non-complemented checksum of a buffer.
> >   *
> > @@ -119,8 +138,8 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m,
> uint32_t off, uint32_t len,
> >  {
> >       const struct rte_mbuf *seg;
> >       const char *buf;
> > -     uint32_t sum, tmp;
> > -     uint32_t seglen, done;
> > +     uint32_t seglen, done, tmp;
> > +     uint64_t sum;
> >
> >       /* easy case: all data in the first segment */
> >       if (off + len <= rte_pktmbuf_data_len(m)) {
> > @@ -157,7 +176,7 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m,
> uint32_t off, uint32_t len,
> >       for (;;) {
> >               tmp = __rte_raw_cksum(buf, seglen, 0);
> >               if (done & 1)
> > -                     tmp = rte_bswap16((uint16_t)tmp);
> > +                     tmp = rte_bswap32(tmp);
> >               sum += tmp;
> >               done += seglen;
> >               if (done == len)
> > @@ -169,7 +188,7 @@ rte_raw_cksum_mbuf(const struct rte_mbuf *m,
> uint32_t off, uint32_t len,
> >                       seglen = len - done;
> >       }
> >
> > -     *cksum = __rte_raw_cksum_reduce(sum);
> > +     *cksum = __rte_raw_cksum_reduce_u64(sum);
> >       return 0;
> >  }
> >
>
>

[-- Attachment #2: Type: text/html, Size: 12542 bytes --]

^ permalink raw reply

* [PATCH v2] app/testpmd: add VLAN priority insert support
From: Xingui Yang @ 2026-06-16 13:10 UTC (permalink / raw)
  To: dev
  Cc: stephen, david.marchand, aman.deep.singh, fengchengwen,
	yangshuaisong, lihuisong, liuyonglong, kangfenglong
In-Reply-To: <20260612081411.2798403-1-yangxingui@huawei.com>

The tx_vlan set and tx_qinq set commands only accepted VLAN ID in range
[0, 4095]. This prevented users from setting 802.1p priority and CFI
bits when using hardware VLAN insertion.

Since mbuf vlan_tci field already supports full 16-bit VLAN Tag Control
Information (TCI), relax the validation for TX paths to allow priority
and CFI bits. The vlan_id parameter now accepts:
  - Bits 0-11:  VLAN ID (0-4095)
  - Bit 12:    CFI (Canonical Format Indicator)
  - Bits 13-15: Priority (0-7, 802.1p CoS)

Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Suggested-by: fengchengwen <fengchengwen@huawei.com>
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
v2:
- Removed --enable-vlan-priority option and global variable as suggested
  by Stephen Hemminger. The feature is now always enabled for TX paths
- RX VLAN filter continues to enforce strict VLAN ID validation as
  suggested by fengchengwen
- Added documentation updates for testpmd_funcs.rst and release notes

 app/test-pmd/config.c                       | 13 ++++++++-----
 doc/guides/rel_notes/release_26_07.rst      |  7 +++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 17 ++++++++++++++---
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 9d457ca88e..38758f9c05 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1241,8 +1241,11 @@ void print_valid_ports(void)
 }
 
 static int
-vlan_id_is_invalid(uint16_t vlan_id)
+vlan_id_is_invalid(uint16_t vlan_id, bool is_tx)
 {
+	if (is_tx)
+		return 0;
+
 	if (vlan_id < 4096)
 		return 0;
 	fprintf(stderr, "Invalid vlan_id %d (must be < 4096)\n", vlan_id);
@@ -6876,7 +6879,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)
 
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return 1;
-	if (vlan_id_is_invalid(vlan_id))
+	if (vlan_id_is_invalid(vlan_id, false))
 		return 1;
 	diag = rte_eth_dev_vlan_filter(port_id, vlan_id, on);
 	if (diag == 0)
@@ -6923,7 +6926,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)
 	struct rte_eth_dev_info dev_info;
 	int ret;
 
-	if (vlan_id_is_invalid(vlan_id))
+	if (vlan_id_is_invalid(vlan_id, true))
 		return;
 
 	if (ports[port_id].dev_conf.txmode.offloads &
@@ -6954,9 +6957,9 @@ tx_qinq_set(portid_t port_id, uint16_t vlan_id, uint16_t vlan_id_outer)
 	struct rte_eth_dev_info dev_info;
 	int ret;
 
-	if (vlan_id_is_invalid(vlan_id))
+	if (vlan_id_is_invalid(vlan_id, true))
 		return;
-	if (vlan_id_is_invalid(vlan_id_outer))
+	if (vlan_id_is_invalid(vlan_id_outer, true))
 		return;
 
 	ret = eth_dev_info_get_print_err(port_id, &dev_info);
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 5d7aa8d1bf..e382c7f407 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -150,6 +150,13 @@ New Features
   * Added ``eof`` devarg to use link state to signal end of receive file input.
   * Added unit test suite.
 
+* **Added VLAN priority support in testpmd.**
+
+  Added support for setting VLAN priority and CFI bits in ``tx_vlan set``
+  and ``tx_qinq set`` commands. The ``vlan_tci`` parameter now accepts the
+  full 16-bit VLAN Tag Control Information (TCI) format, which includes
+  priority (bits 13-15), CFI (bit 12), and VLAN ID (bits 0-11).
+
 * **Added AI review helpers.**
 
   Added AGENTS.md file for AI review
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f0f2b0758b..b967810b10 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1120,15 +1120,26 @@ tx_vlan set
 
 Set hardware insertion of VLAN IDs in packets sent on a port::
 
-   testpmd> tx_vlan set (port_id) vlan_id[, vlan_id_outer]
+   testpmd> tx_vlan set (port_id) vlan_tci[, vlan_tci_outer]
+
+The ``vlan_tci`` parameter accepts the full 16-bit VLAN Tag Control Information (TCI)
+format, which includes:
+
+* Bits 0-11:  VLAN ID (0-4095)
+* Bit 12:    CFI (Canonical Format Indicator)
+* Bits 13-15: Priority (0-7, 802.1p CoS)
 
 For example, set a single VLAN ID (5) insertion on port 0::
 
-   tx_vlan set 0 5
+   testpmd> tx_vlan set 0 5
+
+Or, set a VLAN ID with priority (priority=3, VLAN ID=6) insertion on port 0::
+
+   testpmd> tx_vlan set 0 0x6006
 
 Or, set double VLAN ID (inner: 2, outer: 3) insertion on port 1::
 
-   tx_vlan set 1 2 3
+   testpmd> tx_vlan set 1 2 3
 
 
 tx_vlan set pvid
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH] app/testpmd: add VLAN priority insert support
From: yangxingui @ 2026-06-16 13:17 UTC (permalink / raw)
  To: fengchengwen, dev
  Cc: stephen, david.marchand, aman.deep.singh, yangshuaisong,
	lihuisong, liuyonglong, kangfenglong
In-Reply-To: <7ead3834-afaa-4319-83bf-faf19b3ea3ef@huawei.com>



On 2026/6/15 17:46, fengchengwen wrote:
> On 6/12/2026 4:14 PM, Xingui Yang wrote:
>> The tx_vlan set command currently only accepts a VLAN ID in range
>> [0, 4095].  This patch adds support for an extended format that includes
>> 802.1p priority and CFI bits, allowing users to set the VLAN priority
>> tag when inserting VLAN headers in TX packets.
>>
>> The extended format is:
>>    bit 0-11:  VLAN ID (0-4095)
>>    bit 12:    CFI (Canonical Format Indicator)
>>    bit 13-15: Priority (0-7, 802.1p CoS)
>>
>> This is consistent with the VLAN tag structure used by
>> rte_eth_dev_set_vlan_pvid() where the PVID field encodes VLAN ID, CFI
>> and priority in the same format.
>>
>> A new command line option --enable-vlan-priority is added to enable this
>> feature. By default, the feature is disabled to maintain backward
>> compatibility with existing users. When enabled, the
>> vlan_id_is_invalid() function allows any 16-bit value to pass, while the
>> full 16-bit value (including CFI and priority bits) is passed to the
>> driver for hardware VLAN insertion.
>>
>> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
>> ---
>>   app/test-pmd/config.c     | 24 +++++++++++++++---------
>>   app/test-pmd/parameters.c |  6 ++++++
>>   app/test-pmd/testpmd.c    |  5 +++++
>>   app/test-pmd/testpmd.h    |  2 ++
>>   4 files changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 36b9b023e2..80cde109e6 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -1241,12 +1241,18 @@ void print_valid_ports(void)
>>   }
>>   
>>   static int
>> -vlan_id_is_invalid(uint16_t vlan_id)
>> +vlan_id_is_invalid(uint16_t vlan_id, int vlan_priority_ena)
>>   {
>> -	if (vlan_id < 4096)
>> -		return 0;
>> -	fprintf(stderr, "Invalid vlan_id %d (must be < 4096)\n", vlan_id);
>> -	return 1;
>> +	if (!vlan_priority_ena && vlan_id >= 4096) {
>> +		fprintf(stderr, "Invalid vlan_id %d (must be < 4096)\n", vlan_id);
>> +		return 1;
>> +	}
>> +
>> +	/*
>> +	 * When vlan_priority_ena is enabled, allow any 16-bit value
>> +	 * to pass priority and CFI bits to the driver.
>> +	 */
>> +	return 0;
>>   }
>>   
>>   static uint32_t
>> @@ -6876,7 +6882,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)
>>   
>>   	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>   		return 1;
>> -	if (vlan_id_is_invalid(vlan_id))
>> +	if (vlan_id_is_invalid(vlan_id, vlan_priority_insert_ena))
> 
> Just vlan_id_is_invalid(vlan_id, false) because Rx is no need to impl this.
> 
>>   		return 1;
>>   	diag = rte_eth_dev_vlan_filter(port_id, vlan_id, on);
>>   	if (diag == 0)
>> @@ -6923,7 +6929,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>>   	struct rte_eth_dev_info dev_info;
>>   	int ret;
>>   
>> -	if (vlan_id_is_invalid(vlan_id))
>> +	if (vlan_id_is_invalid(vlan_id, vlan_priority_insert_ena))
>>   		return;
>>   
>>   	if (ports[port_id].dev_conf.txmode.offloads &
>> @@ -6954,9 +6960,9 @@ tx_qinq_set(portid_t port_id, uint16_t vlan_id, uint16_t vlan_id_outer)
>>   	struct rte_eth_dev_info dev_info;
>>   	int ret;
>>   
>> -	if (vlan_id_is_invalid(vlan_id))
>> +	if (vlan_id_is_invalid(vlan_id, vlan_priority_insert_ena))
>>   		return;
>> -	if (vlan_id_is_invalid(vlan_id_outer))
>> +	if (vlan_id_is_invalid(vlan_id_outer, vlan_priority_insert_ena))
>>   		return;
>>   
>>   	ret = eth_dev_info_get_print_err(port_id, &dev_info);
>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>> index 8c3b1244e7..3f37498d3b 100644
>> --- a/app/test-pmd/parameters.c
>> +++ b/app/test-pmd/parameters.c
>> @@ -117,6 +117,8 @@ enum {
>>   	TESTPMD_OPT_ENABLE_HW_VLAN_EXTEND_NUM,
>>   #define TESTPMD_OPT_ENABLE_HW_QINQ_STRIP "enable-hw-qinq-strip"
>>   	TESTPMD_OPT_ENABLE_HW_QINQ_STRIP_NUM,
>> +#define TESTPMD_OPT_ENABLE_VLAN_PRIORITY "enable-vlan-priority"
>> +	TESTPMD_OPT_ENABLE_VLAN_PRIORITY_NUM,
> 
> How about TESTPMD_OPT_ENABLE_VLAN_INSERT_PRI "enable-vlan-insert-pri"

I have adopted the simpler approach as suggested by Stephen.

>>   #define TESTPMD_OPT_ENABLE_DROP_EN "enable-drop-en"
>>   	TESTPMD_OPT_ENABLE_DROP_EN_NUM,
>>   #define TESTPMD_OPT_DISABLE_RSS "disable-rss"
>> @@ -461,6 +463,7 @@ usage(char* progname)
>>   	printf("  --enable-hw-vlan-strip: enable hardware vlan strip.\n");
>>   	printf("  --enable-hw-vlan-extend: enable hardware vlan extend.\n");
>>   	printf("  --enable-hw-qinq-strip: enable hardware qinq strip.\n");
>> +	printf("  --enable-vlan-priority: enable vlan priority insert.\n");
>>   	printf("  --enable-drop-en: enable per queue packet drop.\n");
>>   	printf("  --disable-rss: disable rss.\n");
>>   	printf("  --enable-rss: Force rss even for single-queue operation.\n");
>> @@ -1259,6 +1262,9 @@ launch_args_parse(int argc, char** argv)
>>   		case TESTPMD_OPT_ENABLE_HW_QINQ_STRIP_NUM:
>>   			rx_offloads |= RTE_ETH_RX_OFFLOAD_QINQ_STRIP;
>>   			break;
>> +		case TESTPMD_OPT_ENABLE_VLAN_PRIORITY_NUM:
>> +			vlan_priority_insert_ena = 1;
> 
> How about tx_insert_vlan_pri_en
> 
>> +			break;

I have adopted the simpler approach as suggested by Stephen, which 
eliminates the need for a new command-line option and global variable.

> 
> We need also update the testpmd document

OK.

Thanks,
Xingui



^ permalink raw reply

* Re: [PATCH] app/testpmd: add VLAN priority insert support
From: yangxingui @ 2026-06-16 13:19 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, david.marchand, aman.deep.singh, fengchengwen, yangshuaisong,
	lihuisong, liuyonglong, kangfenglong
In-Reply-To: <20260615121214.6fb7d8b7@phoenix.local>



On 2026/6/16 3:12, Stephen Hemminger wrote:
> On Fri, 12 Jun 2026 16:14:11 +0800
> Xingui Yang <yangxingui@huawei.com> wrote:
> 
>> The tx_vlan set command currently only accepts a VLAN ID in range
>> [0, 4095].  This patch adds support for an extended format that includes
>> 802.1p priority and CFI bits, allowing users to set the VLAN priority
>> tag when inserting VLAN headers in TX packets.
>>
>> The extended format is:
>>    bit 0-11:  VLAN ID (0-4095)
>>    bit 12:    CFI (Canonical Format Indicator)
>>    bit 13-15: Priority (0-7, 802.1p CoS)
>>
>> This is consistent with the VLAN tag structure used by
>> rte_eth_dev_set_vlan_pvid() where the PVID field encodes VLAN ID, CFI
>> and priority in the same format.
>>
>> A new command line option --enable-vlan-priority is added to enable this
>> feature. By default, the feature is disabled to maintain backward
>> compatibility with existing users. When enabled, the
>> vlan_id_is_invalid() function allows any 16-bit value to pass, while the
>> full 16-bit value (including CFI and priority bits) is passed to the
>> driver for hardware VLAN insertion.
>>
>> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
>> ---
> 
> 
> Having ability to set priority bits is good, and testpmd should allow it.
> The mbuf vlan_tci is already a full 16-bit TCI (priority/CFI/VID), and
> the TX insert path copies tx_vlan_id straight into it.  So priority
> insert already works; the only thing in the way is the < 4096 check.
> 
> Do you actually need a new option for this?  Both of_push_vlan +
> of_set_vlan_pcp (rte_flow) and "tx_vlan set pvid" already let you set
> the priority bits today, with no new code.
> 
> If you still want "tx_vlan set" itself to carry priority, I'd suggest
> a smaller change: relax only the TX insert validators and drop the
> option and the global.  Don't touch rx_vft_set -- it feeds the VLAN
> filter, which only takes a VLAN ID and rejects > 4095 anyway, so the
> flag just turns a clear error into a confusing one.
> 
> Either way, if the option stays, please document it, and add a release note.
> The commit message why the existing paths aren't enough.

Thank you for the suggestion. I have implemented the simpler approach in v2.

Thanks,
Xingui

^ permalink raw reply

* Re: [PATCH 2/2] ethdev: return 0 from dummy queue count
From: Stephen Hemminger @ 2026-06-16 14:07 UTC (permalink / raw)
  To: Maxime Leroy
  Cc: dev, stable, Thomas Monjalon, Andrew Rybchenko, Sunil Kumar Kori,
	Morten Brørup
In-Reply-To: <20260616094259.686555-3-maxime@leroys.fr>

On Tue, 16 Jun 2026 11:42:58 +0200
Maxime Leroy <maxime@leroys.fr> wrote:

> The dummy rx_queue_count/tx_queue_count callback returned -ENOTSUP. On a
> port that is not started (freshly allocated, or stopped once the fast-path
> ops are reset to dummies) there are no packets queued, so the truthful
> answer is 0, not an error: querying the count is not an unsupported
> operation. This also matches the dummy Rx/Tx burst, which reports 0
> packets.
> 
> A poll-mode worker checking rte_eth_rx_queue_count() across a concurrent
> port stop then sees an empty queue instead of a negative error.
> 
> Fixes: 066f3d9cc21c ("ethdev: remove callback checks from fast path")
> Cc: stable@dpdk.org
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Maxime Leroy <maxime@leroys.fr>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

^ permalink raw reply

* Re: [PATCH v2 6/6] net/dpaa2: drop the fake software VLAN strip offload
From: Stephen Hemminger @ 2026-06-16 14:11 UTC (permalink / raw)
  To: Maxime Leroy; +Cc: dev, Hemant Agrawal, Sachin Saxena
In-Reply-To: <20260616102727.708948-7-maxime@leroys.fr>

On Tue, 16 Jun 2026 12:27:26 +0200
Maxime Leroy <maxime@leroys.fr> wrote:

> RTE_ETH_RX_OFFLOAD_VLAN_STRIP is advertised, but no hardware VLAN strip
> backs it: when enabled, the Rx burst calls rte_vlan_strip() on every
> frame, a software op masquerading as a hardware offload.
> 
> It saves a forwarding application nothing: the datapath reads the L2
> header anyway to classify or strip. The offload does not remove that
> read, it relocates it into the driver Rx burst, where it is far more
> expensive.
> 
> The cost is a matter of timing. rte_vlan_strip() reaches the L2 header
> through rte_pktmbuf_mtod(), which dereferences mbuf->buf_addr. On a
> freshly recycled buffer that mbuf cacheline is cold. eth_fd_to_mbuf()
> has just written other fields of it (data_off, ol_flags), but buf_addr
> is a persistent field it does not rewrite. A write does not stall: it
> posts to the store buffer while the line fills in the background, and
> the rewritten fields are forwarded straight from there. buf_addr has
> nothing to forward, so it must be read from the line, whose fill is
> still in flight, and the read stalls. The ethertype read that follows,
> on the cold payload line, stalls again. Read later by the application,
> when the fill has completed, the same read hits. The offload just
> performs it at the worst possible moment.
> 
> Measured on a single-core port-to-port forwarding test over two 10G
> ports (one core at 2 GHz, 64-byte untagged frames):
> 
>   - throughput 4.22 -> 5.00 Mpps (+18 percent)
>   - IPC 0.93 -> 1.25: the cost was memory stall, not compute
>   - L3/DRAM-bound L2 refills 319M -> 200M over 10s (-37 percent)
> 
> perf confirms it: with the offload, the buf_addr load (the cold mbuf
> field) and the payload load account for about 84 percent of the Rx
> burst's L2 refills; removing it, those vanish and only the inherent DQRR
> dequeue misses remain.
> 
> Stop advertising VLAN_STRIP and remove the rte_vlan_strip() calls from
> every Rx path. This is a behavioural change: the tag is left in the
> frame, so an application must strip it itself, on the L2 header it
> already reads.
> 
> Signed-off-by: Maxime Leroy <maxime@leroys.fr>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

^ permalink raw reply

* Re: [PATCH v2] app/testpmd: add VLAN priority insert support
From: Stephen Hemminger @ 2026-06-16 14:23 UTC (permalink / raw)
  To: Xingui Yang
  Cc: dev, david.marchand, aman.deep.singh, fengchengwen, yangshuaisong,
	lihuisong, liuyonglong, kangfenglong
In-Reply-To: <20260616131001.2955655-1-yangxingui@huawei.com>

On Tue, 16 Jun 2026 21:10:01 +0800
Xingui Yang <yangxingui@huawei.com> wrote:

> The tx_vlan set and tx_qinq set commands only accepted VLAN ID in range
> [0, 4095]. This prevented users from setting 802.1p priority and CFI
> bits when using hardware VLAN insertion.
> 
> Since mbuf vlan_tci field already supports full 16-bit VLAN Tag Control
> Information (TCI), relax the validation for TX paths to allow priority
> and CFI bits. The vlan_id parameter now accepts:
>   - Bits 0-11:  VLAN ID (0-4095)
>   - Bit 12:    CFI (Canonical Format Indicator)
>   - Bits 13-15: Priority (0-7, 802.1p CoS)
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Suggested-by: fengchengwen <fengchengwen@huawei.com>
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> ---
> v2:
> - Removed --enable-vlan-priority option and global variable as suggested
>   by Stephen Hemminger. The feature is now always enabled for TX paths
> - RX VLAN filter continues to enforce strict VLAN ID validation as
>   suggested by fengchengwen
> - Added documentation updates for testpmd_funcs.rst and release notes
> 
>  app/test-pmd/config.c                       | 13 ++++++++-----
>  doc/guides/rel_notes/release_26_07.rst      |  7 +++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 17 ++++++++++++++---
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 9d457ca88e..38758f9c05 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1241,8 +1241,11 @@ void print_valid_ports(void)
>  }
>  
>  static int
> -vlan_id_is_invalid(uint16_t vlan_id)
> +vlan_id_is_invalid(uint16_t vlan_id, bool is_tx)
>  {
> +	if (is_tx)
> +		return 0;
> +
>  	if (vlan_id < 4096)
>  		return 0;
>  	fprintf(stderr, "Invalid vlan_id %d (must be < 4096)\n", vlan_id);
> @@ -6876,7 +6879,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)
>  
>  	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
> -	if (vlan_id_is_invalid(vlan_id))
> +	if (vlan_id_is_invalid(vlan_id, false))
>  		return 1;
>  	diag = rte_eth_dev_vlan_filter(port_id, vlan_id, on);
>  	if (diag == 0)
> @@ -6923,7 +6926,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>  	struct rte_eth_dev_info dev_info;
>  	int ret;
>  
> -	if (vlan_id_is_invalid(vlan_id))
> +	if (vlan_id_is_invalid(vlan_id, true))
>  		return;

Why have the is_tx flag if it is always used as constant?
Just remove the whole vlan_id_is_invalid() branch test in the transmit path.
Maybe add a comment that any VLAN is allowed on transmit?

Or make a new function. Since VLAN of 0xffff is reserved. Though you might want
to allow it since testpmd is for testing even invalid packets.


^ permalink raw reply

* Re: [RFC 1/4] telemetry: allow commands to receive file descriptors
From: Stephen Hemminger @ 2026-06-16 14:26 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev
In-Reply-To: <ajFCXCZfikPJTrLH@bricha3-mobl1.ger.corp.intel.com>

On Tue, 16 Jun 2026 13:32:28 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Tue, Jun 09, 2026 at 02:02:02PM -0700, Stephen Hemminger wrote:
> > Add rte_telemetry_register_cmd_fd_arg() to register a command whose
> > callback also receives file descriptors passed by the client as
> > SCM_RIGHTS ancillary data. The callback owns the descriptors and must
> > close them.
> > 
> > This lets a client open a file itself and hand the descriptor to the
> > primary process, so DPDK never opens the path. That avoids path and
> > permission problems and works across container filesystem namespaces.
> > 
> > Existing commands and clients are unaffected. If unsolicited file
> > descriptor is passed, it is closed.
> >   
> 
> This scheme seems reasonable in general. My only concern is whether the
> lack of potential windows support is an issue? For regular telemetry, there
> was always the option of a windows implementation using regular
> TCP/UDP/SCTP sockets bound to localhost. However, AFAIK there is no windows
> implementation of anything that supports file descriptors or handles
> between processes.
> 
> Some other pieces of feedback inline below.
> 
> /Bruce

I have new version (testing) that passes filename as parameter.
That should work without the fd passing.

^ permalink raw reply

* Re: [RFC 0/4] alternative capture mechanism
From: Stephen Hemminger @ 2026-06-16 14:28 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev
In-Reply-To: <ajFDjhDc5hXhE8km@bricha3-mobl1.ger.corp.intel.com>

On Tue, 16 Jun 2026 13:37:34 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Tue, Jun 09, 2026 at 02:02:01PM -0700, Stephen Hemminger wrote:
> > This is an RFC for an alternative way to capture packets from a DPDK
> > application. I did brief demo of similar mechanism at DPDK summit but
> > this is more complete. Capture runs in the primary process and is driven
> > entirely over telemetry; no secondary process is involved.
> > 
> > A client asks the application to start capturing and passes it a file
> > descriptor to write to. The application writes pcapng to that descriptor.
> > A Wireshark extcap script is the intended front end, but the control path
> > is just telemetry and the output is just a pipe, so other front ends are
> > possible.
> > 
> >   1/4  telemetry: let a command receive file descriptors from the client
> >   2/4  capture: the library
> >   3/4  test: functional test
> >   4/4  app: the Wireshark extcap script and its documentation
> > 
> > Setup and usage are in doc/guides/tools/wireshark_extcap.rst.
> > 
> > Primary process only for now; secondary-process capture is possible as
> > follow-on. Posting as RFC to get feedback on the approach.
> > 
> > The extcap script is dual licensed (BSD-3-Clause OR GPL-2.0-or-later) as
> > it may be more useful in the Wireshark tree.
> >   
> 
> One concern I have though - does this cause system-calls to be made in the
> fast-path because we are writting to a passed in FD? For performance
> reasons, would it not be better to use a memory buffer for this, thereby
> avoiding syscalls? For example, rather than passing in an FD to telemetry,
> we could pass in a key to be passed to shmget (going old-school!), or
> name parameter for shm_open. Thereafter with the memory buffer we can use a
> circular ring or similar to pass the data from app to client.
> 
> /Bruce

The system calls are contained inside the thread spawned when capture starts.
The flow is:
         callback -> ring -> capture thread -> FIFO -> wireshark

^ permalink raw reply

* RE: [PATCH v2 6/6] net/dpaa2: drop the fake software VLAN strip offload
From: Hemant Agrawal @ 2026-06-16 15:40 UTC (permalink / raw)
  To: Stephen Hemminger, Maxime Leroy; +Cc: dev@dpdk.org, Sachin Saxena
In-Reply-To: <20260616071110.1baf1beb@phoenix.local>



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: 16 June 2026 19:41
> To: Maxime Leroy <maxime@leroys.fr>
> Cc: dev@dpdk.org; Hemant Agrawal <hemant.agrawal@nxp.com>; Sachin
> Saxena <sachin.saxena@nxp.com>
> Subject: Re: [PATCH v2 6/6] net/dpaa2: drop the fake software VLAN strip
> offload
> Importance: High
> 
> On Tue, 16 Jun 2026 12:27:26 +0200
> Maxime Leroy <maxime@leroys.fr> wrote:
> 
> > RTE_ETH_RX_OFFLOAD_VLAN_STRIP is advertised, but no hardware VLAN
> > strip backs it: when enabled, the Rx burst calls rte_vlan_strip() on
> > every frame, a software op masquerading as a hardware offload.
> >
> > It saves a forwarding application nothing: the datapath reads the L2
> > header anyway to classify or strip. The offload does not remove that
> > read, it relocates it into the driver Rx burst, where it is far more
> > expensive.
> >
> > The cost is a matter of timing. rte_vlan_strip() reaches the L2 header
> > through rte_pktmbuf_mtod(), which dereferences mbuf->buf_addr. On a
> > freshly recycled buffer that mbuf cacheline is cold. eth_fd_to_mbuf()
> > has just written other fields of it (data_off, ol_flags), but buf_addr
> > is a persistent field it does not rewrite. A write does not stall: it
> > posts to the store buffer while the line fills in the background, and
> > the rewritten fields are forwarded straight from there. buf_addr has
> > nothing to forward, so it must be read from the line, whose fill is
> > still in flight, and the read stalls. The ethertype read that follows,
> > on the cold payload line, stalls again. Read later by the application,
> > when the fill has completed, the same read hits. The offload just
> > performs it at the worst possible moment.
> >
> > Measured on a single-core port-to-port forwarding test over two 10G
> > ports (one core at 2 GHz, 64-byte untagged frames):
> >
> >   - throughput 4.22 -> 5.00 Mpps (+18 percent)
> >   - IPC 0.93 -> 1.25: the cost was memory stall, not compute
> >   - L3/DRAM-bound L2 refills 319M -> 200M over 10s (-37 percent)
> >
> > perf confirms it: with the offload, the buf_addr load (the cold mbuf
> > field) and the payload load account for about 84 percent of the Rx
> > burst's L2 refills; removing it, those vanish and only the inherent
> > DQRR dequeue misses remain.
> >
> > Stop advertising VLAN_STRIP and remove the rte_vlan_strip() calls from
> > every Rx path. This is a behavioural change: the tag is left in the
> > frame, so an application must strip it itself, on the L2 header it
> > already reads.
> >
> > Signed-off-by: Maxime Leroy <maxime@leroys.fr>
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

^ permalink raw reply

* Re: DPDK release candidate 26.07-rc1
From: Riley Fletcher @ 2026-06-16 15:48 UTC (permalink / raw)
  To: Thomas Monjalon, dev
In-Reply-To: <EE7NFX47QzqKRvGYzs3QUA@monjalon.net>

IBM - Power Systems Testing
DPDK v26.07-rc1

* Build CI on Fedora 40, 41, 42 and 43 container images for ppc64le
* Basic PF on Mellanox: No issues found
* Performance: TestPMD throughput single core tests
* OS:- RHEL 10.2  kernel: 6.12.0-211.7.3.el10_2.ppc64le
         with gcc version 14.3.1 20251022 (Red Hat 14.3.1-4)

Systems tested:
  - LPARs on IBM Power11 CHRP IBM, 9105-22A
     NICs:
     - Mellanox Technologies MT4129 Family [ConnectX-7 100 GbE 2-Port]
     - Firmware Version: 28.48.1000 (MT_0000000834)
     - OFED 26.04-0.8.6

Regards,

Riley Fletcher

On Thu, 2026-06-11 at 04:52 +0200, Thomas Monjalon wrote:
> A new DPDK release candidate is ready for testing:
> 	https://git.dpdk.org/dpdk/tag/?id=v26.07-rc1
> 
> There are 432 new patches in this snapshot.
> 
> Release notes:
> 	https://doc.dpdk.org/guides/rel_notes/release_26_07.html
> 
> Highlights of 26.07-rc1:
> 	- option --pagesz-mem for per-page-size maximum
> 	- option --no-auto-probing for no device at init
> 	- less mempool cache misses in run-to-completion
> 	- peek style API for staged-ordered ring
> 	- RISC-V vector paths
> 	- PTP helpers and clock relay example
> 	- selective Rx API to save PCI bandwidth
> 	- vhost memory hotplug
> 	- pcap driver enhanced
> 	- LinkData sxe2 NIC driver
> 	- AI review helpers
> 
> Important note:
> Pipelined applications, where ethdev Rx and Tx run on separate
> lcores,
> should adapt to the new algorithm by doubling their configured
> mempool
> cache size, to avoid doubling their mempool cache miss rate.
> 
> Please test and report issues on bugs.dpdk.org.
> 
> We plan to release -rc2 in 2 weeks.
> 
> Thank you everyone
> 

^ permalink raw reply

* [PATCH 0/6] ip_frag: fix reassembly defects and add test
From: Stephen Hemminger @ 2026-06-16 21:05 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The IP reassembly library tracks only a running byte total and reserved
slots for the first and last fragments, with no coverage map. As a result
it mishandles duplicate, overlapping, oversized, and misheadered
fragments, and the IPv4 key is missing a field RFC 791 requires. There
was also no functional test to catch any of it.

These came out of reviewing a duplicate-fragment report on the list.

Patches 1 and 2 are interdependent: the overlap discard relies on the
duplicate handling so an exact duplicate is dropped on its own rather
than discarding the whole datagram. The rest are independent.

Patch 6 adds a functional test modeled on the Linux selftest ip_defrag.c.
It passes on this series; with any single fix reverted the matching case
fails.

Stephen Hemminger (6):
  ip_frag: tolerate duplicate fragments
  ip_frag: discard datagrams with overlapping fragments
  ip_frag: include protocol in IPv4 reassembly key
  ip_frag: drop IPv6 fragments with unexpected headers
  ip_frag: reject oversized reassembled datagrams
  app/test: add test for IP reassembly

 app/test/meson.build              |   1 +
 app/test/test_reassembly.c        | 644 ++++++++++++++++++++++++++++++
 lib/ip_frag/ip_frag_internal.c    |  36 +-
 lib/ip_frag/rte_ipv4_reassembly.c |  17 +-
 lib/ip_frag/rte_ipv6_reassembly.c |  22 +-
 5 files changed, 714 insertions(+), 6 deletions(-)
 create mode 100644 app/test/test_reassembly.c

-- 
2.53.0


^ permalink raw reply

* [PATCH 1/6] ip_frag: tolerate duplicate fragments
From: Stephen Hemminger @ 2026-06-16 21:05 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, stable, Samyak Jain, Konstantin Ananyev
In-Reply-To: <20260616210656.464062-1-stephen@networkplumber.org>

The reassembly code tracked only a running byte total and reserved slots
for the first and last fragments, with no check for a fragment
duplicating data already received. A single duplicate could destroy a
recoverable datagram:
 - a duplicate first or last fragment collided with the reserved slot and
   sent the whole entry down the error path, freeing every collected
   fragment;
 - a duplicate intermediate fragment was appended to a new slot, inflating
   frag_size past total_size so reassembly never completed.

RFC 791 reassembly tolerates duplicates: a fragment covering bytes
already present carries no new information. Check for an exact duplicate
(stored fragment with the same offset and length) and drop only that
mbuf, before frag_size is updated, leaving the entry's accounting
unchanged.

Overlapping fragments with differing bounds are a separate issue
addressed in the next patch.

Fixes: cc8f4d020c0b ("examples/ip_reassembly: initial import")
Cc: stable@dpdk.org
Reported-by: Samyak Jain <samyak.jain@amantyatech.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ip_frag/ip_frag_internal.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/ip_frag/ip_frag_internal.c b/lib/ip_frag/ip_frag_internal.c
index 382f42d0e1..9a03ef995a 100644
--- a/lib/ip_frag/ip_frag_internal.c
+++ b/lib/ip_frag/ip_frag_internal.c
@@ -89,7 +89,23 @@ struct rte_mbuf *
 ip_frag_process(struct ip_frag_pkt *fp, struct rte_ip_frag_death_row *dr,
 	struct rte_mbuf *mb, uint16_t ofs, uint16_t len, uint16_t more_frags)
 {
-	uint32_t idx;
+	uint32_t i, idx;
+
+	/*
+	 * Discard an exact duplicate fragment. If a previously stored fragment
+	 * already covers the same offset and length, this fragment carries no
+	 * new data. Reassembly is tolerant of duplicates (RFC 791), so drop
+	 * only this mbuf and keep the reassembly entry intact rather than
+	 * treating it as an error. Fragments overlapping an existing one with
+	 * different bounds are not handled here.
+	 */
+	for (i = 0; i != fp->last_idx; i++) {
+		if (fp->frags[i].mb != NULL && fp->frags[i].ofs == ofs &&
+				fp->frags[i].len == len) {
+			IP_FRAG_MBUF2DR(dr, mb);
+			return NULL;
+		}
+	}
 
 	fp->frag_size += len;
 
-- 
2.53.0


^ permalink raw reply related

* [PATCH 2/6] ip_frag: discard datagrams with overlapping fragments
From: Stephen Hemminger @ 2026-06-16 21:05 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, stable, Konstantin Ananyev
In-Reply-To: <20260616210656.464062-1-stephen@networkplumber.org>

Existing code does not handle overlapping fragments.

RFC 8200 (IPv6) requires that on overlap all reassembly is abandoned
andall received fragments are dropped. RFC 791 (IPv4) originally called
fortrimming and rewriting, but Linux discards for IPv4 as well, since
overlap has no legitimate use and is a known attack vector.

Depends on the duplicate-tolerance change so that an exact duplicate is
dropped on its own rather than discarding the whole datagram.

Fixes: cc8f4d020c0b ("examples/ip_reassembly: initial import")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ip_frag/ip_frag_internal.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/lib/ip_frag/ip_frag_internal.c b/lib/ip_frag/ip_frag_internal.c
index 9a03ef995a..2505314a29 100644
--- a/lib/ip_frag/ip_frag_internal.c
+++ b/lib/ip_frag/ip_frag_internal.c
@@ -92,16 +92,34 @@ ip_frag_process(struct ip_frag_pkt *fp, struct rte_ip_frag_death_row *dr,
 	uint32_t i, idx;
 
 	/*
-	 * Discard an exact duplicate fragment. If a previously stored fragment
-	 * already covers the same offset and length, this fragment carries no
-	 * new data. Reassembly is tolerant of duplicates (RFC 791), so drop
-	 * only this mbuf and keep the reassembly entry intact rather than
-	 * treating it as an error. Fragments overlapping an existing one with
-	 * different bounds are not handled here.
+	 * Scan the fragments already collected for this datagram before
+	 * storing the new one. The stored set is kept free of duplicates and
+	 * overlaps, so a single pass is sufficient.
 	 */
 	for (i = 0; i != fp->last_idx; i++) {
-		if (fp->frags[i].mb != NULL && fp->frags[i].ofs == ofs &&
-				fp->frags[i].len == len) {
+		if (fp->frags[i].mb == NULL)
+			continue;
+
+		/*
+		 * Exact duplicate: carries no new data. Reassembly tolerates
+		 * duplicates (RFC 791), so drop only this mbuf and keep the
+		 * entry.
+		 */
+		if (fp->frags[i].ofs == ofs && fp->frags[i].len == len) {
+			IP_FRAG_MBUF2DR(dr, mb);
+			return NULL;
+		}
+
+		/*
+		 * Overlap with an existing fragment. Per RFC 8200 section 4.5
+		 * (and RFC 5722) the datagram must be discarded; the same is
+		 * applied to IPv4. Free all collected fragments, drop this one,
+		 * and invalidate the entry.
+		 */
+		if (ofs < fp->frags[i].ofs + fp->frags[i].len &&
+				fp->frags[i].ofs < ofs + len) {
+			ip_frag_free(fp, dr);
+			ip_frag_key_invalidate(&fp->key);
 			IP_FRAG_MBUF2DR(dr, mb);
 			return NULL;
 		}
-- 
2.53.0


^ permalink raw reply related

* [PATCH 3/6] ip_frag: include protocol in IPv4 reassembly key
From: Stephen Hemminger @ 2026-06-16 21:05 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, stable, Konstantin Ananyev
In-Reply-To: <20260616210656.464062-1-stephen@networkplumber.org>

DPDK IPv4 reassembly code was not following RFC 791 section 3.2
which says:
    The internet identification field (ID) is used together with the
    source and destination address, and the protocol fields, to identify
    datagram fragments for reassembly.

Omitting the protocol means two datagrams between the
same pair of hosts that share an IP id but carry different protocols
(for example UDP and ICMP) are merged into a single reassembly context,
producing a corrupted datagram.

Fold the protocol into the unused upper bits of the 32-bit id field
of the key. The IPv4 identification is 16 bits and occupies the low
half, so the protocol can be carried in the upper bits without changing
the key layout, the key comparison or the hash.

Fixes: cc8f4d020c0b ("examples/ip_reassembly: initial import")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ip_frag/rte_ipv4_reassembly.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/ip_frag/rte_ipv4_reassembly.c b/lib/ip_frag/rte_ipv4_reassembly.c
index 3c8ae113ba..980f7a3b77 100644
--- a/lib/ip_frag/rte_ipv4_reassembly.c
+++ b/lib/ip_frag/rte_ipv4_reassembly.c
@@ -111,9 +111,15 @@ rte_ipv4_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
 	ip_ofs = (uint16_t)(flag_offset & RTE_IPV4_HDR_OFFSET_MASK);
 	ip_flag = (uint16_t)(flag_offset & RTE_IPV4_HDR_MF_FLAG);
 
+	/*
+	 * RFC 791 requires using: source, destination, identifier field and protocol
+	 */
+
 	/* use first 8 bytes only */
 	memcpy(&key.src_dst[0], &ip_hdr->src_addr, 8);
-	key.id = ip_hdr->packet_id;
+
+	/* packet_id is 16 bits and proto id is 8 bits */
+	key.id = ((uint32_t) ip_hdr->next_proto_id << 16) | ip_hdr->packet_id;
 	key.key_len = IPV4_KEYLEN;
 
 	ip_ofs *= RTE_IPV4_HDR_OFFSET_UNITS;
-- 
2.53.0


^ permalink raw reply related

* [PATCH 4/6] ip_frag: drop IPv6 fragments with unexpected headers
From: Stephen Hemminger @ 2026-06-16 21:05 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, stable, Konstantin Ananyev, Anatoly Burakov,
	Thomas Monjalon
In-Reply-To: <20260616210656.464062-1-stephen@networkplumber.org>

DPDK version of IPv6 reassembly only handles a fragment header placed
directly after the IPv6 header. With other extension headers in the
unfragmentable part, ipv6_frag_reassemble() patches the wrong
next-header field, miscomputes the payload length, and shifts the
wrong bytes, corrupting the result.

Drop the fragment when l3_len covers more than the IPv6 and fragment
headers. RFC 8200 allows a receiver to discard packets whose extension
headers are not in the recommended order, and RFC 9099 recommends
dropping non-conforming fragmented IPv6 packets, so dropping here is
permitted rather than a deviation.

Fixes: 4f1a8f633862 ("ip_frag: add IPv6 reassembly")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ip_frag/rte_ipv6_reassembly.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/ip_frag/rte_ipv6_reassembly.c b/lib/ip_frag/rte_ipv6_reassembly.c
index 0e809a01e5..7c1659002b 100644
--- a/lib/ip_frag/rte_ipv6_reassembly.c
+++ b/lib/ip_frag/rte_ipv6_reassembly.c
@@ -180,6 +180,19 @@ rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
 		return NULL;
 	}
 
+	/*
+	 * Only a fragment header directly following the IPv6 header is
+	 * supported. Other extension headers in the unfragmentable part are
+	 * not handled: ipv6_frag_reassemble() assumes l3_len covers exactly
+	 * the IPv6 and fragment headers when it patches the next-header field
+	 * and removes the fragment header. Drop the fragment rather than
+	 * produce a corrupt datagram.
+	 */
+	if (mb->l3_len != sizeof(struct rte_ipv6_hdr) + sizeof(*frag_hdr)) {
+		IP_FRAG_MBUF2DR(dr, mb);
+		return NULL;
+	}
+
 	if (unlikely(trim > 0))
 		rte_pktmbuf_trim(mb, trim);
 
-- 
2.53.0


^ permalink raw reply related

* [PATCH 5/6] ip_frag: reject oversized reassembled datagrams
From: Stephen Hemminger @ 2026-06-16 21:05 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, stable, Konstantin Ananyev
In-Reply-To: <20260616210656.464062-1-stephen@networkplumber.org>

The reassembled total length of a packet must not exceed 65535.
A fragment with a high offset could drive the sum past that,
causing silent truncation since IP payload_len/total_length is 16 bits.

When reassembling a packet the total length should not be allowed
to exceed 65535. A fragment with high offset could drive the sum
past that, causing silent truncation.

A valid datagram never exceeds 65535 bytes, so reject any fragment
whose resulting length would exceed that.
Fold the test into the existing zero-length check.

Fixes: cc8f4d020c0b ("examples/ip_reassembly: initial import")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ip_frag/rte_ipv4_reassembly.c | 9 +++++++--
 lib/ip_frag/rte_ipv6_reassembly.c | 9 +++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/lib/ip_frag/rte_ipv4_reassembly.c b/lib/ip_frag/rte_ipv4_reassembly.c
index 980f7a3b77..727fc58243 100644
--- a/lib/ip_frag/rte_ipv4_reassembly.c
+++ b/lib/ip_frag/rte_ipv4_reassembly.c
@@ -136,8 +136,13 @@ rte_ipv4_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
 		tbl, tbl->max_cycles, tbl->entry_mask, tbl->max_entries,
 		tbl->use_entries);
 
-	/* check that fragment length is greater then zero. */
-	if (ip_len <= 0) {
+	/*
+	 * Drop fragments with no payload, and any fragment whose end would
+	 * make the reassembled datagram exceed the maximum IPv4 size. The
+	 * total_length field is 16 bits, so otherwise it is silently
+	 * truncated while the mbuf still holds the full length.
+	 */
+	if (ip_len <= 0 || ip_ofs + ip_len + mb->l3_len > UINT16_MAX) {
 		IP_FRAG_MBUF2DR(dr, mb);
 		return NULL;
 	}
diff --git a/lib/ip_frag/rte_ipv6_reassembly.c b/lib/ip_frag/rte_ipv6_reassembly.c
index 7c1659002b..0b44275b37 100644
--- a/lib/ip_frag/rte_ipv6_reassembly.c
+++ b/lib/ip_frag/rte_ipv6_reassembly.c
@@ -174,8 +174,13 @@ rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
 		tbl, tbl->max_cycles, tbl->entry_mask, tbl->max_entries,
 		tbl->use_entries);
 
-	/* check that fragment length is greater then zero. */
-	if (ip_len <= 0) {
+	/*
+	 * Drop fragments with no payload, and any fragment whose end would
+	 * make the reassembled payload exceed 65535 bytes. The payload_len
+	 * field is 16 bits, so otherwise it is silently truncated while the
+	 * mbuf still holds the full length.
+	 */
+	if (ip_len <= 0 || ip_ofs + ip_len > UINT16_MAX) {
 		IP_FRAG_MBUF2DR(dr, mb);
 		return NULL;
 	}
-- 
2.53.0


^ permalink raw reply related

* [PATCH 6/6] app/test: add test for IP reassembly
From: Stephen Hemminger @ 2026-06-16 21:05 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger
In-Reply-To: <20260616210656.464062-1-stephen@networkplumber.org>

There was no functional test for IPv4/IPv6 reassembly,
only a performance test. Add a new test  modeled on the Linux kernel
selftest tools/testing/selftests/net/ip_defrag.c. This is new
code so no license conflict.

Test covers:
size and fragment-size sweep across in-order, reverse,
odd/even and block delivery orders with byte-exact payload validation;
minimum 8-byte fragments; the fragment-count limit;
timeout of incomplete datagrams;
and the duplicate, overlap, extension-header and
oversized-fragment cases fixed earlier in this series.

The reassembled packet is checked with rte_mbuf_check() to catch
malformed segment chains in addition to wrong content.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/meson.build       |   1 +
 app/test/test_reassembly.c | 644 +++++++++++++++++++++++++++++++++++++
 2 files changed, 645 insertions(+)
 create mode 100644 app/test/test_reassembly.c

diff --git a/app/test/meson.build b/app/test/meson.build
index 61024125a7..b8c2208d0b 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -160,6 +160,7 @@ source_file_deps = {
     'test_rawdev.c': ['rawdev', 'bus_vdev', 'raw_skeleton'],
     'test_rcu_qsbr.c': ['rcu', 'hash'],
     'test_rcu_qsbr_perf.c': ['rcu', 'hash'],
+    'test_reassembly.c': ['net', 'ip_frag'],
     'test_reassembly_perf.c': ['net', 'ip_frag'],
     'test_reciprocal_division.c': [],
     'test_reciprocal_division_perf.c': [],
diff --git a/app/test/test_reassembly.c b/app/test/test_reassembly.c
new file mode 100644
index 0000000000..9cada5f3b4
--- /dev/null
+++ b/app/test/test_reassembly.c
@@ -0,0 +1,644 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026
+ *
+ * Functional unit tests for the IP reassembly path of librte_ip_frag.
+ *
+ * Coverage mirrors the Linux selftest tools/testing/selftests/net/ip_defrag.c
+ * adapted to the library API and to DPDK-specific constraints:
+ *
+ *   - size / fragment-size sweep, bounded by RTE_LIBRTE_IP_FRAG_MAX_FRAG
+ *   - in-order, reverse, odd-then-even, and block-reordered delivery
+ *   - byte-exact validation of the reassembled payload (not just length)
+ *   - minimum (8-byte) fragments
+ *   - fragment-count boundary: exactly MAX reassembles, MAX + 1 fails
+ *   - incomplete datagram reaped on timeout
+ *   - zero-length fragment rejected
+ *   - duplicate fragment tolerated in a reordered set
+ *   - overlapping fragments (leading/trailing/contained) discarded
+ *   - IPv6 fragment with extension headers in the unfragmentable part dropped
+ *   - fragment whose end exceeds the maximum datagram size dropped
+ *
+ * The last four groups depend on the corresponding reassembly fixes
+ * (duplicate tolerance, overlap discard, extension-header drop, oversize
+ * drop); they pass once those are applied and fail on unpatched code. The
+ * remaining cases pass regardless.
+ *
+ * Fragments use l2_len == 0; the library reads the L3 header at offset 0.
+ */
+
+#include "test.h"
+
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_cycles.h>
+#include <rte_ip.h>
+#include <rte_ip_frag.h>
+#include <rte_log.h>
+#include <rte_mbuf.h>
+#include <rte_mempool.h>
+
+#define NB_MBUF		1024
+#define MBUF_CACHE	0		/* exact accounting for leak checks */
+#define MBUF_DATA	2048
+#define V4_L3_LEN	((uint16_t)sizeof(struct rte_ipv4_hdr))
+#define V6_L3_LEN	((uint16_t)(sizeof(struct rte_ipv6_hdr) + \
+				    RTE_IPV6_FRAG_HDR_SIZE))
+#define TEST_ID		0x4242
+
+#ifndef RTE_LIBRTE_IP_FRAG_MAX_FRAG
+#define RTE_LIBRTE_IP_FRAG_MAX_FRAG 8
+#endif
+#define MAX_FRAG	RTE_LIBRTE_IP_FRAG_MAX_FRAG
+
+#define MAX_PAYLOAD	(MAX_FRAG * 256)	/* keeps a fragment in one mbuf */
+
+enum family { V4, V6 };
+enum order  { IN_ORDER, REVERSE, ODD_EVEN, BLOCK };
+
+struct frag_desc {
+	uint16_t ofs;	/* byte offset into the payload */
+	uint16_t plen;	/* payload bytes after L3 */
+	uint8_t  mf;
+};
+
+static struct rte_mempool *pkt_pool;
+
+/* position-dependent payload pattern, non-periodic at 256 so a misordered
+ * reassembly is detected even when lengths line up.
+ */
+static inline uint8_t
+pat(uint32_t k)
+{
+	return (uint8_t)(k * 31u + 7u);
+}
+
+/* ------------------------------- harness -------------------------------- */
+
+static int
+testsuite_setup(void)
+{
+	/* the table create/destroy per case is chatty at INFO level */
+	rte_log_set_level_pattern("lib.ip_frag", RTE_LOG_NOTICE);
+
+	pkt_pool = rte_pktmbuf_pool_create("REASM_POOL", NB_MBUF, MBUF_CACHE,
+					   0, MBUF_DATA, SOCKET_ID_ANY);
+	return pkt_pool == NULL ? TEST_FAILED : TEST_SUCCESS;
+}
+
+static void
+testsuite_teardown(void)
+{
+	rte_mempool_free(pkt_pool);
+	pkt_pool = NULL;
+}
+
+/* Every case must start and end with a full pool, so a leak in one case is
+ * pinpointed here rather than silently masking the next one.
+ */
+static int
+ut_setup(void)
+{
+	if (rte_mempool_avail_count(pkt_pool) != NB_MBUF) {
+		printf("pool not full at case start: %u/%u\n",
+		       rte_mempool_avail_count(pkt_pool), NB_MBUF);
+		return TEST_FAILED;
+	}
+	return TEST_SUCCESS;
+}
+
+static struct rte_ip_frag_tbl *
+tbl_new(uint64_t max_cycles)
+{
+	return rte_ip_frag_table_create(16, MAX_FRAG, 16, max_cycles,
+					rte_socket_id());
+}
+
+/* Build one fragment with a position-dependent payload. */
+static struct rte_mbuf *
+build_frag(enum family fam, uint16_t ofs, uint16_t plen, uint8_t mf)
+{
+	struct rte_mbuf *m = rte_pktmbuf_alloc(pkt_pool);
+	uint16_t l3 = (fam == V4) ? V4_L3_LEN : V6_L3_LEN;
+	char *p;
+	uint16_t i;
+
+	if (m == NULL)
+		return NULL;
+	m->data_off = 0;
+
+	if (fam == V4) {
+		struct rte_ipv4_hdr *ip = rte_pktmbuf_mtod(m,
+						struct rte_ipv4_hdr *);
+		uint16_t fo = ofs / RTE_IPV4_HDR_OFFSET_UNITS;
+
+		memset(ip, 0, V4_L3_LEN);
+		if (mf)
+			fo |= RTE_IPV4_HDR_MF_FLAG;
+		ip->version_ihl = 0x45;
+		ip->total_length = rte_cpu_to_be_16(V4_L3_LEN + plen);
+		ip->packet_id = rte_cpu_to_be_16(TEST_ID);
+		ip->fragment_offset = rte_cpu_to_be_16(fo);
+		ip->time_to_live = 64;
+		ip->next_proto_id = IPPROTO_UDP;
+		ip->src_addr = rte_cpu_to_be_32(0x0a000001);
+		ip->dst_addr = rte_cpu_to_be_32(0x0a000002);
+	} else {
+		struct rte_ipv6_hdr *ip = rte_pktmbuf_mtod(m,
+						struct rte_ipv6_hdr *);
+		struct rte_ipv6_fragment_ext *fh =
+			rte_pktmbuf_mtod_offset(m,
+				struct rte_ipv6_fragment_ext *,
+				sizeof(struct rte_ipv6_hdr));
+
+		memset(ip, 0, V6_L3_LEN);
+		ip->vtc_flow = rte_cpu_to_be_32(6u << 28);
+		ip->payload_len = rte_cpu_to_be_16(RTE_IPV6_FRAG_HDR_SIZE + plen);
+		ip->proto = IPPROTO_FRAGMENT;
+		ip->hop_limits = 64;
+		ip->src_addr.a[15] = 1;
+		ip->dst_addr.a[15] = 2;
+		fh->next_header = IPPROTO_UDP;
+		fh->reserved = 0;
+		fh->frag_data = rte_cpu_to_be_16(
+				RTE_IPV6_SET_FRAG_DATA(ofs, mf ? 1 : 0));
+		fh->id = rte_cpu_to_be_32(TEST_ID);
+	}
+
+	p = rte_pktmbuf_mtod_offset(m, char *, l3);
+	for (i = 0; i < plen; i++)
+		p[i] = (char)pat(ofs + i);
+
+	m->data_len = m->pkt_len = l3 + plen;
+	m->l2_len = 0;
+	m->l3_len = l3;
+	return m;
+}
+
+static struct rte_mbuf *
+feed(enum family fam, struct rte_ip_frag_tbl *tbl,
+	struct rte_ip_frag_death_row *dr, const struct frag_desc *d, uint64_t tms)
+{
+	struct rte_mbuf *m = build_frag(fam, d->ofs, d->plen, d->mf);
+
+	if (m == NULL)
+		return NULL;
+	if (fam == V4) {
+		struct rte_ipv4_hdr *ip = rte_pktmbuf_mtod(m, struct rte_ipv4_hdr *);
+		return rte_ipv4_frag_reassemble_packet(tbl, dr, m, tms, ip);
+	} else {
+		struct rte_ipv6_hdr *ip = rte_pktmbuf_mtod(m, struct rte_ipv6_hdr *);
+		struct rte_ipv6_fragment_ext *fh =
+			rte_pktmbuf_mtod_offset(m, struct rte_ipv6_fragment_ext *,
+						sizeof(struct rte_ipv6_hdr));
+		return rte_ipv6_frag_reassemble_packet(tbl, dr, m, tms, ip, fh);
+	}
+}
+
+/* Split a datagram of total_plen into fragments of frag_size (multiple of 8).
+ * Returns the fragment count, or -1 if it would exceed MAX_FRAG.
+ */
+static int
+make_datagram(uint16_t total_plen, uint16_t frag_size, struct frag_desc *out)
+{
+	int n = 0;
+	uint16_t ofs = 0;
+
+	while (ofs < total_plen) {
+		uint16_t rem = total_plen - ofs;
+		uint16_t len = rem <= frag_size ? rem : frag_size;
+
+		if (n >= MAX_FRAG)
+			return -1;
+		out[n].ofs = ofs;
+		out[n].plen = len;
+		out[n].mf = (ofs + len < total_plen);
+		ofs += len;
+		n++;
+	}
+	return n;
+}
+
+/* Produce a delivery order (array of indices into descs). */
+static void
+make_order(enum order ord, int n, int *idx)
+{
+	int i, k = 0;
+
+	switch (ord) {
+	case IN_ORDER:
+		for (i = 0; i < n; i++)
+			idx[i] = i;
+		break;
+	case REVERSE:
+		for (i = 0; i < n; i++)
+			idx[i] = n - 1 - i;
+		break;
+	case ODD_EVEN:
+		for (i = 1; i < n; i += 2)
+			idx[k++] = i;
+		for (i = 0; i < n; i += 2)
+			idx[k++] = i;
+		break;
+	case BLOCK: {
+		int t = n / 3 ? n / 3 : 1;
+
+		for (i = 2 * t; i < n; i++)
+			idx[k++] = i;
+		for (i = t; i < 2 * t && i < n; i++)
+			idx[k++] = i;
+		for (i = 0; i < t && i < n; i++)
+			idx[k++] = i;
+		break;
+	}
+	}
+}
+
+/* Feed descs in the given order; return reassembled mbuf or NULL. */
+static struct rte_mbuf *
+run_ordered(enum family fam, const struct frag_desc *descs, int n,
+	    const int *idx)
+{
+	struct rte_ip_frag_death_row dr;
+	struct rte_ip_frag_tbl *tbl;
+	struct rte_mbuf *out = NULL;
+	uint64_t tms = rte_rdtsc();
+	int i;
+
+	memset(&dr, 0, sizeof(dr));
+	tbl = tbl_new(rte_get_tsc_hz());
+	if (tbl == NULL)
+		return NULL;
+	for (i = 0; i < n; i++) {
+		struct rte_mbuf *r = feed(fam, tbl, &dr, &descs[idx[i]], tms);
+
+		if (r != NULL)
+			out = r;
+	}
+	rte_ip_frag_free_death_row(&dr, 0);
+	rte_ip_frag_table_destroy(tbl);
+	return out;
+}
+
+/* Validate length and byte-exact payload, then free. Returns 0 on success.
+ * Note: reassembly strips the IPv6 fragment header, so the reassembled v6
+ * header is sizeof(rte_ipv6_hdr), not the V6_L3_LEN the fragments were built
+ * with. v4 has no fragment header to remove.
+ */
+static int
+validate(struct rte_mbuf *m, enum family fam, uint16_t total_plen)
+{
+	uint16_t l3 = (fam == V4) ? V4_L3_LEN :
+				    (uint16_t)sizeof(struct rte_ipv6_hdr);
+	uint8_t buf[MAX_PAYLOAD];
+	const uint8_t *p;
+	const char *reason;
+	uint16_t k;
+	int rc = 0;
+
+	if (m == NULL)
+		return -1;
+	if (rte_mbuf_check(m, 1, &reason) != 0) {
+		printf("  bad mbuf fam=%d total=%u: %s\n", fam, total_plen,
+		       reason);
+		rte_pktmbuf_free(m);
+		return -1;
+	}
+	if (m->pkt_len != (uint32_t)(l3 + total_plen)) {
+		rte_pktmbuf_free(m);
+		return -1;
+	}
+	p = rte_pktmbuf_read(m, l3, total_plen, buf);
+	if (p == NULL) {
+		rte_pktmbuf_free(m);
+		return -1;
+	}
+	for (k = 0; k < total_plen; k++) {
+		if (p[k] != pat(k)) {
+			rc = -1;
+			break;
+		}
+	}
+	rte_pktmbuf_free(m);
+	return rc;
+}
+
+/* --------------------------- baseline / sweep --------------------------- */
+
+static int
+sweep_one(enum family fam, uint16_t total_plen, uint16_t frag_size)
+{
+	struct frag_desc descs[MAX_FRAG];
+	int idx[MAX_FRAG];
+	const enum order orders[] = { IN_ORDER, REVERSE, ODD_EVEN, BLOCK };
+	int n = make_datagram(total_plen, frag_size, descs);
+	unsigned int o;
+
+	if (n < 2)		/* skip single-fragment / oversized for sweep */
+		return 0;
+
+	for (o = 0; o < RTE_DIM(orders); o++) {
+		make_order(orders[o], n, idx);
+		if (validate(run_ordered(fam, descs, n, idx), fam,
+			     total_plen) != 0) {
+			printf("  sweep fail: fam=%d total=%u fs=%u order=%u n=%d\n",
+			       fam, total_plen, frag_size, orders[o], n);
+			return -1;
+		}
+	}
+	return 0;
+}
+
+static int
+sweep(enum family fam)
+{
+	const uint16_t fsizes[] = { 8, 16, 64, 256 };
+	unsigned int f;
+
+	for (f = 0; f < RTE_DIM(fsizes); f++) {
+		uint16_t fs = fsizes[f];
+		uint16_t total;
+
+		/* cover 2..MAX_FRAG fragments, last fragment partial */
+		for (total = fs + 8; total <= fs * MAX_FRAG; total += fs) {
+			if (sweep_one(fam, total, fs) != 0)
+				return TEST_FAILED;
+			if (total > fs + 4 &&
+			    sweep_one(fam, total - 4, fs) != 0)
+				return TEST_FAILED;
+		}
+	}
+	return TEST_SUCCESS;
+}
+
+static int test_sweep_v4(void) { return sweep(V4); }
+static int test_sweep_v6(void) { return sweep(V6); }
+
+/* Minimum 8-byte fragments. */
+static int
+test_min_fragment(void)
+{
+	struct frag_desc d[3] = {
+		{ 0, 8, 1 }, { 8, 8, 1 }, { 16, 8, 0 },
+	};
+	int idx[3];
+
+	make_order(REVERSE, 3, idx);
+	TEST_ASSERT_SUCCESS(validate(run_ordered(V4, d, 3, idx), V4, 24),
+				"min 8-byte fragments not reassembled");
+	make_order(ODD_EVEN, 3, idx);
+	TEST_ASSERT_SUCCESS(validate(run_ordered(V6, d, 3, idx), V6, 24),
+				"min 8-byte fragments not reassembled (v6)");
+	return TEST_SUCCESS;
+}
+
+/* Exactly MAX_FRAG fragments reassembles; MAX_FRAG + 1 fails. */
+static int
+test_cap_boundary(void)
+{
+	struct frag_desc d[MAX_FRAG + 1];
+	int idx[MAX_FRAG + 1];
+	uint16_t fs = 8, total = fs * MAX_FRAG;
+	int n, i;
+
+	n = make_datagram(total, fs, d);
+	TEST_ASSERT_EQUAL(n, MAX_FRAG, "expected MAX_FRAG fragments");
+	make_order(IN_ORDER, n, idx);
+	TEST_ASSERT_SUCCESS(validate(run_ordered(V4, d, n, idx), V4, total),
+				"MAX_FRAG fragments should reassemble");
+
+	/* one more fragment than the table can hold */
+	for (i = 0; i <= MAX_FRAG; i++) {
+		d[i].ofs = i * fs;
+		d[i].plen = fs;
+		d[i].mf = (i < MAX_FRAG);
+		idx[i] = i;
+	}
+	TEST_ASSERT_NULL(run_ordered(V4, d, MAX_FRAG + 1, idx),
+			     "MAX_FRAG + 1 fragments should not reassemble");
+	TEST_ASSERT_EQUAL(rte_mempool_avail_count(pkt_pool), NB_MBUF,
+			      "overflowing set leaked mbufs");
+	return TEST_SUCCESS;
+}
+
+/* Incomplete datagram: no output, reaped on timeout. */
+static int
+test_incomplete_timeout(void)
+{
+	struct rte_ip_frag_death_row dr;
+	struct rte_ip_frag_tbl *tbl;
+	uint64_t mc = rte_get_tsc_hz(), tms = rte_rdtsc();
+	struct frag_desc d[2] = { { 0, 64, 1 }, { 128, 64, 0 } }; /* gap */
+	struct rte_mbuf *out = NULL;
+	int i;
+
+	memset(&dr, 0, sizeof(dr));
+	tbl = tbl_new(mc);
+	TEST_ASSERT_NOT_NULL(tbl, "table create failed");
+	for (i = 0; i < 2; i++) {
+		struct rte_mbuf *r = feed(V4, tbl, &dr, &d[i], tms);
+
+		if (r != NULL)
+			out = r;
+	}
+	TEST_ASSERT_NULL(out, "incomplete datagram reassembled");
+	rte_ip_frag_table_del_expired_entries(tbl, &dr, tms + mc + 1);
+	rte_ip_frag_free_death_row(&dr, 0);
+	rte_ip_frag_table_destroy(tbl);
+	TEST_ASSERT_EQUAL(rte_mempool_avail_count(pkt_pool), NB_MBUF,
+			      "expired fragments not freed");
+	return TEST_SUCCESS;
+}
+
+static int
+test_zero_len(void)
+{
+	struct frag_desc d = { 0, 0, 1 };
+	int idx = 0;
+
+	TEST_ASSERT_NULL(run_ordered(V4, &d, 1, &idx),
+			     "zero-length fragment accepted");
+	TEST_ASSERT_EQUAL(rte_mempool_avail_count(pkt_pool), NB_MBUF,
+			      "zero-length fragment leaked");
+	return TEST_SUCCESS;
+}
+
+/* --------------------- duplicate / overlap / reject --------------------- */
+
+/* A duplicate anywhere in a reordered set must not break reassembly. */
+static int
+test_dup_tolerated(void)
+{
+	/* offsets 0,64,128,192 with 64B frags; inject a dup of frag 1 */
+	struct frag_desc d[5] = {
+		{   0, 64, 1 }, {  64, 64, 1 }, {  64, 64, 1 }, /* dup */
+		{ 128, 64, 1 }, { 192, 64, 0 },
+	};
+	int idx[5] = { 1, 4, 2, 0, 3 };	/* reordered, dup interleaved */
+
+	TEST_ASSERT_SUCCESS(validate(run_ordered(V4, d, 5, idx), V4, 256),
+				"duplicate fragment broke reassembly");
+	return TEST_SUCCESS;
+}
+
+/* Overlap geometries; the datagram must be discarded and every collected
+ * fragment freed. The last fragment is withheld so that on unfixed code the
+ * entry is *retained* (total_size stays UINT32_MAX) rather than torn down by
+ * the frag_size > total_size path: that retention is what we detect. We
+ * capture the mbufs still held in the table after draining the death row,
+ * before destroying the table (destroy frees held mbufs, hiding the leak).
+ */
+static int
+overlap_case(enum family fam, const struct frag_desc *d, int n, const char *what)
+{
+	struct rte_ip_frag_death_row dr;
+	struct rte_ip_frag_tbl *tbl;
+	struct rte_mbuf *out = NULL;
+	uint64_t tms = rte_rdtsc();
+	unsigned int held;
+	int i;
+
+	memset(&dr, 0, sizeof(dr));
+	tbl = tbl_new(rte_get_tsc_hz());
+	if (tbl == NULL)
+		return -1;
+	for (i = 0; i < n; i++) {
+		struct rte_mbuf *r = feed(fam, tbl, &dr, &d[i], tms);
+
+		if (r != NULL)
+			out = r;
+	}
+	rte_ip_frag_free_death_row(&dr, 0);
+	held = NB_MBUF - rte_mempool_avail_count(pkt_pool);
+	rte_ip_frag_table_destroy(tbl);
+
+	if (out != NULL) {
+		rte_pktmbuf_free(out);
+		printf("  overlap reassembled instead of discarded: %s\n", what);
+		return -1;
+	}
+	if (held != 0) {
+		printf("  overlap kept %u fragment(s) instead of discarding: %s\n",
+		       held, what);
+		return -1;
+	}
+	return 0;
+}
+
+static int
+test_overlap(void)
+{
+	/* last fragment withheld in every case (all MF=1) */
+
+	/* overlapping fragment arrives second */
+	const struct frag_desc tail[2] = { { 0, 600, 1 }, { 300, 600, 1 } };
+	/* overlapping fragment arrives first */
+	const struct frag_desc head[2] = { { 300, 600, 1 }, { 0, 600, 1 } };
+	/* a fragment fully contained in an existing one */
+	const struct frag_desc cont[2] = { { 0, 600, 1 }, { 200, 200, 1 } };
+
+	TEST_ASSERT_SUCCESS(overlap_case(V6, tail, 2, "v6 overlap second"), "");
+	TEST_ASSERT_SUCCESS(overlap_case(V6, head, 2, "v6 overlap first"), "");
+	TEST_ASSERT_SUCCESS(overlap_case(V6, cont, 2, "v6 contained"), "");
+	TEST_ASSERT_SUCCESS(overlap_case(V4, tail, 2, "v4 overlap second"), "");
+	return TEST_SUCCESS;
+}
+
+/* IPv6 fragment with an unfragmentable extension header is dropped, not
+ * stored. Capture whether the fragment is still held in the table after the
+ * death row is drained but before the table is destroyed.
+ */
+static int
+test_v6_ext_header_drop(void)
+{
+	struct rte_ip_frag_death_row dr;
+	struct rte_ip_frag_tbl *tbl;
+	struct rte_mbuf *m, *r;
+	struct rte_ipv6_hdr *ip;
+	struct rte_ipv6_fragment_ext *fh;
+	unsigned int held;
+
+	memset(&dr, 0, sizeof(dr));
+	tbl = tbl_new(rte_get_tsc_hz());
+	TEST_ASSERT_NOT_NULL(tbl, "table create failed");
+
+	m = build_frag(V6, 0, 64, 1);
+	TEST_ASSERT_NOT_NULL(m, "alloc failed");
+	/* pretend an 8-byte extension header sits before the fragment header */
+	m->l3_len += 8;
+	ip = rte_pktmbuf_mtod(m, struct rte_ipv6_hdr *);
+	fh = rte_pktmbuf_mtod_offset(m, struct rte_ipv6_fragment_ext *,
+				     sizeof(struct rte_ipv6_hdr));
+	r = rte_ipv6_frag_reassemble_packet(tbl, &dr, m, rte_rdtsc(), ip, fh);
+	rte_ip_frag_free_death_row(&dr, 0);
+	held = NB_MBUF - rte_mempool_avail_count(pkt_pool);
+	rte_ip_frag_table_destroy(tbl);
+
+	TEST_ASSERT_NULL(r, "fragment with extension header accepted");
+	TEST_ASSERT_EQUAL(held, 0,
+			  "extension-header fragment stored instead of dropped");
+	return TEST_SUCCESS;
+}
+
+/* A fragment whose end exceeds the max datagram size is dropped, not stored. */
+static int
+oversize_drop_one(enum family fam)
+{
+	struct rte_ip_frag_death_row dr;
+	struct rte_ip_frag_tbl *tbl;
+	struct frag_desc d = { 0xFFF8, 64, 0 };	/* offset 65528 + 64 > 65535 */
+	struct rte_mbuf *r;
+	unsigned int held;
+
+	memset(&dr, 0, sizeof(dr));
+	tbl = tbl_new(rte_get_tsc_hz());
+	if (tbl == NULL)
+		return -1;
+	r = feed(fam, tbl, &dr, &d, rte_rdtsc());
+	rte_ip_frag_free_death_row(&dr, 0);
+	held = NB_MBUF - rte_mempool_avail_count(pkt_pool);
+	rte_ip_frag_table_destroy(tbl);
+
+	if (r != NULL) {
+		rte_pktmbuf_free(r);
+		return -1;
+	}
+	return held == 0 ? 0 : -1;
+}
+
+static int
+test_oversize_drop(void)
+{
+	TEST_ASSERT_SUCCESS(oversize_drop_one(V4),
+			    "oversized v4 fragment stored instead of dropped");
+	TEST_ASSERT_SUCCESS(oversize_drop_one(V6),
+			    "oversized v6 fragment stored instead of dropped");
+	return TEST_SUCCESS;
+}
+
+static struct unit_test_suite reassembly_testsuite = {
+	.suite_name = "IP Reassembly Unit Test Suite",
+	.setup = testsuite_setup,
+	.teardown = testsuite_teardown,
+	.unit_test_cases = {
+		TEST_CASE_ST(ut_setup, NULL, test_sweep_v4),
+		TEST_CASE_ST(ut_setup, NULL, test_sweep_v6),
+		TEST_CASE_ST(ut_setup, NULL, test_min_fragment),
+		TEST_CASE_ST(ut_setup, NULL, test_cap_boundary),
+		TEST_CASE_ST(ut_setup, NULL, test_incomplete_timeout),
+		TEST_CASE_ST(ut_setup, NULL, test_zero_len),
+		TEST_CASE_ST(ut_setup, NULL, test_dup_tolerated),
+		TEST_CASE_ST(ut_setup, NULL, test_overlap),
+		TEST_CASE_ST(ut_setup, NULL, test_v6_ext_header_drop),
+		TEST_CASE_ST(ut_setup, NULL, test_oversize_drop),
+		TEST_CASES_END()
+	}
+};
+
+static int
+test_reassembly(void)
+{
+	return unit_test_suite_runner(&reassembly_testsuite);
+}
+
+REGISTER_FAST_TEST(reassembly_autotest, NOHUGE_SKIP, ASAN_OK, test_reassembly);
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v10 1/1] net/mana: add device reset support
From: Stephen Hemminger @ 2026-06-16 21:22 UTC (permalink / raw)
  To: Wei Hu; +Cc: dev, longli, weh
In-Reply-To: <20260616123158.43583-2-weh@linux.microsoft.com>

On Tue, 16 Jun 2026 05:31:58 -0700
Wei Hu <weh@linux.microsoft.com> wrote:

> teardown immediately (dev_stop, secondary IPC, dev_close, MR cache
> free) before waiting for the hardware recovery timer to fire. This
> avoids blocking the EAL interrupt thread on multi-second IPC
> timeouts and ibverbs calls. After the recovery delay, the thread
> unregisters the interrupt handler, re-probes the PCI device,
> reinitializes MR caches, and restarts queues. Each function owns
> its own lock scope with no lock hand-off between threads.
> 
> Each queue has an atomic burst_state variable where bit 0 is the
> in-burst flag and bit 1 is a blocked flag. The data path uses a
> single compare-and-swap (0 to 1) to enter a burst, which fails
> immediately if the blocked bit is set. The reset path sets the
> blocked bit via atomic fetch-or and polls bit 0 to wait for
> in-flight bursts to drain. This single-variable design avoids the
> need for sequential consistency ordering.
> 
> A per-device mutex serializes the reset path with ethdev
> operations. The mutex uses PTHREAD_PROCESS_SHARED for multi-process
> support and is held across blocking IB verbs calls. A trylock
> helper encapsulates the lock acquisition and device state check
> for all ethdev operation wrappers. Operations that cannot wait
> (configure, queue setup) return -EBUSY during reset, while
> dev_stop and dev_close join the reset thread before acquiring
> the lock to ensure proper sequencing.
> 
> The reset thread keeps reset_thread_active true throughout its
> lifetime. mana_join_reset_thread uses rte_thread_equal to detect
> the self-join case (when a recovery callback calls dev_stop or
> dev_close from the reset thread itself) and calls
> rte_thread_detach instead of join, so thread resources are freed
> on exit. External callers join normally.
> 
> The condvar wait in the reset thread uses a predicate loop that
> checks dev_state under reset_cond_mutex, so a PCI remove signal
> that arrives before the thread enters the wait is not lost. The
> PCI remove callback sets dev_state to RESET_FAILED under the
> same mutex before signaling. A lock/unlock barrier on
> reset_ops_lock in the PCI remove path ensures teardown has
> completed before emitting the INTR_RMV event.
> 
> Multi-process support is included: secondary processes unmap and
> remap doorbell pages via IPC during the reset enter and exit
> phases. The secondary RESET_EXIT handler closes the received fd
> unconditionally after processing, even when the doorbell page is
> already mapped. Data path functions in both primary and secondary
> processes check the device state atomically and return early when
> the device is not active.
> 
> The driver emits RTE_ETH_EVENT_ERR_RECOVERING before entering the
> reset path so that upper layers (e.g. netvsc) can switch their
> data path before queues are stopped. The event is emitted outside
> the reset lock to avoid deadlock if the callback calls dev_stop or
> dev_close. On completion, the driver emits RECOVERY_SUCCESS or
> RECOVERY_FAILED after releasing the lock. If a recovery callback
> triggers dev_stop or dev_close, the self-join detection in
> mana_join_reset_thread detaches the thread to avoid deadlock. If
> the enter phase fails internally, RECOVERY_FAILED is sent
> immediately so the application receives a terminal event. A PCI
> device removal event callback distinguishes hot-remove from
> service reset.
> 
> Documentation for the device reset feature is added in the MANA
> NIC guide and the 26.07 release notes.
> 
> Signed-off-by: Wei Hu <weh@microsoft.com>
> ---
Applied to next-net

^ permalink raw reply

* Re: [PATCH v6 00/21] Wangxun Fixes
From: Stephen Hemminger @ 2026-06-16 21:42 UTC (permalink / raw)
  To: Zaiyu Wang; +Cc: dev
In-Reply-To: <20260616122030.9688-1-zaiyuwang@trustnetic.com>

On Tue, 16 Jun 2026 20:20:08 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:

> This series fixes several issues found on Wangxun Emerald, Sapphire and
> Amber-lite NICs, with a focus on link-related problems.
> ---
> v6:
> - Fixed more issues identified by AI review
> ---
> v5:
> - Fixed issues identified by AI review
> ---
> v4:
> - Fixed issues identified by devtools scripts
> ---
> v3:
> - Addressed Stephen's comments
> ---
> v2:
> - Fixed compilation error and code style issues
> ---
> 
> Zaiyu Wang (21):
>   net/txgbe: remove duplicate xstats counters
>   net/ngbe: remove duplicate xstats counters
>   net/ngbe: add missing CDR config for YT PHY
>   net/ngbe: fix VF promiscuous and allmulticast
>   net/txgbe: fix inaccuracy in Tx rate limiting
>   net/txgbe: fix link status check condition
>   net/txgbe: fix Tx desc free logic
>   net/txgbe: fix link flow control registers for Amber-Lite
>   net/txgbe: fix link flow control config for Sapphire
>   net/txgbe: fix a mass of unknown interrupts
>   net/txgbe: fix traffic class priority configuration
>   net/txgbe: fix link stability for 25G NIC
>   net/txgbe: fix link stability for 40G NIC
>   net/txgbe: fix link stability for Amber-Lite backplane mode
>   net/txgbe: fix FEC mode configuration on 25G NIC
>   net/txgbe: fix SFP module identification
>   net/txgbe: fix get module info operation
>   net/txgbe: fix get EEPROM operation
>   net/txgbe: fix to reset Tx write-back pointer
>   net/txgbe: fix to enable Tx desc check
>   net/txgbe: fix temperature track for AML NIC
> 
>  drivers/net/ngbe/base/ngbe_phy_yt.c       |    3 +
>  drivers/net/ngbe/ngbe_ethdev.c            |    5 -
>  drivers/net/ngbe/ngbe_ethdev_vf.c         |   11 +-
>  drivers/net/txgbe/base/meson.build        |    2 +
>  drivers/net/txgbe/base/txgbe.h            |    2 +
>  drivers/net/txgbe/base/txgbe_aml.c        |  185 +-
>  drivers/net/txgbe/base/txgbe_aml.h        |    6 +-
>  drivers/net/txgbe/base/txgbe_aml40.c      |  114 +-
>  drivers/net/txgbe/base/txgbe_aml40.h      |    6 +-
>  drivers/net/txgbe/base/txgbe_dcb_hw.c     |    2 +-
>  drivers/net/txgbe/base/txgbe_e56.c        | 3773 +++++++++++++++++++++
>  drivers/net/txgbe/base/txgbe_e56.h        | 1753 ++++++++++
>  drivers/net/txgbe/base/txgbe_e56_bp.c     | 2597 ++++++++++++++
>  drivers/net/txgbe/base/txgbe_e56_bp.h     |  282 ++
>  drivers/net/txgbe/base/txgbe_hw.c         |   54 +-
>  drivers/net/txgbe/base/txgbe_hw.h         |    4 +-
>  drivers/net/txgbe/base/txgbe_osdep.h      |    4 +
>  drivers/net/txgbe/base/txgbe_phy.c        |  362 +-
>  drivers/net/txgbe/base/txgbe_phy.h        |   46 +-
>  drivers/net/txgbe/base/txgbe_regs.h       |   13 +-
>  drivers/net/txgbe/base/txgbe_type.h       |   43 +-
>  drivers/net/txgbe/txgbe_ethdev.c          |  472 ++-
>  drivers/net/txgbe/txgbe_ethdev.h          |    7 +-
>  drivers/net/txgbe/txgbe_rxtx.c            |  109 +-
>  drivers/net/txgbe/txgbe_rxtx.h            |   36 +
>  drivers/net/txgbe/txgbe_rxtx_vec_common.h |   17 +-
>  26 files changed, 9464 insertions(+), 444 deletions(-)
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56.c
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56.h
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56_bp.c
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56_bp.h
> 

AI review was mostly clean, do you want to change this one thing?

Review of [PATCH v6 00/21] net/{txgbe,ngbe} fixes from Zaiyu Wang

All findings from the v5 review are addressed:

  - 07/21: Tx desc free now loads the 32-bit headwb_mem atomically and
    casts to uint16_t at the use site, removing the type pun.
  - 14/21: kr_read_poll() switches from usleep() to usec_delay() to
    match the rest of the txgbe base layer.
  - 16/21: stray tab+space indent in txgbe_read_i2c_sff8636() is gone.
  - 17/21: stray tab+space indent in txgbe_get_module_info() is gone.
    TXGBE_SFF_DDM_IMPLEMENTED is now indented as a bit mask under the
    8472_SWAP register definition, which reads correctly.
  - 18/21: page is reset to 0 inside each iteration of the for loop so
    it no longer accumulates across iterations.  The flat-memory check
    is now driven by an explicit read of module byte 2 via
    read_i2c_sff8636() into a local is_flat_mem flag, instead of
    reading data[0x2] out of the caller's output buffer.  databyte is
    cleared to 0 at the top of each loop body so the skipped-read
    branch can no longer leak the previous iteration's value.
  - 20/21: RTE_LIBRTE_SECURITY is corrected to RTE_LIB_SECURITY so the
    !txq->using_ipsec guard is actually preprocessed in.

One small remaining comment on 18/21, not blocking.

Patch 18/21 (net/txgbe: fix get EEPROM operation)

Info: is_flat_mem has inverted semantics relative to its name, which
is going to confuse future maintainers.  SFF-8636 byte 2 bit 2
("Flat_mem") is 1 for flat-memory modules and 0 for paged-memory
modules (this is the same convention ethtool uses).  The new code
treats the bit the opposite way:

  +	bool is_flat_mem = true;
  ...
  +		if (rdata & 0x4)
  +			is_flat_mem = false;
  ...
  +			if (page == 0 || is_flat_mem) {
  +				status = hw->phy.read_i2c_sff8636(hw, page, ...);

So when the module reports flat (bit set), is_flat_mem is set to
false; when paged (bit clear), is_flat_mem stays true.  The condition
'page == 0 || is_flat_mem' reads only page 0 in the flat case and any
page in the paged case, which is the right behavior, but the variable
ends up meaning "paged memory is OK to read" rather than "module is
flat".  Suggest either:

	bool is_flat_mem = false;
	if (rdata & 0x4)
		is_flat_mem = true;
	...
	if (page == 0 || !is_flat_mem) {
		...
	}

or renaming to allow_paged_reads (or similar) and keeping the existing
default and condition.



^ permalink raw reply

* Re: [PATCH 0/6] net/dpaa2: RSS fixes and improvements
From: Stephen Hemminger @ 2026-06-16 22:07 UTC (permalink / raw)
  To: Maxime Leroy; +Cc: dev
In-Reply-To: <20260616104717.723087-1-maxime@leroys.fr>

On Tue, 16 Jun 2026 12:47:09 +0200
Maxime Leroy <maxime@leroys.fr> wrote:

> A set of RSS fixes and improvements for the dpaa2 PMD.
> 
> Patches 1 and 2 fix the RSS hash key: the L4 destination port was never
> added (both extracts used the source port), and SCTP now uses the same L4
> port extraction as TCP/UDP. Both are tagged for stable.
> 
> Patches 3 and 4 are small cleanups in the key builder (a dead num_extracts
> increment, and the unset PPPoE guard flag).
> 
> Patch 5 honours RTE_ETH_RSS_LEVEL_INNERMOST so tunnelled traffic hashes on
> the inner IP header. Patch 6 implements reta_query / reta_update as an
> emulation over the HW distribution-size mechanism, since dpaa2 has no
> software-visible indirection table.
> 
> Tested on LX2160A (lx2160acex7).

Applied to next-net

^ permalink raw reply

* Re: [RFC] devtools: add tool calling support to review-patch.py
From: Stephen Hemminger @ 2026-06-16 22:42 UTC (permalink / raw)
  To: Aaron Conole; +Cc: dev, David Marchand
In-Reply-To: <20260609182652.1053422-1-aconole@redhat.com>

On Tue,  9 Jun 2026 14:26:47 -0400
Aaron Conole <aconole@redhat.com> wrote:

> Add an iterative tool-use loop to review-patch.py for the Anthropic
> and OpenAI providers. The reviewer can now look up additional context
> from the DPDK source tree when the patch alone is insufficient,
> rather than having to guess at surrounding code, API contracts, or
> function signatures.
> 
> Tool calling is enabled by default with a limit of 10 rounds. Pass
> '--tool-rounds 0' to disable it and restore the previous single-shot
> behavior.  The round limit prevents runaway cost on large patches
> that when reached will force the model to deliver a final judgement.
> 
> Initial tool set:
>   - grep           Searches for regex across the file system with
>                    optional path restrictions and case-insensitive
>                    matches.
>   - file_read      Line range read of a specific path.
> 
> Both tools are limited to the repository root to prevent path
> traversal.  Path outputs are relative to the repo root.
> 
> The system prompt is extended when tool calling is active to
> encourage the model to use tools only when genuinely needed,
> keeping unnecessary round trips and token costs under control
> and to a minimum.
> 
> Internally, _common.py gains send_request_raw() (returning the
> raw response dict) so the tool-calling loops can inspect
> stop_reason / finish_reason before extracting text.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---

Well AI saw the security bypass here...

Error
=====

devtools/ai/review-patch.py: path containment check is bypassable
(_tool_grep and _tool_file_read)

	repo_resolved = Path(repo_root).resolve()
	search_path = (repo_resolved / rel_path).resolve()
	if not str(search_path).startswith(str(repo_resolved)):
		return "Error: path is outside the repository"

str.startswith() on the resolved path string is a prefix match, not a
path-component match. If repo_root resolves to /home/me/dpdk, then
/home/me/dpdk-private/secrets passes the check, since the string starts
with "/home/me/dpdk". The commit message states both tools are "limited
to the repository root to prevent path traversal" -- that property does
not hold for sibling directories sharing the name prefix. Same bug in
both tool helpers.

	# is_relative_to (3.9+), raises nothing, no string games
	if not search_path.is_relative_to(repo_resolved):
		return "Error: path is outside the repository"

(Absolute rel_path and ../ escapes are already caught by resolve() +
this fix; only the prefix case is currently open.)


Warning
=======

devtools/ai/_common.py: unrelated reformatting mixed into a feature
patch. The docstring re-wrapping in print_token_summary(),
_extract_text(), _print_verbose_usage(), and the classify_review()
regex reflow in review-patch.py are cosmetic churn unrelated to tool
calling. Drop them or split into a separate cleanup; they inflate the
diff and obscure the actual change.

devtools/ai/review-patch.py: grep --include list excludes the files the
tool prompt points the model at. _tool_grep restricts to
*.[ch]/*.py/*.rst/*.ini, but TOOL_PROMPT_EXTENSION explicitly directs
the model to check ".ci/" scripts, meson.build, and MAINTAINERS. Those
are extensionless or shell, so grep silently returns "No matches found"
rather than surfacing the omission. Either widen the include set or
note the restriction in the tool description so the model doesn't draw
false conclusions from empty results.


Info
====

xai is OpenAI wire-compatible and already shares build_openai_request()
elsewhere, but tool calling is gated to provider == "openai" in
call_api() and the verbose banner. xai (and any future
OpenAI-compatible provider) falls back to single-shot with no notice.
Reusing call_api_with_tools_openai() for xai looks free.

send_request_raw() bypasses _extract_text(), which is where the
"error" in result check lived. The tool loops read stop_reason /
choices directly, so a provider error returned with HTTP 200 yields an
empty string and a misleading "clean" review instead of a hard failure.
Worth a guard on api_result.get("error") at the top of each loop body.

Default-on at 10 rounds is a behavior and cost change for every existing
caller, and grants repo-wide read to the model by default. Reasonable
for an RFC to propose, but worth calling out explicitly for the list --
some CI users may want opt-in rather than opt-out.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox