* [PATCH] net/af_xdp: fix external mbuf transmit
@ 2026-01-05 12:54 David Marchand
2026-01-12 1:03 ` Stephen Hemminger
0 siblings, 1 reply; 3+ messages in thread
From: David Marchand @ 2026-01-05 12:54 UTC (permalink / raw)
To: dev; +Cc: stable, Ciara Loftus, Maryam Tahhan, Xiaolong Ye, Kevin Laatz
A mbuf may be pointing at external buffer like when forwarding a TSO
packet coming from a vhost-user port in OVS (see
RTE_VHOST_USER_EXTBUF_SUPPORT).
Checking the mbuf mempool to trigger the zero copy optimisation is
broken: the driver will (at best) read 0s or (at worse) crash when
reading more data than what the direct buffer can contain.
Fixes: d8a210774e1d ("net/af_xdp: support unaligned umem chunks")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
drivers/net/af_xdp/rte_eth_af_xdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index f2a8fb95d1..10dbcf1333 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -608,7 +608,7 @@ af_xdp_tx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
for (i = 0; i < nb_pkts; i++) {
mbuf = bufs[i];
- if (mbuf->pool == umem->mb_pool) {
+ if (RTE_MBUF_DIRECT(mbuf) && mbuf->pool == umem->mb_pool) {
desc = reserve_and_fill(txq, mbuf, umem, NULL);
if (!desc) {
kick_tx(txq, cq);
--
2.52.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] net/af_xdp: fix external mbuf transmit
2026-01-05 12:54 [PATCH] net/af_xdp: fix external mbuf transmit David Marchand
@ 2026-01-12 1:03 ` Stephen Hemminger
2026-01-12 7:58 ` David Marchand
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2026-01-12 1:03 UTC (permalink / raw)
To: David Marchand
Cc: dev, stable, Ciara Loftus, Maryam Tahhan, Xiaolong Ye, Kevin Laatz
On Mon, 5 Jan 2026 13:54:41 +0100
David Marchand <david.marchand@redhat.com> wrote:
> A mbuf may be pointing at external buffer like when forwarding a TSO
> packet coming from a vhost-user port in OVS (see
> RTE_VHOST_USER_EXTBUF_SUPPORT).
>
> Checking the mbuf mempool to trigger the zero copy optimisation is
> broken: the driver will (at best) read 0s or (at worse) crash when
> reading more data than what the direct buffer can contain.
>
> Fixes: d8a210774e1d ("net/af_xdp: support unaligned umem chunks")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
Looks good to me and Claude ;-) but he has more to say.
Queued to next-net
## DPDK Patch Review: net/af_xdp: fix external mbuf transmit
### Summary
This patch fixes a bug in the AF_XDP driver's zero-copy transmit path where mbufs pointing to external buffers (e.g., from vhost-user with `RTE_VHOST_USER_EXTBUF_SUPPORT`) could cause data corruption or crashes. The fix adds an `RTE_MBUF_DIRECT()` check before using the zero-copy optimization.
---
### Commit Message Review
| Criterion | Status | Details |
|-----------|--------|---------|
| Subject length | ✓ Pass | 38 characters (limit: 60) |
| Subject format | ✓ Pass | `net/af_xdp:` prefix is correct for driver |
| Subject case | ✓ Pass | Lowercase after colon |
| Imperative mood | ✓ Pass | "fix" not "fixes" or "fixed" |
| No trailing period | ✓ Pass | |
| Body line wrap | ✓ Pass | All lines under 75 characters |
| Body doesn't start with "It" | ✓ Pass | Starts with "A mbuf" |
| `Fixes:` tag | ✓ Pass | 12-char SHA: `d8a210774e1d` |
| `Cc: stable@dpdk.org` | ✓ Pass | Present for bug fix |
| `Signed-off-by:` | ✓ Pass | Present with real name and email |
| Tag order | ✓ Pass | Fixes → Cc → blank line → Signed-off-by |
---
### Code Review
**Change location:** `drivers/net/af_xdp/rte_eth_af_xdp.c`, line 608 (in `af_xdp_tx_zc()`)
```c
// Before:
if (mbuf->pool == umem->mb_pool) {
// After:
if (RTE_MBUF_DIRECT(mbuf) && mbuf->pool == umem->mb_pool) {
```
| Criterion | Status | Details |
|-----------|--------|---------|
| Line length | ✓ Pass | Well under 100 characters |
| Macro naming | ✓ Pass | `RTE_MBUF_DIRECT` uses correct `RTE_` prefix |
| Technical correctness | ✓ Pass | Properly gates zero-copy path |
| Minimal change | ✓ Pass | Single-line fix, targeted |
#### Warning (Minor)
**Implicit boolean comparison:** The `RTE_MBUF_DIRECT(mbuf)` macro returns an integer-like value (0 or non-zero). Per DPDK guidelines, explicit comparison with zero is preferred for integers:
```c
// Suggested (strict interpretation):
if (RTE_MBUF_DIRECT(mbuf) != 0 && mbuf->pool == umem->mb_pool) {
```
However, this macro is semantically boolean and is commonly used in direct form throughout DPDK. This is a stylistic preference, not a blocking issue.
---
### Technical Analysis
The fix is correct. Looking at the `af_xdp_tx_zc()` function in the provided source:
1. **The bug:** Line 624 in the original code only checks `mbuf->pool == umem->mb_pool` to decide if zero-copy can be used. This fails for indirect/external mbufs where `mbuf->buf_addr` points to external memory, not the pool's memory region.
2. **The consequence:** When zero-copy is incorrectly triggered, `reserve_and_fill()` computes addresses relative to `umem->buffer`, which produces invalid addresses for external buffer data.
3. **The fix:** Adding `RTE_MBUF_DIRECT(mbuf)` ensures the mbuf data resides in its own attached buffer, making the pool check meaningful.
4. **Fallback path:** When the check fails, the code correctly falls through to the else branch (lines 633-647) which allocates a local mbuf and performs a copy.
---
### Verdict
| Severity | Count | Items |
|----------|-------|-------|
| **Error** | 0 | — |
| **Warning** | 1 | Implicit boolean check on `RTE_MBUF_DIRECT()` |
| **Info** | 0 | — |
**Recommendation:** **Accept** — This is a well-crafted bug fix. The commit message is clear, properly attributed, and follows DPDK conventions. The code change is minimal, correct, and addresses a real crash/corruption scenario. The warning about implicit boolean comparison is minor and matches existing DPDK patterns.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] net/af_xdp: fix external mbuf transmit
2026-01-12 1:03 ` Stephen Hemminger
@ 2026-01-12 7:58 ` David Marchand
0 siblings, 0 replies; 3+ messages in thread
From: David Marchand @ 2026-01-12 7:58 UTC (permalink / raw)
To: Stephen Hemminger, Morten Brørup
Cc: dev, stable, Ciara Loftus, Maryam Tahhan, Xiaolong Ye,
Kevin Laatz, Thomas Monjalon
Hello,
On Mon, 12 Jan 2026 at 02:04, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> #### Warning (Minor)
>
> **Implicit boolean comparison:** The `RTE_MBUF_DIRECT(mbuf)` macro returns an integer-like value (0 or non-zero). Per DPDK guidelines, explicit comparison with zero is preferred for integers:
>
> ```c
> // Suggested (strict interpretation):
> if (RTE_MBUF_DIRECT(mbuf) != 0 && mbuf->pool == umem->mb_pool) {
> ```
>
> However, this macro is semantically boolean and is commonly used in direct form throughout DPDK. This is a stylistic preference, not a blocking issue.
All callers in DPDK are using this macro as a boolean, and I found no
opensource project doing differently.
I would either fix the macro or change nothing at all.
--
David Marchand
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-01-12 7:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-05 12:54 [PATCH] net/af_xdp: fix external mbuf transmit David Marchand
2026-01-12 1:03 ` Stephen Hemminger
2026-01-12 7:58 ` David Marchand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).