<html><head><meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"><style>body { line-height: 1.5; }blockquote { margin-top: 0px; margin-bottom: 0px; margin-left: 0.5em; }.fox_html_content_dummy_textarea { position: absolute; overflow: hidden; width: 1px; height: 1px; opacity: 0; }body { font-size: 14px; font-family: "Microsoft YaHei UI"; color: rgb(0, 0, 0); line-height: 1.5; }</style></head><body>
<div><span style="background-color: transparent;">Hi Stephen,</span><span></span></div><div><br></div><div><div>Introduce the 'sxe2_txrx_check_mbuf' helper function to validate</div><div>outgoing mbuf tunnel type flags when RTE_ETHDEV_DEBUG_TX is enabled.</div><div>The function checks that the RTE_MBUF_F_TX_TUNNEL_x flag in mbuf</div><div>ol_flags matches the actual tunnel protocol detected in the packet</div><div>(GTP, VXLAN, VXLAN-GPE, Geneve, GRE, or IPIP).</div><div><br></div><div>The validation uses only UDP port matching and L4 protocol detection;</div><div>it does NOT re-derive header lengths or checksum fields already</div><div>verified by rte_validate_tx_offload() and rte_net_intel_cksum_prepare().</div><div><br></div><div>All code is wrapped inside RTE_ETHDEV_DEBUG_TX conditional compilation</div><div>to ensure zero overhead in production builds.</div></div><hr style="width: 210px; height: 1px;" color="#b5c4df" size="1" align="left">
<div><span><div style="MARGIN: 10px; FONT-FAMILY: verdana; FONT-SIZE: 10pt"><div>liujie5@linkdatatechnology.com</div></div></span></div>
<blockquote style="margin-Top: 0px; margin-Bottom: 0px; margin-Left: 0.5em; margin-Right: inherit"><div> </div><div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm"><div style="PADDING-RIGHT: 8px; PADDING-LEFT: 8px; FONT-SIZE: 12px;FONT-FAMILY:tahoma;COLOR:#000000; BACKGROUND: #efefef; PADDING-BOTTOM: 8px; PADDING-TOP: 8px"><div><b>From:</b> <a href="mailto:stephen@networkplumber.org">Stephen Hemminger</a></div><div><b>Date:</b> 2026-06-27 02:21</div><div><b>To:</b> <a href="mailto:liujie5@linkdatatechnology.com">liujie5</a></div><div><b>CC:</b> <a href="mailto:dev@dpdk.org">dev</a></div><div><b>Subject:</b> Re: [PATCH v9 19/23] net/sxe2: add mbuf validation in Tx debug mode</div></div></div><div><div>On Fri, 26 Jun 2026 14:47:51 +0800</div>
<div>liujie5@linkdatatechnology.com wrote:</div>
<div> </div>
<div>> From: Jie Liu <liujie5@linkdatatechnology.com></div>
<div>> </div>
<div>> Introduce the `sxe2_txrx_check_mbuf` helper function to validate outgoing</div>
<div>> mbufs when `RTE_ETHDEV_DEBUG_TX` is enabled. This helps developers catch</div>
<div>> malformed mbufs (e.g., invalid segment lengths, bad offload flags, or</div>
<div>> unaligned buffers) before passing them to the hardware rings, avoiding</div>
<div>> potential hardware hangs or silent packet drops.</div>
<div>> </div>
<div>> The validation is fully wrapped inside `RTE_ETHDEV_DEBUG_TX` conditional</div>
<div>> compilation blocks to ensure zero performance overhead in standard</div>
<div>> production builds.</div>
<div>> </div>
<div>> Signed-off-by: Jie Liu <liujie5@linkdatatechnology.com></div>
<div>> ---</div>
<div> </div>
<div>I don't think I was clear enough before, the verbose description is:</div>
<div> </div>
<div> </div>
<div>Current state (v9):</div>
<div> </div>
<div>The new sxe2_txrx_check_mbuf() helper added in 19/23 is called only</div>
<div>from sxe2_tx_pkts_prepare(), which is the driver's tx_prepare</div>
<div>callback. It is not called from any tx_burst path (sxe2_tx_pkts, the</div>
<div>vec paths, the poll path).</div>
<div> </div>
<div>The commit message claims the validation is "fully wrapped inside</div>
<div>RTE_ETHDEV_DEBUG_TX conditional compilation blocks to ensure zero</div>
<div>performance overhead in standard production builds." This is not what</div>
<div>the code does. There is no #ifdef RTE_ETHDEV_DEBUG_TX anywhere in the</div>
<div>new file or around the new call site. In fact the patch *removes* an</div>
<div>existing #ifdef RTE_ETHDEV_DEBUG_TX that previously gated</div>
<div>rte_validate_tx_offload() in the same loop, making that always-on too.</div>
<div> </div>
<div>The new function is marked __rte_unused both in the header</div>
<div>declaration and in the .c definition, even though it has exactly one</div>
<div>caller. That attribute was likely a leftover from an earlier draft</div>
<div>where the function was only conditionally referenced.</div>
<div> </div>
<div>The validation itself (595 lines) re-derives outer/inner L2/L3/L4</div>
<div>lengths from the packet bytes and compares them against what the mbuf</div>
<div>header declares - i.e., it's verifying that the application</div>
<div>correctly set mbuf->l2_len, l3_len, outer_l2_len, etc. before</div>
<div>submitting. Much of this overlaps what rte_validate_tx_offload() and</div>
<div>rte_net_intel_cksum_prepare() already check, both of which are called</div>
<div>in the same tx_prepare loop.</div>
<div> </div>
<div>Desired state:</div>
<div> </div>
<div>tx_burst should not contain validation checks for application errors.</div>
<div>The fast path runs once per packet at line rate, and any branch that</div>
<div>exists to catch a caller bug is paid on every well-behaved packet</div>
<div>forever. For driver assumptions that an application must satisfy,</div>
<div>the three sensible options are:</div>
<div> </div>
<div> 1. RTE_ASSERT() - compiles out in release builds, surfaces the</div>
<div> assumption to static analyzers, and fires for users running</div>
<div> debug builds.</div>
<div> </div>
<div> 2. unlikely() with drop-and-counter - if the check is cheap and</div>
<div> the assumption can be violated in production. The bad packet</div>
<div> is dropped, a queue stat is incremented, but the burst loop</div>
<div> continues.</div>
<div> </div>
<div> 3. Document the assumption in the API contract and don't check it</div>
<div> at runtime.</div>
<div> </div>
<div>tx_prepare is where validation belongs, and an unconditional check</div>
<div>there is acceptable - tx_prepare is opt-in, and an application that</div>
<div>calls it has already agreed to pay for inspection. The 595-line</div>
<div>function as written is mostly fine in that context. What's wrong is:</div>
<div> </div>
<div> (a) The commit message says one thing and the code does another.</div>
<div> The author should either add the #ifdef RTE_ETHDEV_DEBUG_TX</div>
<div> they claimed to add, or rewrite the commit message to describe</div>
<div> what the patch actually does.</div>
<div> </div>
<div> (b) Driver-specific tx_prepare validation should cover sxe2-</div>
<div> specific hardware constraints (descriptor count limits, buffer</div>
<div> alignment, segment size limits) - not redo the generic offload-</div>
<div> flag and length-consistency checks that rte_validate_tx_offload()</div>
<div> and rte_net_intel_cksum_prepare() perform. As currently written,</div>
<div> every packet going through tx_prepare gets its offload flags</div>
<div> validated roughly twice.</div>
<div> </div>
<div> (c) __rte_unused on the function should be dropped.</div>
<div> </div>
<div> </div>
<div>Redo this with one of the above and resubmit</div>
</div></blockquote>
</body></html>