Re: [PATCH dpdk v3] net: fix VLAN packet type

[Kevin Traynor <ktraynor@redhat.com>]

On 4/22/26 2:32 PM, Robin Jarry wrote:
> Since commit 1f250674085a ("net: fix packet type for stacked VLAN"),
> rte_net_get_ptype() uses |= to set the L2 ptype inside the VLAN
> parsing loop. Since pkt_type is already initialized with
> RTE_PTYPE_L2_ETHER (0x1), or-ing it with RTE_PTYPE_L2_ETHER_VLAN
> (0x6) results in RTE_PTYPE_L2_ETHER_QINQ (0x7). This causes single
> VLAN frames to be misidentified as QinQ.
> 
> This was detected while testing DPDK 25.11.1 in grout. The net/tap
> driver calls rte_net_get_ptype() in tap_verify_csum() to determine the
> L2 header length. With the wrong ptype, l2_len is set to 22 (ether
> + QinQ = 14 + 8) instead of 18 (ether + VLAN = 14 + 4), shifting the IP
> header pointer by 4 bytes. The checksum is then computed on garbage
> data, causing valid packets to be dropped.
> 
> Initialize pkt_type to 0 and defer the RTE_PTYPE_L2_ETHER assignment to
> the l3 label, only if no VLAN/QinQ type was set in the loop. This avoids
> the bitwise-or conflict between the L2 ptype constants entirely.
> 
> Add a new net_ptype_autotest unit test that verifies the ptype and
> header lengths (l2_len, l3_len, l4_len) returned by rte_net_get_ptype()
> for plain Ethernet, single VLAN, stacked VLAN (two 802.1Q tags), and
> QinQ (802.1ad + 802.1Q) frames, with both IPv4/IPv6 and UDP/TCP
> combinations.
> 
> Fixes: 1f250674085a ("net: fix packet type for stacked VLAN")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Robin Jarry <rjarry@redhat.com>

Hi Robin,

Thanks for reporting this.

> ---
> 
> Notes:
>     v3:
>     
>     * changed the approach: initialize pkt_type=0 and only set it to
>       RTE_PTYPE_L2_ETHER if neither of VLAN nor QINQ matched.
>     * extended the unit tests to check for header lengths and added ipv6 / tcp
>       cases.
>     
>     v2: added new ptype tests
> 
>  app/test/meson.build      |   1 +
>  app/test/test_net_ptype.c | 231 ++++++++++++++++++++++++++++++++++++++
>  lib/net/rte_net.c         |   4 +-
>  3 files changed, 235 insertions(+), 1 deletion(-)
>  create mode 100644 app/test/test_net_ptype.c
> 

<snip>
> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
> index 458b4814a9c9..0228f1eb2f18 100644
> --- a/lib/net/rte_net.c
> +++ b/lib/net/rte_net.c
> @@ -331,8 +331,8 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf *m,
>  	struct rte_net_hdr_lens local_hdr_lens;
>  	const struct rte_ether_hdr *eh;
>  	struct rte_ether_hdr eh_copy;
> -	uint32_t pkt_type = RTE_PTYPE_L2_ETHER;
>  	uint32_t off = 0, vlan_depth = 0;
> +	uint32_t pkt_type = 0;
>  	uint16_t proto;
>  	int ret;
>  
> @@ -392,6 +392,8 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf *m,
>  	}
>  

Not shown in diff, but for:

		pkt_type |=
			proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) ?
				 RTE_PTYPE_L2_ETHER_VLAN :
				 RTE_PTYPE_L2_ETHER_QINQ;

It seems to be produce the right result, but I'm not sure we should be
treating the ptype L2 defines as bitmasks. Maybe I'm wrong and it was
planned, but it looks like a coincidence it works now because QINQ (0x7)
happens to be a superset of VLAN (0x6) for the OR.

Perhaps you could check vlan_depth and assign VLAN or QINQ based on that?

thanks,
Kevin.

>  l3:
> +	if (pkt_type == 0)
> +		pkt_type = RTE_PTYPE_L2_ETHER;
>  	if ((layers & RTE_PTYPE_L3_MASK) == 0)
>  		return pkt_type;
>