|WARNING| [2/2] lpm: use scalar lookupx4 on RISC-V
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Oct 5 09:16:05 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/170529
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-05
# DPDK Patch Review
## Patch 1/2: lpm: load tbl24 entries up front in scalar lookupx4
### Errors
None identified.
### Warnings
**Release notes missing:**
This patch modifies the LPM library's scalar lookup implementation to improve performance on in-order cores by overlapping cache misses. This is a performance optimization that affects API behavior (albeit in a transparent way). The change should be documented in `doc/guides/rel_notes/release_25_03.rst` (or the appropriate current release file) under the LPM library section.
### Info
**Positive observations:**
1. The refactored code correctly preserves the original semantics - both the original `rte_lpm_lookup()` calls and the new direct table access produce the same results.
2. The new helper function `__rte_lpm_lookupx4_hop()` properly handles the extended entry path and returns the correct default value when lookup fails.
3. Prefetching the four tbl24 entries before any resolution is a sound optimization for in-order cores.
4. The code correctly uses bitwise operations consistent with the existing LPM implementation.
**Style note:**
The new helper function `__rte_lpm_lookupx4_hop()` uses an `unlikely()` wrapper around the extended entry check, which is appropriate given that extended entries are the exception rather than the rule in typical LPM usage.
---
## Patch 2/2: lpm: use scalar lookupx4 on RISC-V
### Errors
**Missing documentation of removed header:**
The patch removes `rte_lpm_rvv.h` from `indirect_headers` in meson.build and deletes the file entirely, but does not update any documentation. While `indirect_headers` means this header was not part of the public API, the removal of architecture-specific code should still be noted in release notes. Add an entry to the release notes explaining that the RISC-V vector implementation has been removed in favor of the scalar implementation due to superior performance.
### Warnings
**No justification for removal in commit message:**
The commit message states "the scalar version is faster on every core tried" but does not provide any performance data or specify which cores were tested. While the reasoning is plausible (RVV version always issues a masked gather which may be slower than the optimized scalar code from patch 1/2), including benchmark results in the commit message would strengthen the justification for removing architecture-specific optimized code.
### Info
**File removal is clean:**
The patch correctly removes all references to `rte_lpm_rvv.h`:
1. Removed from meson.build's `indirect_headers` list
2. Removed from the architecture-specific `#include` chain in `rte_lpm.h`
3. File deleted entirely
The fallback to `rte_lpm_scalar.h` via the `#else` branch in `rte_lpm.h` is correct.
---
## General Observations
**Series coherence:**
The two patches work well together - patch 1/2 optimizes the scalar implementation, making it competitive with the RISC-V vector version, which justifies removing the RVV code in patch 2/2.
**Testing note:**
The patches modify core lookup logic. Ensure that:
1. The LPM unit tests in `app/test/test_lpm*.c` pass on all architectures
2. Performance regression tests have been run on x86, ARM, and RISC-V
3. Both patches compile independently (for bisectability)
More information about the test-report
mailing list