|WARNING| fib6: fix error code propagation on next hop update
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Jun 5 19:43:40 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/165033
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-05
# DPDK Patch Review
## Patch Summary
The patch fixes error code propagation in `lib/fib/trie.c`. When `modify_dp()` fails (e.g., rejecting an out-of-range next hop with `-EINVAL`), the error was silently swallowed and success (0) was incorrectly returned to the caller.
---
## Review Findings
### Correctness Issues
**Error: Missing error check on `rte_rib6_set_nh()` call**
The code updates the RIB node (`rte_rib6_set_nh(node, next_hop)`) only when `modify_dp()` succeeds (`ret == 0`), but does not check whether `rte_rib6_set_nh()` itself succeeded. If `rte_rib6_set_nh()` can fail, the dataplane and RIB state may diverge.
**Suggested fix:**
```c
ret = modify_dp(dp, rib, &ip_masked, depth, next_hop);
if (ret == 0) {
ret = rte_rib6_set_nh(node, next_hop);
if (ret != 0) {
/* Rollback modify_dp if needed, or propagate error */
}
}
return ret;
```
Verify whether `rte_rib6_set_nh()` can fail. If it cannot, this is acceptable as-is. If it can fail but rollback is not possible, at minimum document the potential for inconsistency.
---
### Code Quality
**Info: Consider adding a comment explaining the fix**
The patch correctly propagates the error from `modify_dp()`, but the logic flow (conditional `set_nh` followed by unconditional return) is slightly non-obvious. A brief comment would improve maintainability:
```c
/* Update dataplane */
ret = modify_dp(dp, rib, &ip_masked, depth, next_hop);
if (ret == 0)
rte_rib6_set_nh(node, next_hop); /* Only update RIB on success */
/* Propagate modify_dp result (success or error) */
return ret;
```
---
## Summary
**Errors:** 1 (unchecked return value from `rte_rib6_set_nh()`)
**Warnings:** 0
**Info:** 1 (code clarity suggestion)
The core fix (returning `ret` instead of hardcoded `0`) is correct and addresses a real bug where error codes were silently dropped. The remaining concern is whether `rte_rib6_set_nh()` itself can fail and whether that failure needs to be handled.
More information about the test-report
mailing list