From: Stephen Hemminger <stephen@networkplumber.org>
To: Robin Jarry <rjarry@redhat.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH dpdk v2] net/tap: use offsets provided by rte_net_get_ptype
Date: Tue, 28 Apr 2026 06:36:06 -0700 [thread overview]
Message-ID: <20260428063606.21b6ceb8@phoenix.local> (raw)
In-Reply-To: <20260427104152.1145165-2-rjarry@redhat.com>
On Mon, 27 Apr 2026 12:41:53 +0200
Robin Jarry <rjarry@redhat.com> wrote:
> Instead of guessing what are the proper header lengths, pass
> a rte_net_hdr_lens struct to rte_net_get_ptype and use it to get the
> proper header lengths/offsets in tap_verify_csum.
>
> This allows supporting stacked VLAN/QinQ tags and IPv6 extensions.
>
> Signed-off-by: Robin Jarry <rjarry@redhat.com>
> ---
AI review spotted some things.
But it looks like more of a bug in rte_net_get_ptype
Now let me check the upstream source for the IPv6 header context:Found the bug. Let me verify the IPv6 payload_len semantics:That confirms the bug. `payload_len` is documented as "IP payload size, including ext. headers" — exactly what I needed.
---
**Review of [PATCH dpdk v2] net/tap: use offsets provided by rte_net_get_ptype**
Error:
The IPv6 packet length check double-counts extension header bytes.
`rte_ipv6_hdr.payload_len` is defined as "IP payload size, including
ext. headers" (rte_ip6.h:487), meaning it already includes extension
header bytes counted from just after the 40-byte fixed IPv6 header.
But `hlen->l3_len` for `RTE_PTYPE_L3_IPV6_EXT` is set by
`rte_net_get_ptype` to `40 + extension_headers_size` (rte_net.c:439).
Adding them together counts the extension bytes twice:
if (hlen->l2_len + hlen->l3_len + rte_be_to_cpu_16(iph->payload_len) >
rte_pktmbuf_data_len(mbuf))
return;
For any IPv6 packet with extension headers this check will fire
spuriously, causing `tap_verify_csum` to return early without setting
checksum offload flags — the very case this patch is trying to fix.
The pre-existing code was correct: it used
`sizeof(struct rte_ipv6_hdr)` (40) as the L3 term, leaving
`payload_len` to account for extensions. The fix is to keep that
same constant here:
if (hlen->l2_len + sizeof(struct rte_ipv6_hdr) +
rte_be_to_cpu_16(iph->payload_len) >
rte_pktmbuf_data_len(mbuf))
return;
next prev parent reply other threads:[~2026-04-28 13:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-22 13:36 [PATCH dpdk] net/tap: use offsets provided by rte_net_get_ptype Robin Jarry
2026-04-22 16:32 ` Stephen Hemminger
2026-04-27 10:41 ` [PATCH dpdk v2] " Robin Jarry
2026-04-28 13:36 ` Stephen Hemminger [this message]
2026-04-30 23:28 ` [PATCH dpdk v3] " Robin Jarry
2026-05-03 3:29 ` Stephen Hemminger
2026-05-12 15:16 ` [PATCH dpdk v4] " Robin Jarry
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260428063606.21b6ceb8@phoenix.local \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=rjarry@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox