DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
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;

  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