All of 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 v5] net/tap: use offsets provided by rte_net_get_ptype
Date: Mon, 18 May 2026 10:52:45 -0700	[thread overview]
Message-ID: <20260518105245.237e7b22@phoenix.local> (raw)
In-Reply-To: <20260518082659.13036-2-rjarry@redhat.com>

On Mon, 18 May 2026 10:27:00 +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>

Still needs some polishing around Ipv6 ext headers
per AI review...

[PATCH dpdk v5] net/tap: use offsets provided by rte_net_get_ptype

The refactor is mechanically correct. rte_net_get_ptype() with a
non-NULL rte_net_hdr_lens fills l2_len walking the full VLAN/QinQ
stack (up to RTE_NET_VLAN_MAX_DEPTH), and for IPv4 sets
l3_len = rte_ipv4_hdr_len(ip4h). Replacing the local l2_len/l3_len
arithmetic with hlen->* preserves the existing bounds checks and
genuinely fixes the stacked-VLAN case (the old code only handled
single VLAN +4 or QinQ +8). The unnecessary cast of l4_hdr to
struct rte_udp_hdr * was correctly dropped. The {0} initializer on
the rte_net_hdr_lens stack variable is appropriate since
rte_net_get_ptype only writes the fields it parses.

Info:

  - The commit message says "This allows supporting stacked
    VLAN/QinQ tags and IPv6 extensions." The first half is true.
    The IPv6-extensions half is not: the else branch that catches
    RTE_PTYPE_L3_IPV6_EXT and returns is unchanged, and the
    accompanying comment "IPv6 extensions are not supported." in
    that branch is also unchanged. For an IPv6 packet with
    extension headers, rte_net_get_ptype() will set
    RTE_PTYPE_L3_IPV6_EXT (not RTE_PTYPE_L3_IPV6), so the
    else-if branch is skipped and tap_verify_csum() returns
    without verifying L3 or L4 checksums. The hlen now carries
    the correct l3_len including extensions, so this is a
    prerequisite for IPv6-ext support but not the feature itself.
    Suggest rewording to "This is a prerequisite for IPv6
    extension header support" or dropping that half of the
    sentence.

  - drivers/net/tap/rte_eth_tap.c:549: minor scoping observation
    only (no action needed) -- the hlen struct is filled
    unconditionally on every received packet whereas previously
    rte_net_get_ptype() was called with NULL when offload check
    was disabled. The cost is a handful of stack writes per
    packet; consistent with how other PMDs handle this.

  reply	other threads:[~2026-05-18 17:52 UTC|newest]

Thread overview: 12+ 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
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
2026-05-18  3:14   ` Stephen Hemminger
2026-05-18  8:27 ` [PATCH dpdk v5] " Robin Jarry
2026-05-18 17:52   ` Stephen Hemminger [this message]
2026-05-18 20:54 ` [PATCH dpdk v6] " Robin Jarry
2026-05-18 22:26   ` Stephen Hemminger

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=20260518105245.237e7b22@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.