|WARNING| [2/2] lpm: use scalar lookupx4 on RISC-V
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Oct 5 09:07:42 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.
### Warnings
None.
### Info
**New helper function lacks Doxygen documentation**
The new `__rte_lpm_lookupx4_hop()` helper function is an internal static inline function and does not require Doxygen since it is not part of the public API. However, a brief comment explaining its purpose would improve code clarity:
```c
/*
* Resolve a tbl24 entry to a next hop, performing a tbl8 lookup if needed.
* Returns the next hop ID on success, or defv on lookup failure.
*/
static inline uint32_t
__rte_lpm_lookupx4_hop(const struct rte_lpm *lpm, uint32_t tbl_entry,
uint32_t ip, uint32_t defv)
```
**Code refactoring is correct**
The patch correctly refactors the scalar lookupx4 to match the approach used in SIMD variants (SSE/NEON):
- Loads all four tbl24 entries upfront to allow cache miss overlap on in-order cores
- Extracts the common tbl8 resolution logic into a helper function
- Preserves identical lookup semantics to the original implementation
---
## Patch 2/2: lpm: use scalar lookupx4 on RISC-V
### Errors
None.
### Warnings
**Missing release notes update**
Removal of the RVV-specific implementation is a user-visible change. While it improves performance (as stated in the commit message), it changes which code path is used on RISC-V systems. This should be documented in the release notes:
```rst
* **lpm: improved scalar lookupx4 performance on in-order cores.**
The scalar ``rte_lpm_lookupx4()`` now loads all tbl24 entries before
resolving any of them, allowing cache misses to overlap on in-order cores.
The RISC-V vector implementation was removed as the improved scalar version
is now faster on all tested cores.
```
Add this to `doc/guides/rel_notes/release_XX_XX.rst` (where XX_XX is the target release version).
---
## Overall Assessment
Both patches are correct and represent a valid performance optimization. The refactoring in patch 1/2 is clean and maintains the original semantics. The removal of the RVV implementation in patch 2/2 is justified by the performance improvement from patch 1/2.
**Summary:**
- **Correctness:** No issues found
- **Style:** Compliant with DPDK coding standards
- **Documentation:** Release notes update recommended for patch 2/2
- **Testing:** Ensure `app/test/test_lpm*` tests pass on RISC-V platforms after this change
More information about the test-report
mailing list