|WARNING| net/af_packet: fix build with old kernel headers
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Nov 6 16:10:40 CET 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172698
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-06
# DPDK Patch Review
## ERRORS
None.
## WARNINGS
### Documentation Location
The documentation change documents a runtime behavior difference between kernel versions (4.20+ supports kernel filtering, <4.20 falls back to PMD filtering). This is correct to document, but the phrasing could be clearer. Consider:
```rst
When ``ignore_outgoing`` is set, ``PACKET_IGNORE_OUTGOING`` is set for single-queue ports,
which moves the filtering from the PMD to the kernel.
On kernels older than 4.20, the PMD does the filtering instead.
For multi-queue ports ``PACKET_FANOUT_FLAG_IGNORE_OUTGOING`` is set
to achieve the same but this only works from Linux 6.2.
For earlier Linux versions, the PMD does the filtering for multi-queue ports.
```
This groups the single-queue and multi-queue cases more clearly.
## INFO
### Release Notes
The patch adds fallback behavior for older kernels (a compatibility improvement) but does not update the release notes. While this is a bug fix (Fixes: tag present), the addition of graceful degradation on old kernels and the new documentation of kernel version requirements are worth mentioning in the release notes under "Resolved Issues" or similar.
### Variable Initialization
The `pmd_filter` variable is declared uninitialized at line 924:
```c
bool pmd_filter;
```
It is assigned before use (line 1062 for multi-queue, or lines 1067/1070/1095 for single-queue), but initializing to `false` at declaration would make the code more defensive:
```c
bool pmd_filter = false;
```
This is not a bug (all code paths assign before read) but is a hardening suggestion.
### errno Preservation
At lines 1065-1071, the code checks `errno == ENOPROTOOPT` after `setsockopt()` fails. If `PMD_LOG(INFO, ...)` or any function it calls modifies `errno` before the subsequent `else if (rc == -1)` check at line 1071, the second error branch could misfire.
Good practice would be to save `errno` immediately after the failing syscall:
```c
if (rc == -1) {
int saved_errno = errno;
if (saved_errno == ENOPROTOOPT) {
PMD_LOG(INFO, ...);
pmd_filter = true;
} else {
errno = saved_errno; /* restore for PMD_LOG_ERRNO */
PMD_LOG_ERRNO(ERR, ...);
goto error;
}
}
```
However, this is a pre-existing pattern in the file (the code already calls `PMD_LOG_ERRNO` in the else-branch without saving errno). If `PMD_LOG` does not touch `errno`, the current code is safe. Flagging as Info only since this is a general hardening suggestion, not a defect introduced by this patch.
Review-Result: WARNING
More information about the test-report
mailing list