|WARNING| [2/2] lpm: use scalar lookupx4 on RISC-V

[dpdklab at iol.unh.edu]

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

**Missing release notes** (Warning):
This patch improves performance of the scalar LPM lookup path on in-order cores by pre-fetching all four tbl24 entries before resolving any of them. This is a user-visible performance improvement and should be documented in the release notes under the LPM library section.

### Info

**Code structure** (Info):
The new `__rte_lpm_lookupx4_hop()` helper function is a reasonable refactoring that avoids code duplication. The logic is identical to what was previously inlined four times in `rte_lpm_lookupx4()`.

**Performance rationale** (Info):
The commit message states this change benefits in-order cores by overlapping cache misses. This is a valid optimization that matches the existing SSE and NEON implementations' approach. However, consider whether this pattern benefits all architectures or only specific ones, and whether any regression testing was done on out-of-order cores.

---

## PATCH 2/2: lpm: use scalar lookupx4 on RISC-V

### Errors

None identified.

### Warnings

**Missing release notes** (Warning):
Removing the RISC-V Vector (RVV) implementation in favor of the scalar path is a significant change affecting RISC-V platforms. This should be documented in the release notes, explaining:
- The RVV implementation has been removed
- The scalar implementation is now used on RISC-V
- Performance justification (faster on tested cores)
- Any implications for users with RVV-capable hardware

**Testing evidence** (Warning):
The commit message states "the scalar version is faster on every core tried" but does not specify which cores were tested or provide performance data. Consider adding this information to the commit message or release notes to justify the removal of vectorized code.

### Info

**API compatibility** (Info):
The change maintains the same external API (`rte_lpm_lookupx4` remains available and functional) so no ABI break is introduced. The change is internal to the implementation selection.

**Vector implementation removal** (Info):
Removing vector implementations in favor of scalar code is unusual but can be justified when:
1. The vector implementation has higher overhead than the scalar path
2. The data parallelism doesn't offset the overhead
3. The scalar implementation has been optimized (as in patch 1/2)

In this case, the RVV implementation's always-masked gather for tbl8 may indeed be slower than the optimized scalar path, especially if tbl8 lookups are infrequent.

---

## General Comments

**Series coherence**:
The two patches work well together. Patch 1/2 optimizes the scalar implementation, making it competitive with or faster than the RVV implementation, which justifies removing the RVV code in patch 2/2.

**Code quality**:
Both patches are clean, focused, and maintain DPDK coding style. No forbidden tokens, deprecated API usage, or style violations were identified.

**Documentation gaps**:
The primary weakness is the lack of release notes updates. Both patches make user-visible changes that should be documented.