|WARNING| [v7,24/24] eal: remove rte_atomicNN functions

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-06

# DPDK Patch Review: rte_atomic Removal Series (v7)

## Patch 01/24: bpf: use C11 atomics in BPF_ST_ATOMIC_REG

**Status:** No issues found.

**Summary:** Converts BPF atomic store macro from deprecated `rte_atomic*_add`/`rte_atomic*_exchange` to `rte_atomic_fetch_add_explicit`/`rte_atomic_exchange_explicit` with `seq_cst` ordering, matching the previous implicit semantics and the Linux kernel BPF interpreter.

---

## Patch 02/24: net/bonding: use stdatomic

**Status:** No issues found.

**Summary:** Converts `rx_marker_timer` and `warnings_to_show` from `rte_atomic*_t` to `RTE_ATOMIC(uint*)` and uses `rte_atomic_*_explicit()`. The Dekker-style update of `rx_marker_timer` (compare-exchange weak loop) and the release store on success are correct.

---

## Patch 03/24: net/nbl: remove unused rte_atomic16 field

**Status:** No issues found.

**Summary:** Removes dead code (`tx_current_queue` was never used). No behavioral change.

---

## Patch 04/24: net/ena: replace use of rte_atomicNN

**Errors:**

1. **Removed variable `ena_alloc_cnt` is not unused**  
   The commit message states "Remove variable `ena_alloc_cnt` is defined by not used. It is a leftover from previous memzone naming scheme." However, the code shows:
   ```c
   -rte_atomic64_t ena_alloc_cnt;
   ...
   -	alloc_cnt = rte_atomic32_add_return(&hwdev->os_dep.dma_alloc_cnt, 1);
   +	alloc_cnt = rte_atomic_fetch_add_explicit(&hwdev->os_dep.dma_alloc_cnt,
   +						  1, rte_memory_order_relaxed);
   	snprintf(z_name, sizeof(z_name), "%s_%d",
   		 hwdev->pcidev_hdl->name, alloc_cnt);
   ```
   The variable **is** used: the fetch-add result is still assigned to `alloc_cnt` and used in the `snprintf()`. The global `ena_alloc_cnt` *defined at file scope* is indeed removed (it appears only in the -/+ context lines as being moved to `hwdev->os_dep.dma_alloc_cnt`), but the commit message is misleading because it's unclear which `ena_alloc_cnt` is meant. The *file-scope global* was already gone in the current code (the code now uses `hwdev->os_dep.dma_alloc_cnt`); this patch only converts that existing usage to stdatomic. If the patch is also removing a stale file-scope declaration, that should be stated clearly and shown in the diff. As written, the commit message is confusing.

**Summary:** The actual conversion to stdatomic is correct (relaxed ordering is fine for a counter used only for unique naming). The commit message should clarify which variable is being removed or correct the claim if the removal already happened in an earlier commit.

---

## Patch 05/24: net/failsafe: convert to stdatomic

**Status:** No issues found.

**Summary:** Converts `refcnt[]` from `rte_atomic64_t` to `RTE_ATOMIC(uint64_t)` and the P/V macros from `rte_atomic64_set/read` to `rte_atomic_*_explicit()` with acquire (on P, which is the "take reference" operation) and release (on V, the "drop reference" operation) ordering. The pairing is correct.

---

## Patch 06/24: net/enic: do not use deprecated rte_atomic64

**Status:** No issues found.

**Summary:** Removes rte_atomic from statistics fields. As stated in the commit message, DPDK PMD statistics do not require atomicity under contention. Converting to plain `uint64_t` and replacing the atomic increments with plain `++` is correct.

---

## Patch 07/24: net/pfe: use ethdev linkstatus helpers

**Status:** No issues found.

**Summary:** Replaces open-coded link status read/write using deprecated `rte_atomic64_cmpset` with the existing `rte_eth_linkstatus_get()`/`rte_eth_linkstatus_set()` helpers. The helper usage is correct.

---

## Patch 08/24: net/sfc: replace rte_atomic with stdatomic

**Status:** No issues found.

**Summary:** Converts `restart_required` from `rte_atomic32_t` to `RTE_ATOMIC(bool)` and replaces `rte_atomic32_test_and_set`, `rte_atomic32_set`, and `rte_atomic32_init` with `rte_atomic_exchange_explicit` and `rte_atomic_store_explicit` using `seq_cst` ordering. The exchange-for-true / store-false pairing preserves the original semantics. Initialization is now a relaxed store (zero-init before device is on the list), which is safe. Link status update converted to `rte_eth_linkstatus_set()` is correct.

---

## Patch 09/24: crypto/ccp: replace use of rte_atomic64 with stdatomic

**Status:** No issues found.

