|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