DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code
@ 2026-08-24 19:22 William Bland
  2026-08-24 19:22 ` [PATCH 2/3] net: add VLAN insert function with a TPID argument William Bland
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: William Bland @ 2026-08-24 19:22 UTC (permalink / raw)
  To: Changchun Ouyang, Huawei Xie, Stephen Hemminger; +Cc: dev, William Bland

The implementation returns -EINVAL when the mbuf is shared or
indirect, but the doc comment inaccurately stated -EPERM.

Fixes: c974021a5949 ("ether: add soft vlan encap/decap")

Signed-off-by: William Bland <blandwwa@gmail.com>
---
 lib/net/rte_ether.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
index c9a0b536c3..cb10d8fb06 100644
--- a/lib/net/rte_ether.h
+++ b/lib/net/rte_ether.h
@@ -387,7 +387,8 @@ static inline int rte_vlan_strip(struct rte_mbuf *m)
  *   The packet mbuf.
  * @return
  *   - 0: On success
- *   -EPERM: mbuf is shared overwriting would be unsafe
+ *   -EINVAL: overwriting would be unsafe because mbuf is shared or
+ *            indirect, or mbuf's first segment is too short
  *   -ENOSPC: not enough headroom in mbuf
  */
 static inline int rte_vlan_insert(struct rte_mbuf **m)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] net: add VLAN insert function with a TPID argument
  2026-08-24 19:22 [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code William Bland
@ 2026-08-24 19:22 ` William Bland
  2026-08-24 19:22 ` [PATCH 3/3] net/af_packet: fix QinQ outer TPID on VLAN reinsertion William Bland
  2026-08-24 20:59 ` [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code Stephen Hemminger
  2 siblings, 0 replies; 4+ messages in thread
From: William Bland @ 2026-08-24 19:22 UTC (permalink / raw)
  Cc: dev, William Bland

rte_vlan_insert() hardcodes EtherType 0x8100 (802.1Q). This is wrong
when reinserting a stripped tag whose original TPID was 0x88a8 (802.1ad
QinQ).

Add rte_vlan_insert_tpid() which takes an explicit tpid parameter, and
rewrite rte_vlan_insert() as a wrapper that passes RTE_ETHER_TYPE_VLAN.

Signed-off-by: William Bland <blandwwa@gmail.com>
---
 lib/net/rte_ether.h | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
index cb10d8fb06..1410ec1968 100644
--- a/lib/net/rte_ether.h
+++ b/lib/net/rte_ether.h
@@ -379,19 +379,21 @@ static inline int rte_vlan_strip(struct rte_mbuf *m)
 }
 
 /**
- * Insert VLAN tag into mbuf.
+ * Insert VLAN tag with the given TPID into mbuf.
  *
- * Software version of VLAN unstripping
+ * Software version of VLAN unstripping.
  *
  * @param m
  *   The packet mbuf.
+ * @param tpid
+ *   Tag Protocol Identifier to insert (host order).
  * @return
  *   - 0: On success
  *   -EINVAL: overwriting would be unsafe because mbuf is shared or
  *            indirect, or mbuf's first segment is too short
  *   -ENOSPC: not enough headroom in mbuf
  */
-static inline int rte_vlan_insert(struct rte_mbuf **m)
+static inline int rte_vlan_insert_tpid(struct rte_mbuf **m, uint16_t tpid)
 {
 	struct rte_ether_hdr *oh, *nh;
 	struct rte_vlan_hdr *vh;
@@ -411,7 +413,7 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
 		return -ENOSPC;
 
 	memmove(nh, oh, 2 * RTE_ETHER_ADDR_LEN);
-	nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN);
+	nh->ether_type = rte_cpu_to_be_16(tpid);
 
 	vh = (struct rte_vlan_hdr *) (nh + 1);
 	vh->vlan_tci = rte_cpu_to_be_16((*m)->vlan_tci);
@@ -426,6 +428,26 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
 	return 0;
 }
 
