|SUCCESS| net/af_packet: move size check to after vlan insert

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Oct 29 00:22:23 CET 2026


Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172235

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-28

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

None identified.

## INFO

**Vlan insertion error path leaks modified mbuf**

```c
if (mbuf->ol_flags & RTE_MBUF_F_TX_VLAN) {
	if (rte_vlan_insert(&mbuf)) {
		rte_pktmbuf_free(mbuf);
		continue;
	}
}
```

`rte_vlan_insert()` may modify `mbuf` (reallocate with headroom) before failing.
If it returns non-zero, the `mbuf` pointer may now point to a new mbuf,
and the original mbuf (if it was reallocated and the realloc failed partway)
could be leaked, or the new mbuf (if realloc succeeded but a later step failed)
is what gets freed.

This is **pre-existing code** -- the patch does not introduce or change this logic.
The patch only moves an unrelated size check.

Worth verifying `rte_vlan_insert()` semantics:
does it guarantee the `mbuf` pointer is valid (either unchanged or successfully reallocated)
when it returns an error, or can it leave `mbuf` pointing to freed memory?
If the latter, the `rte_pktmbuf_free(mbuf)` on error is a use-after-free.

Checking `lib/net/rte_vlan.h` implementation would confirm.
This note is for whoever works on af_packet Tx error handling next.

---

**Patch correctness**

The moved size check is now applied **after** vlan insertion,
which is correct: the check must validate the final packet size,
not the pre-insertion size.
A packet at exactly `frame_data_size` before insertion
would exceed the limit after adding 4 bytes of VLAN tag and would be silently transmitted truncated
or cause ring corruption.

The fix is sound.

Review-Result: CLEAN


More information about the test-report mailing list