|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