|SUCCESS| [v3] net/tap: use offsets provided by rte_net_get_ptype

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-01

Commit: 20260430232806.125220-2-rjarry at redhat.com
Author: Robin Jarry <rjarry at redhat.com>
Subject: [PATCH dpdk v3] net/tap: use offsets provided by rte_net_get_ptype

This patch modifies tap_verify_csum() to use the rte_net_hdr_lens
structure populated by rte_net_get_ptype() instead of manually
calculating header offsets. This enables support for stacked VLAN/QinQ
tags and IPv6 extensions.

> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index a5d460a0b3cb..f807697aa6a8 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c

[ ... ]

> @@ -327,79 +327,66 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive, int persistent)
>  }
>  
>  static void
> -tap_verify_csum(struct rte_mbuf *mbuf)
> +tap_verify_csum(struct rte_mbuf *mbuf, const struct rte_net_hdr_lens *hlen)
>  {
> -	uint32_t l2 = mbuf->packet_type & RTE_PTYPE_L2_MASK;
>  	uint32_t l3 = mbuf->packet_type & RTE_PTYPE_L3_MASK;
>  	uint32_t l4 = mbuf->packet_type & RTE_PTYPE_L4_MASK;
> -	unsigned int l2_len = sizeof(struct rte_ether_hdr);
> -	unsigned int l3_len;
> +	uint32_t l4_off = hlen->l2_len + hlen->l3_len;
>  	uint16_t cksum = 0;
>  	void *l3_hdr;
>  	void *l4_hdr;
> -	struct rte_udp_hdr *udp_hdr;

[ ... ]

> +	} else if (l3 == RTE_PTYPE_L3_IPV6 || l3 == RTE_PTYPE_L3_IPV6_EXT) {
>  		struct rte_ipv6_hdr *iph = l3_hdr;
>  
> -		l3_len = sizeof(struct rte_ipv6_hdr);
>  		/* check that the total length reported by header is not
>  		 * greater than the total received size
>  		 */
> -		if (l2_len + l3_len + rte_be_to_cpu_16(iph->payload_len) >
> +		if (hlen->l2_len + sizeof(*iph) + rte_be_to_cpu_16(iph->payload_len) >
>  				rte_pktmbuf_data_len(mbuf))
>  			return;

Does this code correctly handle IPv6 extension headers? The comment in
v3 notes indicates the double count issue was fixed, but the calculation
appears incorrect. For IPv6, the payload_len field does not include the
base header length. When IPv6 extensions are present, hlen->l3_len will
include both the base header and extensions, but this validation
computes: base_header + base_header + extensions + payload_len. Should
this be hlen->l2_len + hlen->l3_len +
rte_be_to_cpu_16(iph->payload_len)?

>  	} else {
>  		/* - RTE_PTYPE_L3_IPV4_EXT_UNKNOWN cannot happen because
>  		 *   mbuf->packet_type is filled by rte_net_get_ptype() which
>  		 *   never returns this value.
> -		 * - IPv6 extensions are not supported.
>  		 */
>  		return;
>  	}

[ ... ]

> @@ -561,10 +548,11 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  			continue;
>  		}
>  
> -		mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> +		struct rte_net_hdr_lens hlen = {0};

Is this variable declaration placement compliant with C89? The DPDK
coding style requires C89 compatibility, which mandates variable
declarations at the beginning of the block scope. This declaration
appears in the middle of the pmd_rx_burst() function after the
continue statement.

> +		mbuf->packet_type = rte_net_get_ptype(mbuf, &hlen,
>  						      RTE_PTYPE_ALL_MASK);
>  		if (rxq->rxmode->offloads & RTE_ETH_RX_OFFLOAD_CHECKSUM)
> -			tap_verify_csum(mbuf);
> +			tap_verify_csum(mbuf, &hlen);
>  
>  		/* account for the receive frame */
>  		bufs[num_rx++] = mbuf;