|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