+/**
+ * Insert VLAN tag into mbuf.
+ *
+ * Software version of VLAN unstripping. Always inserts an 802.1Q tag
+ * (TPID 0x8100). Use rte_vlan_insert_tpid() when the original TPID may
+ * differ (e.g. 802.1ad QinQ outer tags).
+ *
+ * @param m
+ *   The packet mbuf.
+ * @return
+ *   - 0: On success
+ *   -EINVAL: overwriting would be unsafe because mbuf is shared or
+ *            indirect, or mbuf's first segment is too short
+ *   -ENOSPC: not enough headroom in mbuf
+ */
+static inline int rte_vlan_insert(struct rte_mbuf **m)
+{
+	return rte_vlan_insert_tpid(m, RTE_ETHER_TYPE_VLAN);
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] net/af_packet: fix QinQ outer TPID on VLAN reinsertion
  2026-08-24 19:22 [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code William Bland
  2026-08-24 19:22 ` [PATCH 2/3] net: add VLAN insert function with a TPID argument William Bland
@ 2026-08-24 19:22 ` William Bland
  2026-08-24 20:59 ` [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code Stephen Hemminger
  2 siblings, 0 replies; 4+ messages in thread
From: William Bland @ 2026-08-24 19:22 UTC (permalink / raw)
  To: Stephen Hemminger, Ferruh Yigit, Chas Williams; +Cc: dev, William Bland

eth_af_packet_rx() called rte_vlan_insert(), which hardcodes EtherType
0x8100 (802.1Q). This was wrong when reinserting a stripped tag whose
original TPID was 0x88a8 (802.1ad QinQ).

Fix by reading the actual TPID from tp_vlan_tpid (struct tpacket2_hdr),
then passing it to rte_vlan_insert_tpid().

Fixes: 23deeebfcfa8 ("net/af_packet: support 802.1Q VLAN")

Signed-off-by: William Bland <blandwwa@gmail.com>
---
 app/test/meson.build                      |   2 +-
 app/test/test_net_ether.c                 | 122 ++++++++++++++++++++++
 drivers/net/af_packet/rte_eth_af_packet.c |  10 +-
 3 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 51abeeb732..e927d4306a 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -134,7 +134,7 @@ source_file_deps = {
     'test_meter.c': ['meter'],
     'test_metrics.c': ['metrics'],
     'test_mp_secondary.c': ['hash'],
-    'test_net_ether.c': ['net'],
+    'test_net_ether.c': ['net', 'mbuf'],
     'test_net_ip6.c': ['net'],
     'test_pcapng.c': ['net_null', 'net', 'ethdev', 'pcapng', 'bus_vdev'],
     'test_pdcp.c': ['eventdev', 'pdcp', 'net', 'timer', 'security'],
diff --git a/app/test/test_net_ether.c b/app/test/test_net_ether.c
index ec11224171..19f9c85e5b 100644
--- a/app/test/test_net_ether.c
+++ b/app/test/test_net_ether.c
@@ -3,6 +3,8 @@
  */
 
 #include <rte_ether.h>
+#include <rte_mbuf.h>
+#include <rte_mempool.h>
 
 #include <rte_test.h>
 #include "test.h"
@@ -162,4 +164,124 @@ test_net_ether(void)
 	return 0;
 }
 
+/*
+ * Build a minimal Ethernet frame in an mbuf: Ethernet header with the given
+ * ether_type followed by payload_len zero bytes.
+ */
+static struct rte_mbuf *
+alloc_frame(struct rte_mempool *mp, uint16_t ether_type, uint16_t payload_len)
+{
+	struct rte_ether_hdr *eh;
+	struct rte_mbuf *m;
+
+	m = rte_pktmbuf_alloc(mp);
+	if (m == NULL)
+		return NULL;
+
+	eh = (struct rte_ether_hdr *)rte_pktmbuf_append(m,
+			sizeof(*eh) + payload_len);
+	if (eh == NULL) {
+		rte_pktmbuf_free(m);
+		return NULL;
+	}
+
+	memset(eh->dst_addr.addr_bytes, 0xff, RTE_ETHER_ADDR_LEN);
+	memset(eh->src_addr.addr_bytes, 0x00, RTE_ETHER_ADDR_LEN);
+	eh->ether_type = rte_cpu_to_be_16(ether_type);
+
+	return m;
+}
+
+static int
+test_vlan_insert_8021q(struct rte_mempool *mp)
+{
+	struct rte_ether_hdr *eh;
+	struct rte_vlan_hdr *vh;
+	struct rte_mbuf *m;
+	int ret;
+
+	m = alloc_frame(mp, RTE_ETHER_TYPE_IPV4, 46);
+	TEST_ASSERT_NOT_NULL(m, "Failed to allocate mbuf");
+
+	m->vlan_tci = 100;
+	m->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED;
+
+	ret = rte_vlan_insert(&m);
+	TEST_ASSERT_SUCCESS(ret, "rte_vlan_insert failed");
+
+	eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	TEST_ASSERT_EQUAL(rte_be_to_cpu_16(eh->ether_type), RTE_ETHER_TYPE_VLAN,
+		"Expected 802.1Q TPID 0x%04x, got 0x%04x",
+		RTE_ETHER_TYPE_VLAN, rte_be_to_cpu_16(eh->ether_type));
+
+	vh = (struct rte_vlan_hdr *)(eh + 1);
+	TEST_ASSERT_EQUAL(rte_be_to_cpu_16(vh->vlan_tci), 100,
+		"Expected VID 100, got %u", rte_be_to_cpu_16(vh->vlan_tci));
+
+	rte_pktmbuf_free(m);
+	return TEST_SUCCESS;
+}
+
+static int
+test_vlan_insert_tpid(struct rte_mempool *mp)
+{
+	struct rte_ether_hdr *eh;
+	struct rte_vlan_hdr *vh;
+	struct rte_mbuf *m;
+	int ret;
+
+	m = alloc_frame(mp, RTE_ETHER_TYPE_VLAN, sizeof(*vh) + 46);
+	TEST_ASSERT_NOT_NULL(m, "Failed to allocate mbuf");
+
+	vh = (struct rte_vlan_hdr *)(rte_pktmbuf_mtod(m, struct rte_ether_hdr *) + 1);
+	vh->vlan_tci = rte_cpu_to_be_16(200);
+	vh->eth_proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
+
+	m->vlan_tci = 50;
+	m->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED;
+
+	ret = rte_vlan_insert_tpid(&m, RTE_ETHER_TYPE_QINQ);
+	TEST_ASSERT_SUCCESS(ret, "rte_vlan_insert_tpid failed");
+
+	eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+
+	TEST_ASSERT_EQUAL(rte_be_to_cpu_16(eh->ether_type), RTE_ETHER_TYPE_QINQ,
+		"Outer TPID: expected 0x%04x (802.1ad) got 0x%04x",
+		RTE_ETHER_TYPE_QINQ, rte_be_to_cpu_16(eh->ether_type));
+
+	vh = (struct rte_vlan_hdr *)(eh + 1);
+	TEST_ASSERT_EQUAL(rte_be_to_cpu_16(vh->vlan_tci), 50,
+		"Outer VID: expected 50, got %u",
+		rte_be_to_cpu_16(vh->vlan_tci));
+
+	rte_pktmbuf_free(m);
+	return TEST_SUCCESS;
+}
+
+static int
+test_vlan_insert(void)
+{
+	struct rte_mempool *mp;
+	int ret;
+
+	mp = rte_pktmbuf_pool_create("vlan_insert_test_pool", 64, 0, 0,
+				     RTE_MBUF_DEFAULT_BUF_SIZE,
+				     SOCKET_ID_ANY);
+	if (mp == NULL) {
+		fprintf(stderr, "Failed to create mempool\n");
+		return -1;
+	}
+
+	ret = test_vlan_insert_8021q(mp);
+	if (ret != TEST_SUCCESS)
+		goto out;
+
+	ret = test_vlan_insert_tpid(mp);
+
+out:
+	rte_mempool_free(mp);
+	return ret;
+}
+
 REGISTER_FAST_TEST(net_ether_autotest, NOHUGE_OK, ASAN_OK, test_net_ether);
+REGISTER_FAST_TEST(vlan_insert_autotest, NOHUGE_OK, ASAN_OK, test_vlan_insert);
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index b0ff22ea55..8225e0d7f9 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -222,11 +222,17 @@ eth_af_packet_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 
 		/* check for vlan info */
 		if (ppd->tp_status & TP_STATUS_VLAN_VALID) {
+			uint16_t tpid;
+
 			mbuf->vlan_tci = ppd->tp_vlan_tci;
 			mbuf->ol_flags |= (RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED);
 
-			if (!pkt_q->vlan_strip && rte_vlan_insert(&mbuf))
-				PMD_LOG(ERR, "Failed to reinsert VLAN tag");
+			if (!pkt_q->vlan_strip) {
+				tpid = (ppd->tp_status & TP_STATUS_VLAN_TPID_VALID) ?
+					ppd->tp_vlan_tpid : RTE_ETHER_TYPE_VLAN;
+				if (rte_vlan_insert_tpid(&mbuf, tpid))
+					PMD_LOG(ERR, "Failed to reinsert VLAN tag");
+			}
 		}
 
 		/* add kernel provided timestamp when offloading is enabled */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code
  2026-08-24 19:22 [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code William Bland
  2026-08-24 19:22 ` [PATCH 2/3] net: add VLAN insert function with a TPID argument William Bland
  2026-08-24 19:22 ` [PATCH 3/3] net/af_packet: fix QinQ outer TPID on VLAN reinsertion William Bland
@ 2026-08-24 20:59 ` Stephen Hemminger
  2 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2026-08-24 20:59 UTC (permalink / raw)
  To: William Bland; +Cc: Changchun Ouyang, Huawei Xie, dev

On Mon, 24 Aug 2026 15:22:25 -0400
William Bland <blandwwa@gmail.com> wrote:

> The implementation returns -EINVAL when the mbuf is shared or
> indirect, but the doc comment inaccurately stated -EPERM.
> 
> Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
> 
> Signed-off-by: William Bland <blandwwa@gmail.com>
> ---
>  lib/net/rte_ether.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
> index c9a0b536c3..cb10d8fb06 100644
> --- a/lib/net/rte_ether.h
> +++ b/lib/net/rte_ether.h
> @@ -387,7 +387,8 @@ static inline int rte_vlan_strip(struct rte_mbuf *m)
>   *   The packet mbuf.
>   * @return
>   *   - 0: On success
> - *   -EPERM: mbuf is shared overwriting would be unsafe
> + *   -EINVAL: overwriting would be unsafe because mbuf is shared or
> + *            indirect, or mbuf's first segment is too short
>   *   -ENOSPC: not enough headroom in mbuf
>   */
>  static inline int rte_vlan_insert(struct rte_mbuf **m)

This patch series is good and addresses bugs. But I would like it
to go a little further by:

1. Add cc: stable@dpdk.org to bugfixes
2. Do a little more refactoring so the new function can be marked experimental
3. Be more careful in the af_packet pkt_strip flag handling
4. Use test.h unit_test_suite_runner() pattern in existing test.

Long winded AI description of same stuff...

Review: [PATCH 0/3] VLAN insert with TPID / af_packet QinQ fix

Series applies cleanly on main (d55ccd4). All three Fixes: hashes
resolve. No correctness issues in the header change or the tests;
one pre-existing flag-handling problem in the af_packet Rx path is
touched by patch 3 and should be fixed separately (see below).

Patch 1/3: net: fix VLAN insert doc comment for shared-mbuf error code

Warning: Missing Cc: stable@dpdk.org. This is a fix (has a Fixes:
tag) for text that shipped in every stable branch.

Patch 2/3: net: add VLAN insert function with a TPID argument

Warning: rte_vlan_insert_tpid() is new public API and is not marked
__rte_experimental. The precedent in the same library is
rte_ipv4_cksum_simple() in rte_ip4.h, which carries the
"@warning @b EXPERIMENTAL" Doxygen note and the attribute.

Note the complication: as structured, the stable rte_vlan_insert()
wrapper calls rte_vlan_insert_tpid(). If the latter is marked
experimental, every translation unit that includes rte_ether.h
without ALLOW_EXPERIMENTAL_API gets the deprecated-attribute warning
from inside the wrapper body, which breaks existing users of the
stable function under -Werror. Suggested structure: move the body
into an unmarked helper and have both public functions call it.

	static inline int
	__rte_vlan_insert(struct rte_mbuf **m, uint16_t tpid)
	{
		/* current body */
	}

	static inline int rte_vlan_insert(struct rte_mbuf **m)
	{
		return __rte_vlan_insert(m, RTE_ETHER_TYPE_VLAN);
	}

	__rte_experimental
	static inline int
	rte_vlan_insert_tpid(struct rte_mbuf **m, uint16_t tpid)
	{
		return __rte_vlan_insert(m, tpid);
	}

Warning: No release notes entry. A new public function in lib/net
needs a line under "New Features" in
doc/guides/rel_notes/release_26_11.rst.

Warning: The unit tests for rte_vlan_insert() and
rte_vlan_insert_tpid() (app/test/test_net_ether.c, app/test/meson.build)
are added in patch 3, the af_packet fix. They test the library API
introduced here and should be part of this patch. That also keeps
the driver fix self-contained for backporting.

Patch 3/3: net/af_packet: fix QinQ outer TPID on VLAN reinsertion

Warning: The Rx VLAN block sets RTE_MBUF_F_RX_VLAN_STRIPPED
unconditionally and then, when vlan_strip is off, relies on
rte_vlan_insert_tpid() to clear it again. The non-strip path only
ends up with correct flags because the insert helper undoes the
flag; if the insert fails the mbuf is delivered with STRIPPED set
and an error logged. This dates from d41d39bcf7 ("net/af_packet:
reinsert stripped VLAN tag"), not from this patch, but this patch
reshapes exactly that block and is the place to sort it out. Split
it: one patch that fixes the flag handling (Fixes: d41d39bcf7,
Cc: stable), then the TPID change on top. Suggested shape for the
first:

	if (ppd->tp_status & TP_STATUS_VLAN_VALID) {
		mbuf->vlan_tci = ppd->tp_vlan_tci;
		mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN;

		if (pkt_q->vlan_strip) {
			mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN_STRIPPED;
		} else if (rte_vlan_insert(&mbuf) != 0) {
			PMD_LOG(ERR, "Failed to reinsert VLAN tag");
			mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN_STRIPPED;
		}
	}

Warning: Missing Cc: stable@dpdk.org on a bug fix. Because the fix
depends on the new function from patch 2, a backport will need that
patch as well; worth stating in the commit message so the stable
maintainers know to pick both.

Warning: The tests add a second REGISTER_FAST_TEST
(vlan_insert_autotest) to test_net_ether.c alongside
net_ether_autotest. Use unit_test_suite_runner() instead: convert
the existing test_net_ether() into a suite with TEST_CASE entries
for the four address tests, add the two VLAN insert cases, and
create/free the mempool in the suite's .setup/.teardown so the
individual cases do not need a mempool parameter. That also
replaces the fprintf()/return -1 error reporting in
test_vlan_insert() with the TEST_ASSERT macros used elsewhere in
the file.

Info: test_vlan_insert_tpid() builds a frame that already carries an
802.1Q tag and then inserts an 802.1ad outer tag, but only checks
the new outer header. Checking that the pushed-down inner tag
survived (new vh->eth_proto == RTE_ETHER_TYPE_VLAN, and the following
rte_vlan_hdr still has TCI 200 / eth_proto IPv4) is what actually
exercises the QinQ case the test is named for.

Info: alloc_frame() comment says the header is "followed by
payload_len zero bytes" but rte_pktmbuf_append() does not zero the
data; either memset the payload or drop "zero" from the comment.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-24 20:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 19:22 [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code William Bland
2026-08-24 19:22 ` [PATCH 2/3] net: add VLAN insert function with a TPID argument William Bland
2026-08-24 19:22 ` [PATCH 3/3] net/af_packet: fix QinQ outer TPID on VLAN reinsertion William Bland
2026-08-24 20:59 ` [PATCH 1/3] net: fix VLAN insert doc comment for shared-mbuf error code Stephen Hemminger

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