|WARNING| net/bnxt: fix completion metadata format type for Thor
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Oct 6 02:58:35 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/170627
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-06
# DPDK Patch Review
## Patch Summary
This patch fixes the RX completion record metadata format type for Thor devices in the bnxt driver. It changes from Type 3 to Type 1 for Thor, keeping Type 3 only for Thor2.
## Review Findings
### Errors
None found.
### Warnings
**1. Indentation inconsistency (lines 487-489)**
The `bnxt_hwrm_vnic_update()` call changes indentation from the original code. While the new indentation aligns arguments with the opening parenthesis (which is acceptable), it creates inconsistency with the surrounding code style. The original double-indent for continuation lines should be preserved.
Current:
```c
rc = bnxt_hwrm_vnic_update(bp,
vnic,
HWRM_VNIC_UPDATE_INPUT_ENABLES_METADATA_FORMAT_TYPE_VALID);
```
Should be (matching original style):
```c
rc = bnxt_hwrm_vnic_update(bp,
vnic,
HWRM_VNIC_UPDATE_INPUT_ENABLES_METADATA_FORMAT_TYPE_VALID);
```
Or maintain the original indentation exactly:
```c
rc = bnxt_hwrm_vnic_update(bp,
vnic,
HWRM_VNIC_UPDATE_INPUT_ENABLES_METADATA_FORMAT_TYPE_VALID);
```
### Info
**1. Implicit comparison acceptable (line 490)**
The code uses `if (unlikely(rc))` which is an implicit comparison. According to DPDK style, explicit comparison (`if (unlikely(rc != 0))`) is preferred. However, since this line is unchanged from the original code and the patch is a minimal fix, this is acceptable in context. New code should use explicit comparison.
**2. Variable declaration location**
The `vnic` pointer is declared at the top of the conditional block. This is acceptable C style, though C99 allows declaring at point of first use. No change needed.
## Summary
The patch correctness is sound - it fixes a real bug where Thor devices were incorrectly configured with Type 3 metadata format. The logic properly distinguishes between Thor2 (Type 3) and other Thor variants (Type 1).
The only issue is minor indentation inconsistency introduced during the refactoring. Consider maintaining the exact indentation style of the original `bnxt_hwrm_vnic_update()` call to avoid unnecessary whitespace changes.
More information about the test-report
mailing list