|SUCCESS| [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:12:16 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
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: Update MCDI Headers
### Info
**Definition list suggestion in RST documentation:**
This patch updates enumeration definitions in C headers, not RST documentation, so this guideline does not apply.
**No issues found in this patch.** This is a straightforward import of vendor MCDI header definitions from upstream. The changes are:
- New statistic source IDs (PM, RXDP, LL, FC, VADAPTER)
- New statistic enum values for each source
- Privilege category changes from `SRIOV_CTG_LINK` to `SRIOV_CTG_GENERAL`
- Added `MC_CMD_GET_NETPORT_STATISTICS_V2_IN` message definition
- Documentation clarifications
All changes follow the existing code patterns in `drivers/common/sfc_efx/base/`, which is vendor base code and uses different naming conventions per the exception in the guidelines.
---
## Patch 2/3: Add Support for VADAPTER Statistics IDs
### Errors
None.
### Warnings
None.
### Info
**No issues found in this patch.** This patch:
- Adds mapping entries for VADAPTER statistics to the existing `efx_np_mac_stat_map[]` array
- Follows the exact pattern of existing `EFX_NP_STAT_MAC` and `EFX_NP_STAT_PHY` macro usage
- Uses the MCDI header definitions added in patch 1/3
- Maintains consistency with the surrounding code style
The use of the `EFX_NP_STAT_VADAPTER` macro pattern is consistent with the existing driver base code style. The macro is properly undefined after use.
---
## Patch 3/3: Switch Netport Stats to Use EVB Port ID
### Errors
None.
### Warnings
None.
### Info
**No issues found in this patch.** This patch:
- Updates `efx_np_mac_stats()` signature to accept `vport_id` parameter
- Switches MCDI message from `MC_CMD_GET_NETPORT_STATISTICS_IN` to `MC_CMD_GET_NETPORT_STATISTICS_V2_IN` to support the new field
- Updates all call sites to pass the vport_id (6 locations total)
- Adds EVB_PORT_ID handling with appropriate comment explaining the disable case
- Adjusts argument order for consistency: `(enp, nph, vport_id, esmp, action, period_ms)`
The new argument order places related parameters together logically (vport_id with the handle, action with period). All call sites are updated in the same patch, preventing compilation issues.
The comment about not using `EVB_PORT_ID_ASSIGNED` when disabling is valuable context for future maintainers.
---
## Summary
**All three patches are clean.** No correctness bugs, style violations, or process issues were found.
The patches:
- Are properly ordered (headers first, then usage)
- Compile independently (each adds definitions before using them)
- Follow the existing code patterns in the driver base directory
- Maintain API consistency across call sites
More information about the test-report
mailing list