**Summary:** Converts `dma_alloc_cnt` and `inuse` from `rte_atomic*_t` to `RTE_ATOMIC(uint32_t)` and uses `rte_atomic_*_explicit()` with relaxed ordering for the allocator counter and pool in-use tracking. Relaxed is correct for both (counter is for unique naming; pool in-use is checked under a lock or in cleanup paths where no concurrent access occurs).

---

## Patch 10/24: bus/dpaa: replace rte_atomic16 with stdatomic

**Status:** No issues found.

**Summary:** Converts `global_portals_used[]` from `rte_atomic16_t` to `RTE_ATOMIC(bool)` and replaces the `test_and_set`/`clear` pattern with `rte_atomic_exchange_explicit(..., true, acquire)` and `rte_atomic_store_explicit(..., false, release)`. Acquire on claim, release on free is the standard lock-like pairing and is correct.

---

## Patch 11/24: bus/fslmc: replace rte_atomic16 with stdatomic

**Errors:**

1. **Missing release ordering on the free side of `ref_count` compare-exchange**  
   In `dpaa2_put_qbman_swp()`:
   ```c
   +		rte_atomic_store_explicit(&dpio_dev->ref_count, 0,
   +					  rte_memory_order_release);
   ```
   This is correct.

   However, in `dpaa2_get_qbman_swp()`, the claim side:
   ```c
   +		if (rte_atomic_compare_exchange_strong_explicit(&dpio_dev->ref_count,
   +				&expected, 1, rte_memory_order_acquire,
   +				rte_memory_order_relaxed))
   ```
   The acquire ordering on success is correct (pairs with the release on free). The relaxed on failure is also correct (retry loop, no data dependency on the failed read).

   **However**, in the error path:
   ```c
   -		rte_atomic16_clear(&dpio_dev->ref_count);
   +		rte_atomic_store_explicit(&dpio_dev->ref_count, 0,
   +					  rte_memory_order_release);
   ```
   This release store is correct (same as the normal free path).

   Upon closer inspection, the ordering here is actually correct. The comment about "Missing release ordering" would be wrong because the release is present on both free paths. This is not an error.

**Revised:** No errors.

**Status:** No issues found.

**Summary:** Converts `in_use` flags from `rte_atomic16_t` to `RTE_ATOMIC(uint16_t)` and `ref_count` from `rte_atomic16_t` to `RTE_ATOMIC(uint16_t)`. Acquire on claim, release on free, with explicit handling of the failure cases. The pairing is correct.

---

## Patch 12/24: net/netvsc: replace rte_atomic32 with stdatomic

**Status:** No issues found.

**Summary:** Converts `rndis_req_id`, `rndis_pending`, and `rxbuf_outstanding` from `rte_atomic32_t` to `RTE_ATOMIC(uint32_t)` and replaces the increment/decrement/compare-exchange operations with `rte_atomic_*_explicit()` using `seq_cst` where the original code had implicit full barriers and `relaxed`/`release` where appropriate. The `rndis_pending` CAS loop in `rte_eth_bond_8023ad.c` (wait for response) is converted correctly.

---

## Patch 13/24: event/sw: convert from rte_atomic32 to stdatomic

**Status:** No issues found.

**Summary:** Converts `inflights` from `rte_atomic32_t` to `RTE_ATOMIC(uint32_t)` and uses `rte_atomic_*_explicit()` with `relaxed` for the fast-path read and `acquire`/`release` for the credit update (acquire when adding credits, release when returning credits). The ordering is appropriate for a credit-based flow control mechanism.

---

## Patch 14/24: bus/vmbus: convert from rte_atomic to stdatomic

**Status:** No issues found.

**Summary:** Converts `tbr->windex` from `rte_atomic32_t` to `RTE_ATOMIC(uint32_t)` and replaces the reservation/publish pattern (CAS loop to reserve slots, wait for previous producer, release-store to publish) with `rte_atomic_compare_exchange_weak_explicit()` (acquire on success, relaxed on failure) and `rte_wait_until_equal_32()` + `rte_atomic_store_explicit(..., release)` for the publish side. The memory ordering correctly implements the two-cursor lockless producer pattern: acquire on reservation to pair with the previous producer's release, release on publish to make the written data visible to the host and the next producer. The comment explaining the cast to suppress the packed-struct alignment warning is helpful.

---

## Patch 15/24: common/dpaax: remove unused atomic wrappers

**Status:** No issues found.

**Summary:** Removes dead code (compatibility macros for `rte_atomic*` that are not used anywhere in the current codebase). No behavioral change.

---

## Patch 16/24: net/bnx2x: convert from rte_atomic32 to stdatomic

**Status:** No issues found.

