|WARNING| net/bnxt: check return code of bnxt_hwrm_vnic_rss_cfg in flow create
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Sep 15 18:15:32 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/169567
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-15
# DPDK Patch Review: net/bnxt RSS error checking
## Summary
This patch adds error checking for `bnxt_hwrm_vnic_rss_cfg()` in the flow creation path. The change is correct and necessary.
---
## Errors
None found.
---
## Warnings
### 1. Inconsistent error handling pattern
**Issue:** The error path sets `rc = -rte_errno` after calling `rte_flow_error_set()`, but this is incorrect. The `rte_flow_error_set()` function does not modify `rte_errno`. The original error code from `bnxt_hwrm_vnic_rss_cfg()` is already in `rc` (negative value).
**Current code:**
```c
rc = bnxt_hwrm_vnic_rss_cfg(bp, vnic);
if (rc) {
rte_flow_error_set(error,
-rc,
RTE_FLOW_ERROR_TYPE_ACTION,
act,
"VNIC RSS configure failed");
rc = -rte_errno; /* WRONG: rte_flow_error_set doesn't set rte_errno */
goto ret;
}
```
**Suggested fix:**
```c
rc = bnxt_hwrm_vnic_rss_cfg(bp, vnic);
if (rc) {
rte_flow_error_set(error,
-rc,
RTE_FLOW_ERROR_TYPE_ACTION,
act,
"VNIC RSS configure failed");
goto ret; /* rc already contains the error */
}
```
The `rc` variable already contains the negative error code from `bnxt_hwrm_vnic_rss_cfg()`, which is the correct value to return. The assignment `rc = -rte_errno` will likely overwrite it with zero or an unrelated value.
---
## Info
### Error propagation context
The function signature and existing error paths in this file should be verified to ensure the error value propagation is consistent. However, the core issue is that `rc = -rte_errno` is definitively wrong here since `rte_flow_error_set()` is a reporting function, not an error-generating function.
### Positive aspects
- Correctly checks the return value of a previously unchecked function
- Properly uses `rte_flow_error_set()` to provide user feedback
- Error path correctly jumps to the `ret` label for cleanup
- First parameter to `rte_flow_error_set()` correctly negates `rc` (which is negative on error)
More information about the test-report
mailing list