|WARNING| [v1] fib6: fix sign conversion of tbl8 indexes
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Oct 21 20:16:42 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171819
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-21
# DPDK Patch Review
## ERRORS
### Integer multiply without widening cast
**Line 333 in diff context:**
```c
p = get_tbl_p_by_idx(dp->tbl8, (tbl8_idx *
TRIE_TBL8_GRP_NUM_ENT) + *ip_part, dp->nh_sz);
```
The multiplication `tbl8_idx * TRIE_TBL8_GRP_NUM_ENT` is performed in `int` (since `tbl8_idx` is `int` and the constant is likely `int`), then the result is widened to `uint64_t` for the function parameter. When `tbl8_idx` is large (e.g., 0x800001), this 32x32 multiply can overflow before widening, producing an incorrect index.
**Fix:** Cast `tbl8_idx` to `uint64_t` before the multiply:
```c
p = get_tbl_p_by_idx(dp->tbl8, ((uint64_t)tbl8_idx *
TRIE_TBL8_GRP_NUM_ENT) + *ip_part, dp->nh_sz);
```
Note: The patch already fixes two similar shifts (`tbl8_idx << 1`) but misses this multiply in the same function (`write_edge()`).
---
## WARNINGS
None identified.
---
## Review Summary
The patch correctly addresses the sign-extension bug described in the commit message by making `idx` and `prev_idx` unsigned and using a separate signed `tbl8_idx` for error handling. The casts added to the shift operations are also correct and necessary.
However, the patch misses one related integer overflow: the multiplication in `write_edge()` at line 333 (diff context) suffers from the same narrowing issue as the shifts. The multiply is performed in `int`, and when `tbl8_idx` exceeds 2^23, the product can overflow before being widened to the 64-bit index parameter.
This is the same class of bug the patch is fixing elsewhere (integer operation at wrong width), and leaving it unfixed means the index calculation can still produce bogus pointers in the same code path. The commit message states "a modified fib6_perf_autotest with 750,000 random /128 routes crashed" -- this multiply is in the hot path for those route additions and should be widened.
Review-Result: ERROR
More information about the test-report
mailing list