**Summary:** Converts `sc->scan_fp` from `rte_atomic32_t` to `RTE_ATOMIC(uint32_t)` and replaces `rte_atomic32_set` and `rte_atomic32_read` with `rte_atomic_store_explicit(..., seq_cst)` and `rte_atomic_load_explicit(..., seq_cst)`. The use of `seq_cst` is conservative and preserves the previous full-barrier semantics of the deprecated API.

---

## Patch 17/24: bus/fslmc: replace rte_atomic32 with stdatomic

**Status:** No issues found.

**Summary:** Converts the `atomic_t` typedef from `rte_atomic32_t` to `RTE_ATOMIC(uint32_t)` and replaces the macros for `atomic_set`, `atomic_inc`, and `atomic_dec_and_test` with `rte_atomic_*_explicit()` equivalents using `relaxed` for the set and `seq_cst` for the inc and dec_and_test. The ordering choices match the original semantics.

---

## Patch 18/24: drivers/event: replace rte_atomic32 in selftests

**Status:** No issues found.

**Summary:** Converts `total_events` from `rte_atomic32_t *` to `__rte_atomic uint32_t *` in the event driver selftests (dpaa2, octeontx) and replaces `rte_atomic32_read`, `rte_atomic32_set`, `rte_atomic32_sub`, and `rte_atomic32_exchange` with `rte_atomic_*_explicit()`. The dequeue-side decrements use `relaxed` in dpaa2 (appropriate for a simple counter) and `release` in octeontx (preserving the previous full-barrier semantics). The control-plane loads are `relaxed`. The conversions are correct.

---

## Patch 19/24: net/hinic: replace rte_atomic32 with stdatomic

**Status:** No issues found.

**Summary:** Converts `dma_alloc_cnt` and `inuse` from `rte_atomic32_t` to `RTE_ATOMIC(uint32_t)` and uses `rte_atomic_*_explicit()` with relaxed ordering. Relaxed is correct for the allocator counter (unique naming) and the pool in-use tracking (logged in cleanup, not used for synchronization).

---

## Patch 20/24: net/txgbe: replace rte_atomic32 with stdatomic

**Warnings:**

1. **Inverted return value logic in rte_atomic32_test_and_set has been corrected**  
   The commit message states "Note: The previous `rte_atomic32_test_and_set` return value was inverted relative to what this code expected; this patch incidentally corrects that." This is a correctness fix hidden inside an atomic conversion patch. It should be a separate patch with a `Fixes:` tag, landed before the conversion, so that the fix can be backported to stable branches independently of the API migration. As written, a backport of the fix would pull in the entire stdatomic conversion, which is not appropriate for a stable branch.

**Summary:** The conversion from `rte_atomic32_t` to `RTE_ATOMIC(bool)` for `swfw_busy` is correct: the exchange-for-true on the acquire side and store-false-release on the drop side implement a simple spinlock. However, bundling a logic fix (inverted return value) with an API migration patch makes backporting harder. The fix should be split out.

---

## Patch 21/24: net/vhost: use stdatomic instead of rte_atomic32

**Status:** No issues found.

**Summary:** Converts `allow_queuing`, `while_queuing`, `started`, and `dev_attached` from `rte_atomic32_t` to `RTE_ATOMIC(uint32_t)` and replaces `rte_atomic32_read`/`rte_atomic32_set` with `rte_atomic_*_explicit()`. The Dekker-style mutual-visibility pattern on `allow_queuing`/`while_queuing` now uses explicit `seq_cst` stores and loads, which is stronger than the previous plain volatile stores/loads but correct (and closes a latent ordering hole on weak-memory architectures). The data-path exit store of `while_queuing=0` is `release`, ordering preceding slot accesses before the control thread observes the idle state. The `started` and `dev_attached` flags are control-plane and use `relaxed`. The conversions are correct.

---

## Patch 22/24: vdpa/ifc: replace rte_atomic32 with stdatomic

**Status:** No issues found.

**Summary:** Converts `started`, `dev_attached`, and `running` from `rte_atomic32_t` to `RTE_ATOMIC(bool)` and replaces `rte_atomic32_read`/`rte_atomic32_set` with `rte_atomic_*_explicit(..., seq_cst)`. The use of `seq_cst` is conservative and preserves the original full-barrier semantics. The flags are grouped with existing `bool` fields to reduce padding.

---

## Patch 23/24: test/atomic: drop old rte_atomicNN tests

**Status:** No issues found.

**Summary:** Removes all test code for the now-deleted `rte_atomic16_*`, `rte_atomic32_*`, and `rte_atomic64_*` functions. Only the 128-bit compare-and-swap test remains (which is outside the scope of the deprecated API). The test is now a simple