|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