|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 11:53:53 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
## Summary
This patch adds error checking for `bnxt_hwrm_vnic_rss_cfg()` in the flow creation path. The fix is correct in principle but contains a significant error in the error handling logic.
---
## Errors
### 1. Incorrect error code propagation causing wrong errno value
**Location**: `drivers/net/bnxt/bnxt_flow.c:1641`
The code sets `rte_flow_error_set()` with `-rc` (negated error code), then assigns `rc = -rte_errno`. This is wrong because:
1. `rte_flow_error_set()` expects a positive error code as its second argument (it stores this in `error->type`)
2. The function does NOT set `rte_errno`
3. Reading `rte_errno` here will return whatever the last libc function left in errno, not the RSS config failure code
4. The original error code from `bnxt_hwrm_vnic_rss_cfg()` is lost
**Current code:**
```c
rc = bnxt_hwrm_vnic_rss_cfg(bp, vnic);
if (rc) {
rte_flow_error_set(error,
-rc, /* WRONG: negates rc */
RTE_FLOW_ERROR_TYPE_ACTION,
act,
"VNIC RSS configure failed");
rc = -rte_errno; /* WRONG: reads unrelated errno */
goto ret;
}
```
**Fix:**
```c
rc = bnxt_hwrm_vnic_rss_cfg(bp, vnic);
if (rc) {
rte_flow_error_set(error,
-rc, /* Correct: expects positive errno */
RTE_FLOW_ERROR_TYPE_ACTION,
act,
"VNIC RSS configure failed");
goto ret; /* rc already contains the error */
}
```
`bnxt_hwrm_vnic_rss_cfg()` returns negative error codes on failure. The calling convention in `bnxt_validate_and_parse_flow()` is to return negative error codes. Simply propagate `rc` as-is; the `rc = -rte_errno` assignment should be deleted entirely.
---
## Warnings
None.
---
## Info
### Code context observation
The existing error paths in this function (lines 1590-1596, for example) follow the pattern of calling `rte_flow_error_set()` and then using `goto ret` without reassigning `rc`. The patch should match this established pattern.
More information about the test-report
mailing list