Re: Re: [PATCH v9 19/23] net/sxe2: add mbuf validation in Tx debug mode

["liujie5@linkdatatechnology.com" <liujie5@linkdatatechnology.com>]

[-- Attachment #1: Type: text/plain, Size: 4963 bytes --]

Hi Stephen,

Introduce the 'sxe2_txrx_check_mbuf' helper function to validate
outgoing mbuf tunnel type flags when RTE_ETHDEV_DEBUG_TX is enabled.
The function checks that the RTE_MBUF_F_TX_TUNNEL_x flag in mbuf
ol_flags matches the actual tunnel protocol detected in the packet
(GTP, VXLAN, VXLAN-GPE, Geneve, GRE, or IPIP).

The validation uses only UDP port matching and L4 protocol detection;
it does NOT re-derive header lengths or checksum fields already
verified by rte_validate_tx_offload() and rte_net_intel_cksum_prepare().

All code is wrapped inside RTE_ETHDEV_DEBUG_TX conditional compilation
to ensure zero overhead in production builds.


liujie5@linkdatatechnology.com
 
From: Stephen Hemminger
Date: 2026-06-27 02:21
To: liujie5
CC: dev
Subject: Re: [PATCH v9 19/23] net/sxe2: add mbuf validation in Tx debug mode
On Fri, 26 Jun 2026 14:47:51 +0800
liujie5@linkdatatechnology.com wrote:
 
> From: Jie Liu <liujie5@linkdatatechnology.com>
> 
> Introduce the `sxe2_txrx_check_mbuf` helper function to validate outgoing
> mbufs when `RTE_ETHDEV_DEBUG_TX` is enabled. This helps developers catch
> malformed mbufs (e.g., invalid segment lengths, bad offload flags, or
> unaligned buffers) before passing them to the hardware rings, avoiding
> potential hardware hangs or silent packet drops.
> 
> The validation is fully wrapped inside `RTE_ETHDEV_DEBUG_TX` conditional
> compilation blocks to ensure zero performance overhead in standard
> production builds.
> 
> Signed-off-by: Jie Liu <liujie5@linkdatatechnology.com>
> ---
 
I don't think I was clear enough before, the verbose description is:
 
 
Current state (v9):
 
The new sxe2_txrx_check_mbuf() helper added in 19/23 is called only
from sxe2_tx_pkts_prepare(), which is the driver's tx_prepare
callback. It is not called from any tx_burst path (sxe2_tx_pkts, the
vec paths, the poll path).
 
The commit message claims the validation is "fully wrapped inside
RTE_ETHDEV_DEBUG_TX conditional compilation blocks to ensure zero
performance overhead in standard production builds." This is not what
the code does. There is no #ifdef RTE_ETHDEV_DEBUG_TX anywhere in the
new file or around the new call site. In fact the patch *removes* an
existing #ifdef RTE_ETHDEV_DEBUG_TX that previously gated
rte_validate_tx_offload() in the same loop, making that always-on too.
 
The new function is marked __rte_unused both in the header
declaration and in the .c definition, even though it has exactly one
caller. That attribute was likely a leftover from an earlier draft
where the function was only conditionally referenced.
 
The validation itself (595 lines) re-derives outer/inner L2/L3/L4
lengths from the packet bytes and compares them against what the mbuf
header declares - i.e., it's verifying that the application
correctly set mbuf->l2_len, l3_len, outer_l2_len, etc. before
submitting. Much of this overlaps what rte_validate_tx_offload() and
rte_net_intel_cksum_prepare() already check, both of which are called
in the same tx_prepare loop.
 
Desired state:
 
tx_burst should not contain validation checks for application errors.
The fast path runs once per packet at line rate, and any branch that
exists to catch a caller bug is paid on every well-behaved packet
forever. For driver assumptions that an application must satisfy,
the three sensible options are:
 
  1. RTE_ASSERT() - compiles out in release builds, surfaces the
     assumption to static analyzers, and fires for users running
     debug builds.
 
  2. unlikely() with drop-and-counter - if the check is cheap and
     the assumption can be violated in production. The bad packet
     is dropped, a queue stat is incremented, but the burst loop
     continues.
 
  3. Document the assumption in the API contract and don't check it
     at runtime.
 
tx_prepare is where validation belongs, and an unconditional check
there is acceptable - tx_prepare is opt-in, and an application that
calls it has already agreed to pay for inspection. The 595-line
function as written is mostly fine in that context. What's wrong is:
 
  (a) The commit message says one thing and the code does another.
      The author should either add the #ifdef RTE_ETHDEV_DEBUG_TX
      they claimed to add, or rewrite the commit message to describe
      what the patch actually does.
 
  (b) Driver-specific tx_prepare validation should cover sxe2-
      specific hardware constraints (descriptor count limits, buffer
      alignment, segment size limits) - not redo the generic offload-
      flag and length-consistency checks that rte_validate_tx_offload()
      and rte_net_intel_cksum_prepare() perform. As currently written,
      every packet going through tx_prepare gets its offload flags
      validated roughly twice.
 
  (c) __rte_unused on the function should be dropped.
 
 
Redo this with one of the above and resubmit

[-- Attachment #2: Type: text/html, Size: 7959 bytes --]