|WARNING| [10/10] ci: run reference unit tests against ABI

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-07-17

# DPDK Patch Review

## PATCH 01/10: bbdev: fix stats aggregation from queues

### Errors

**Statistics accumulation using `=` instead of `+=`:**
The loop at line 759 uses `+=` for `enqueue_status_count` array accumulation:
```c
stats->enqueue_status_count[i] += q_stats->enqueue_status_count[i];
```
This is correct - it properly accumulates the counter across queues.

**No correctness issues found in this patch.**

### Warnings

**Missing release notes update:**
This patch fixes a correctness bug (stats not aggregated) in the bbdev library's device-level stats retrieval. This is a user-visible behavior change that should be documented.

The patch includes:
- `Fixes:` tag pointing to the commit that introduced the incomplete aggregation
- `Cc: stable at dpdk.org` for backporting

But it does NOT update `doc/guides/rel_notes/release_26_07.rst` to document the fix. API behavior changes (even bug fixes that restore expected behavior) should appear in release notes so users know the stats they retrieve will now be complete.

---

## PATCH 02/10: bbdev: add per-queue statistics API

### Errors

**None.**

### Warnings

**None.**

### Info

The new `rte_bbdev_queue_stats_get()` function is properly marked `__rte_experimental`, has complete Doxygen documentation, correct `RTE_EXPORT_EXPERIMENTAL_SYMBOL` placement, and the release notes document the addition. The implementation validates inputs correctly. This is a well-structured API addition.

---

## PATCH 03/10: hash: fix GFNI stubs export

### Errors

**None.** The change from `RTE_EXPORT_INTERNAL_SYMBOL` to `RTE_EXPORT_SYMBOL` for the GFNI stub functions is correct. These stubs are called through the public (inlined) GFNI API, so applications linking without `ALLOW_INTERNAL_API` must be able to resolve them. Internal symbols cannot be part of public API call paths.

### Warnings

**None.**

---

## PATCH 04/10: test: uninline helper for forking

### Errors

**None.**

### Warnings

**None.** Moving `process_dup()` and related helpers from an inline header to a `.c` file is appropriate - these are not performance-critical and using internal API. The `MAINTAINERS` update includes the new `process.c` file.

---

## PATCH 05/10: test/bonding: get MAC address with public API

### Errors

**None.** Replacing direct `rte_eth_devices[].data->mac_addrs` access with `rte_eth_macaddr_get()` is the correct approach. The code changes local variables from pointers to values, which is appropriate since `rte_eth_macaddr_get()` copies the address.

### Warnings

**None.**

---

## PATCH 06/10: test/devargs: check driver presence with public API

### Errors

**None.** The patch removes the direct call to `rte_bus_find_by_name()` and `vdev_bus->parse()` (internal bus API) and replaces it with `rte_devargs_parse()`, which is the correct public API for validating devargs strings. The pattern is appropriate: allocate a `rte_devargs` on the stack, call `rte_devargs_parse()`, check return, then `rte_devargs_reset()`.

### Warnings

**None.**

---

## PATCH 07/10: test/vdev: find device with public API

### Errors

**None.** Removing the internal `bus->find_device()` and `rte_kvargs` usage in favor of the public `RTE_DEV_FOREACH()` iterator API is correct. The simplified `find_vdev_by_name()` helper directly compares device names instead of constructing kvargs, which is clearer and uses only public API.

### Warnings

**None.**

---

## PATCH 08/10: test: limit internal API usage

### Errors

**None.** The patch correctly moves from global `-DALLOW_INTERNAL_API` in `app/test/meson.build` to selective `#define ALLOW_INTERNAL_API` at the top of only those test files that actually require internal API access. This reduces the blast radius and ensures tests that don't need internal API can't accidentally depend on it.

Files that need internal API:
- `process.c` - uses `RTE_EAL_DEVARGS_FOREACH`, `rte_devargs_type_count()`, `rte_eal_driver_path_count()`, `RTE_EAL_DRIVER_PATH_FOREACH`
- `test_devargs.c` - includes `rte_class.h` (internal)
- `test_external_mem.c` - uses `eal_memalloc_*` internal functions
- `test_malloc.c` - uses `malloc_heap_*` internal functions
- `test_mempool.c` - uses `rte_mempool_ops_table` internal global
- `test_pdump.c` - uses `rte_pdump_*` which may be internal depending on configuration
- `virtual_pmd.c` - uses `ethdev_driver.h` internal header

This is appropriate.

### Warnings

**None.**

---

## PATCH 09/10: ci: make ABI reference generation faster

### Errors

**None.**

### Warnings

**None.** The optimization is reasonable: when generating ABI reference, disable apps (except for the fix in PATCH 10, but that's a separate concern), examples, tests, docs, and header checks. The reference only needs the drivers and libraries built and installed. This reduces build time without affecting ABI validation correctness.

---

## PATCH 10/10: ci: run reference unit tests against ABI

### Errors

**None.**

### Warnings

**Hardcoded temporary repository and branch for testing:**
```bash
REF_GIT_REPO: https://github.com/david-marchand/dpdk
REF_GIT_TAG: 26.03
```
The commit message notes this is temporary ("FIXME: for now, use a branch of mine with backports on 26.03") due to bugs 1967 (devargs/vdev tests broken against reference ABI). Before merge, this should be reverted to the official repository and tag, or the necessary fixes should be applied to the reference branch first.

**FIXME comment in script forces cache invalidation:**
```bash
# FIXME: force regeneration in CI for this patch
if [ ! -e reference/usr/local/bin/dpdk-test ]; then
    rm -rf reference
fi
```
This is a temporary measure for testing the patch and should be removed before merge.

### Info

The ABI test approach is sound: build `dpdk-test` from the reference release, then run it against the current release's libraries/drivers via `LD_LIBRARY_PATH`. Tests added since the reference are skipped via `ABI_SKIP_TESTS`. This catches ABI breaks on experimental/internal symbols promoted to stable or changes in behavior that break consumers built against the reference ABI.

---

## Summary

### Errors: 0

### Warnings: 3

1. **PATCH 01**: Missing release notes entry for user-visible bug fix
2. **PATCH 10**: Temporary hardcoded test repository must be reverted before merge
3. **PATCH 10**: FIXME comment forcing cache invalidation should be removed

### Info: 2

All patches are well-structured. The series successfully removes unnecessary internal API usage from tests, fixes real bugs in bbdev stats aggregation, corrects GFNI stub symbol export, and improves CI ABI checking. The only issues are documentation (missing release note) and temporary test scaffolding that should be cleaned up before merge.