|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