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