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