|WARNING| [v3,9/9] ci: run reference binaries against current ABI

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-21

# DPDK Patch Review

## Patch 1/9: bbdev: add per-queue statistics API

### Errors

None found.

### Warnings

1. **Missing `__rte_experimental` tag in implementation** (lib/bbdev/rte_bbdev.c:818):
   The function declaration in the header correctly uses `__rte_experimental`, but the implementation should also be marked (on the line before the return type) for consistency with DPDK conventions.

   ```c
   /* Current - missing tag in implementation */
   RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_bbdev_queue_stats_get, 26.07)
   int rte_bbdev_queue_stats_get(uint16_t dev_id, uint16_t queue_id, struct rte_bbdev_stats *stats)

   /* Suggested */
   RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_bbdev_queue_stats_get, 26.07)
   __rte_experimental
   int rte_bbdev_queue_stats_get(uint16_t dev_id, uint16_t queue_id, struct rte_bbdev_stats *stats)
   ```

2. **Export macro uses incorrect version format** (lib/bbdev/rte_bbdev.c:818):
   `RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_bbdev_queue_stats_get, 26.07)` uses `26.07` which does not match DPDK's `YY.MM` release naming convention (should be `26.11` based on the release notes file name `release_26_11.rst`).

### Info

- Release notes correctly document the new API addition.
- Function provides proper error checking (NULL stats pointer, invalid dev_id, invalid queue_id).
- Documentation includes all required elements (parameters, return values).

---

## Patch 2/9: hash: fix GFNI stubs export

### Errors

None found.

### Warnings

None found.

### Info

- Correctly changes internal symbols (`RTE_EXPORT_INTERNAL_SYMBOL`) to public symbols (`RTE_EXPORT_SYMBOL`) so that inlined public API can call them without requiring `ALLOW_INTERNAL_API`.
- Removes `__rte_internal` tags from function declarations appropriately.

---

## Patch 3/9: test: uninline helper for forking

### Errors

None found.

### Warnings

None found.

### Info

- Moves forking helper functions from inline in header to compiled code in process.c, removing dependency on internal API from the header.
- Updates MAINTAINERS file appropriately.
- Meson build correctly adds process.c only for non-Windows builds (matches the `#ifndef RTE_EXEC_ENV_WINDOWS` guards).

---

## Patch 4/9: test/bonding: get MAC address with public API

### Errors

None found.

### Warnings

None found.

### Info

- Replaces direct `rte_eth_devices[].data->mac_addrs` access with `rte_eth_macaddr_get()` public API.
- Removes dependency on internal `ethdev_driver.h` header.
- Changes from pointer to value semantics (`struct rte_ether_addr` instead of `struct rte_ether_addr *`) are consistent with `rte_eth_macaddr_get()` usage.

---

## Patch 5/9: test/devargs: rely on net null driver

### Errors

None found.

### Warnings

None found.

### Info

- Standardizes test to use `net_null` driver instead of `net_ring`.
- Simplifies code by removing conditional logic for ring driver detection.
- Removes dependency on internal `bus_driver.h`, uses public `rte_bus.h` instead.

---

## Patch 6/9: test/vdev: find device with public API

### Errors

None found.

### Warnings

None found.

### Info

- Replaces internal bus `find_device` method with `RTE_DEV_FOREACH` public API.
- Simplifies device lookup logic significantly.
- Removes dependency on internal `bus_driver.h` and `rte_kvargs.h`.

---

## Patch 7/9: test: limit internal API usage

### Errors

None found.

### Warnings

None found.

### Info

- Removes global `ALLOW_INTERNAL_API` from app/test meson.build.
- Adds `#define ALLOW_INTERNAL_API` only to specific test files that genuinely need internal API for their validation purpose (process.c, test_devargs.c, test_external_mem.c, test_malloc.c, test_mempool.c, test_pdump.c, virtual_pmd.c).
- This is the correct pattern: tests should use public API by default, only enabling internal API where necessary for testing internals.

---

## Patch 8/9: ci: make ABI reference generation faster

### Errors

None found.

### Warnings

None found.

### Info

- Optimizes ABI reference generation by disabling unnecessary components (apps, examples, docs, developer checks).
- Uses `-Denable_apps=test,test-pmd` in one place and `-Ddisable_apps=*` in another; both achieve the goal of limiting what is built (the former keeps test and testpmd for later use, the latter in devtools script disables all apps as none are needed there).
- Performance optimization is appropriate for a CI workflow.

---

## Patch 9/9: ci: run reference binaries against current ABI

### Errors

None found.

### Warnings

1. **Bare integer comparison without explicit `!= 0`** (.ci/linux-build.sh:233, 260):
   ```bash
   [ "$failed" != "true" ]
   ```
   While this is acceptable in shell scripts, DPDK C code style prefers explicit comparisons. However, this is a bash script, not C code, so this is a style observation rather than a violation.

### Info

- Adds backward compatibility testing by running reference release binaries (testpmd and dpdk-test) against current libraries.
- Automatically generates list of tests to skip (tests added since the reference release) using `meson test --list`.
- Provides a `ABI_SKIP_TESTS` mechanism (mentioned in commit message) though the actual skipping uses `DPDK_TEST_SKIP` (likely the correct variable name in DPDK test infrastructure).
- The approach catches regressions on experimental or internal symbols promoted to stable without aliases.

---

## Summary

**Patch series correctness**: All patches are functionally correct with no resource leaks, use-after-free, or other memory safety issues identified.

**Primary findings**:
- Patch 1: Export macro version should be `26.11` not `26.07`; implementation should have `__rte_experimental` tag.

**No other errors found.** The series successfully removes unnecessary internal API usage from tests, adds a useful per-queue statistics API, fixes symbol export issues, and enhances CI ABI checking.