|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:12:41 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
## Patch Summary
This patch fixes a corner case in the af_packet driver where VLAN insertion could cause a packet to exceed the TPACKET_V2 ring slot size. The fix moves the size check to after VLAN insertion.
## Review Findings
### ERRORS
**Resource leak on poll() failure**
In the error path after `poll()` returns < 0, the code breaks out of the loop without freeing `mbuf`:
```c
if (poll(&pfd, 1, -1) < 0)
break;
```
At this point, `mbuf` has been removed from `bufs` (line 319: `mbuf = *bufs++;`) and has passed all checks including VLAN insertion and size validation. When `poll()` fails and the loop breaks, this mbuf is never freed, causing a resource leak.
**Suggested fix:** Free the mbuf before breaking:
```c
if (poll(&pfd, 1, -1) < 0) {
rte_pktmbuf_free(mbuf);
break;
}
```
Note: This is a pre-existing bug not introduced by this patch, but the code review guidelines state that pre-existing bugs involving resource leaks should be flagged as Info. However, since this specific line is in the patch context and the patch touches the same function's error handling logic (adding a new `rte_pktmbuf_free()` call), it's appropriate to highlight it now as the author is already working on resource cleanup in this function.
Actually, upon re-reading the evidence rule: this bug exists only in the context lines, not in the added or changed lines. The patch does not touch the `poll()` call or the `break` statement. Therefore, this should be Info, not Error.
**Correction:** Demoting to Info as this is pre-existing code.
---
### WARNINGS
None identified.
---
### INFO
**Pre-existing: Resource leak on poll() failure**
The existing code has a resource leak when `poll()` fails (line visible in patch context):
```c
if (poll(&pfd, 1, -1) < 0)
break;
```
At this point, `mbuf` has been dequeued and processed but is not freed before breaking. This leak exists in the original code and is not introduced by this patch.
**Suggested fix:** Add `rte_pktmbuf_free(mbuf);` before the break.
---
**Patch achieves its stated goal**
The fix correctly addresses the corner case: by moving the size check to after VLAN insertion, packets that would exceed `frame_data_size` after VLAN header insertion (4 bytes) are now properly detected and dropped. The logic is sound.
Review-Result: CLEAN
More information about the test-report
mailing list