|SUCCESS| net: fix L2 ptype assignment in VLAN loop

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163232

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-22

TodoWrite: Analyzing all hunks in the diff to determine relevance to review comments.

Commit: 1f250674085a
Author: Robin Jarry <rjarry at redhat.com>
Subject: [PATCH dpdk] net: fix L2 ptype assignment in VLAN loop

This patch fixes a bug where single VLAN frames are misidentified as
QinQ due to incorrect use of the |= operator when setting the L2 packet
type. The fix changes the operator to a simple assignment, preventing
the accumulation of flags that causes RTE_PTYPE_L2_ETHER_VLAN (0x6) to
be or-ed with RTE_PTYPE_L2_ETHER (0x1), resulting in
RTE_PTYPE_L2_ETHER_QINQ (0x7).

> diff --git a/app/test/test_cksum.c b/app/test/test_cksum.c
> index ea443382a128..3cc42eccedd9 100644
> --- a/app/test/test_cksum.c
> +++ b/app/test/test_cksum.c
> @@ -75,6 +75,27 @@ static const char test_cksum_ipv6_udp[] = {
>  	0x00, 0x35, 0x00, 0x09, 0x87, 0x70, 0x78,
>  };
>  
> +/* generated in scapy with Ether()/Dot1Q(vlan=42)/IP()/UDP()/Raw('x') */
> +static const char test_cksum_vlan_ipv4_udp[] = {

Does this violate the coding standard's requirement for lowercase hex
digits in initializers? The existing test packets above use lowercase
0xff style, but I see this patch follows that pattern already.

> +	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x81, 0x00, 0x00, 0x2a,
> +	0x08, 0x00, 0x45, 0x00, 0x00, 0x1d, 0x00, 0x01,
> +	0x00, 0x00, 0x40, 0x11, 0x7c, 0xcd, 0x7f, 0x00,
> +	0x00, 0x01, 0x7f, 0x00, 0x00, 0x01, 0x00, 0x35,
> +	0x00, 0x35, 0x00, 0x09, 0x89, 0x6f, 0x78,
> +};
> +
> +/* generated in scapy with Ether()/Dot1AD(vlan=42)/Dot1Q(vlan=43)/IP()/UDP()/Raw('x') */
> +static const char test_cksum_qinq_ipv4_udp[] = {
> +	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00,
> +	0x00, 0x00, 0x00, 0x00, 0x88, 0xa8, 0x00, 0x2a,
> +	0x81, 0x00, 0x00, 0x2b, 0x08, 0x00, 0x45, 0x00,
> +	0x00, 0x1d, 0x00, 0x01, 0x00, 0x00, 0x40, 0x11,
> +	0x7c, 0xcd, 0x7f, 0x00, 0x00, 0x01, 0x7f, 0x00,
> +	0x00, 0x01, 0x00, 0x35, 0x00, 0x35, 0x00, 0x09,
> +	0x89, 0x6f, 0x78,
> +};
> +

[ ... ]

> @@ -252,6 +273,14 @@ test_cksum(void)
>  			  sizeof(test_cksum_ipv6_udp)) < 0)
>  		GOTO_FAIL("checksum error on ipv6_udp");
>  
> +	if (test_l4_cksum(pktmbuf_pool, test_cksum_vlan_ipv4_udp,
> +			  sizeof(test_cksum_vlan_ipv4_udp)) < 0)
> +		GOTO_FAIL("checksum error on vlan_ipv4_udp");
> +
> +	if (test_l4_cksum(pktmbuf_pool, test_cksum_qinq_ipv4_udp,
> +			  sizeof(test_cksum_qinq_ipv4_udp)) < 0)
> +		GOTO_FAIL("checksum error on qinq_ipv4_udp");
> +
>  	if (test_l4_cksum(pktmbuf_pool, test_cksum_ipv4_opts_udp,
>  			  sizeof(test_cksum_ipv4_opts_udp)) < 0)
>  		GOTO_FAIL("checksum error on ipv4_opts_udp");
> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
> index 458b4814a9c9..ea5ba7019089 100644
> --- a/lib/net/rte_net.c
> +++ b/lib/net/rte_net.c
> @@ -359,7 +359,7 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf *m,
>  
>  		if (++vlan_depth > RTE_NET_VLAN_MAX_DEPTH)
>  			return 0;
> -		pkt_type |=
> +		pkt_type =
>  			proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) ?
>  				 RTE_PTYPE_L2_ETHER_VLAN :
>  				 RTE_PTYPE_L2_ETHER_QINQ;

The fix looks correct. In the case of a single VLAN tag, pkt_type was
previously initialized to RTE_PTYPE_L2_ETHER (0x1) earlier in
rte_net_get_ptype(). Or-ing with RTE_PTYPE_L2_ETHER_VLAN (0x6) produced
0x7 which equals RTE_PTYPE_L2_ETHER_QINQ, causing the misidentification.
The simple assignment correctly replaces the L2 type.