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

[dpdklab at iol.unh.edu]

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

**Missing bypass capability validation in show_bypass command:**
The original `check_bypass_enabled()` function validated capability *before* attempting any bypass operations. After removing it, the `cmd_show_bypass_config_parsed()` function now proceeds to call `rte_pmd_ixgbe_bypass_state_show()` and multiple `rte_pmd_ixgbe_bypass_event_show()` calls in a loop, checking `-ENOTSUP` only on the first call. If the first call succeeds but subsequent event queries fail with `-ENOTSUP` (or any other error), the loop continues calling bypass functions and the final `if (rc != 0)` block was deleted, so errors in the loop are not reported.

Original code:
```c
if (!check_bypass_enabled(port_id))
    return;
```

Modified code only checks the first operation:
```c
rc = rte_pmd_ixgbe_bypass_state_show(port_id, &bypass_mode);
if (rc == -ENOTSUP) {
    fprintf(stderr, "Bypass is not enabled on this port.\n");
    return;
}
if (rc != 0) {
    fprintf(stderr, "\tFailed to get bypass mode for port = %d\n",
        port_id);
    return;
}
/* then loops over events without checking rc for -ENOTSUP or other errors */
```

The loop body calls `rte_pmd_ixgbe_bypass_event_show()` four times and `rte_pmd_ixgbe_bypass_wd_timeout_show()` once, updating `rc` on each iteration, but does not check for errors until after the loop. The deleted final error check (`if (rc != 0)`) means errors from the loop are now silently ignored.

**Fix:** Check `rc` inside the loop after each bypass operation, not just after the initial state query. Either check for `-ENOTSUP` on each iteration or verify that all operations will consistently return the same error if bypass is not supported. The removal of the final `if (rc != 0)` check means errors are now dropped.

```c
/* After the loop, the deleted code was: */
if (rc != 0)
    fprintf(stderr,
        "\tFailed to get bypass configuration for port = %d\n",
        port_id);
/* This check should be restored or per-operation error handling added */
```

---

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

No issues found. The removal of unused private headers is correct.

---

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

### Warnings

**Missing `__rte_experimental` on function declarations:**
The three functions moved to `rte_pmd_ice.h` (`rte_pmd_ice_dump_package`, `rte_pmd_ice_dump_switch`, `rte_pmd_ice_dump_txsched`) are marked `__rte_experimental` in the new public header, but there are no corresponding `RTE_EXPORT_EXPERIMENTAL_SYMBOL` macros in the `.c` file where these functions are defined (presumably `ice_diagnose.c` based on the includes patch). The guidelines require that experimental functions have the export macro in the implementation file.

>From the patch:
```c
/* in rte_pmd_ice.h */
__rte_experimental
int rte_pmd_ice_dump_package(uint16_t port, uint8_t **buff, uint32_t *size);
```

The `ice_diagnose.c` patch shows `#include "rte_pmd_ice.h"` added but no export macros.

**Fix:** Add `RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_pmd_ice_dump_package, 26.11)` (and similarly for the other two functions) in `ice_diagnose.c` immediately before each function definition, following the pattern in AGENTS.md:
```c
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_pmd_ice_dump_package, 26.11)
__rte_experimental
int
rte_pmd_ice_dump_package(uint16_t port, uint8_t **buff, uint32_t *size)
{
    /* ... */
}
```

**Documentation could specify error codes more precisely:**
The Doxygen comments for the three dump functions state "or a negative error code on failure" but do not enumerate specific error codes callers should handle (e.g., `-EINVAL` for invalid arguments, `-ENOMEM` for allocation failure). While not strictly required, this would improve API documentation quality.

---

Review-Result: ERROR