|WARNING| [v2, 3/3] common/sfc_efx/base: switch netport stats to use EVB port ID
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Aug 12 20:04:22 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/167594
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-12
# DPDK Patch Review
## PATCH 1/3: common/sfc_efx/base: update MCDI headers
### Errors
None.
### Warnings
None.
### Info
This patch imports MCDI header definitions from upstream.
The changes are mechanical additions of new constants and structure fields for statistics support.
The code is located in `drivers/common/sfc_efx/base/`, which is documented as a vendor base directory
where different naming conventions are permitted per the AGENTS.md guidelines.
---
## PATCH 2/3: common/sfc_efx/base: add support for VADAPTER statistics IDs
### Errors
None.
### Warnings
None.
### Info
This patch adds mappings for VADAPTER statistics following the existing pattern established
for MAC and PHY statistics in the same file.
The code is in the vendor base directory where style exceptions are permitted.
---
## PATCH 3/3: common/sfc_efx/base: switch netport stats to use EVB port ID
### Errors
**Error 1: Missing error check on efx_np_mac_stats() call**
In `efx_mcdi.c` at the call site for `efx_np_mac_stats()`:
```c
rc = efx_np_mac_stats(enp, epp->ep_np_handle,
enp->en_vport_id, NULL, EFX_STATS_CLEAR, 0);
```
The return value `rc` is assigned but never checked before being returned or used.
If `efx_np_mac_stats()` fails, the error is silently propagated without any logging or cleanup.
While this may be intentional pass-through of the error code,
the pattern is inconsistent with defensive programming.
**Suggested fix:**
Verify that all callers of `efx_mcdi_mac_stats_clear()` handle errors correctly,
or add error handling/logging at this point if the failure mode requires it.
### Warnings
None.
### Info
**Parameter reordering:**
The patch reorders parameters in `efx_np_mac_stats()` to match `efx_mcdi_mac_stats()`,
placing `vport_id` before `esmp` and moving `action` after `esmp`.
This improves consistency between the two related functions.
**MCDI version upgrade:**
The code switches from `MC_CMD_GET_NETPORT_STATISTICS_IN` to `MC_CMD_GET_NETPORT_STATISTICS_V2_IN`
to support the new `PORT_ID` field required for VADAPTER statistics.
**Comment regarding EVB_PORT_ID_NULL:**
The code includes a comment explaining why `EVB_PORT_ID_NULL` is used when disabling periodic stats
rather than `EVB_PORT_ID_ASSIGNED`.
This is good defensive documentation.
---
## Summary
The patch series adds support for VADAPTER statistics in the sfc_efx driver base code.
Patches 1 and 2 are clean mechanical additions.
Patch 3 has one potential issue:
- **Error 1** flags a missing error check on an `efx_np_mac_stats()` call,
though this may be intentional error pass-through.
The code is in the vendor base directory (`drivers/common/sfc_efx/base/`)
where naming and style exceptions are permitted per DPDK guidelines.
More information about the test-report
mailing list