|WARNING| [v2, 3/3] net/ice: do not include private headers in testpmd

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Oct 15 17:13:42 CEST 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171450

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-15

# DPDK Patch Review

## Patch 1/3: net/ixgbe: avoid private driver include in testpmd

### Errors

None.

### Warnings

None.

### Info

None.

---

## Patch 2/3: net/iavf: do not include private headers in testpmd

### Errors

None.

### Warnings

None.

### Info

None.

---

## Patch 3/3: net/ice: do not include private headers in testpmd

### Errors

**New public API functions missing export macros:**
The patch adds three public functions to `rte_pmd_ice.h` (`rte_pmd_ice_dump_package`, `rte_pmd_ice_dump_switch`, `rte_pmd_ice_dump_txsched`) that are declared with `__rte_experimental` in the header, but the patch does not show corresponding `RTE_EXPORT_EXPERIMENTAL_SYMBOL(name, version)` macros in the `.c` file where these functions are defined. Per AGENTS.md Symbol Export Rules, new experimental API functions must have export macros in the implementation file.

**Example (assuming `ice_diagnose.c` is where these are implemented):**
```c
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_pmd_ice_dump_package, 26.11)
int
rte_pmd_ice_dump_package(uint16_t port, uint8_t **buff, uint32_t *size)
{
    /* ... */
}
```

Note: The patch shows `ice_diagnose.c` including `rte_pmd_ice.h`, suggesting these functions are defined there. If this patch moves the declarations but the implementations already existed (with export macros), this is not an error. However, the Fixes tag and the description imply new public API, so verify the implementations have the export macros.

### Warnings

**Missing testpmd hooks for new experimental API:**
The patch marks the three diagnostic functions as `__rte_experimental` and documents them as available through the public header, but does not add hooks in `app/testpmd` for these new public functions. AGENTS.md API and Driver Changes section states "New API functions must have hooks in `app/testpmd`."

The `ice_testpmd.c` file shown in the patch uses these functions, but `ice_testpmd.c` is part of the driver tree (`drivers/net/intel/ice/`), not `app/testpmd`. Per the guidelines, "Note: ixgbe_testpmd.c should be considered as part of testpmd," so `ice_testpmd.c` likely serves the same role. If `ice_testpmd.c` is indeed the testpmd integration (registered with testpmd via `testpmd_sources` in meson.build), then this satisfies the requirement. If not, testpmd hooks are missing.

**Recommendation:** Clarify whether `ice_testpmd.c` fulfills the testpmd hook requirement or if separate integration is needed in `app/testpmd`.

### Info

None.

---

## Summary

Patch 1/3 and Patch 2/3 are clean. Patch 3/3 has one correctness concern (missing export macros if these are truly new public functions) and one process concern (testpmd hook requirement, depending on how `ice_testpmd.c` integrates).

Review-Result: ERROR


More information about the test-report mailing list