|WARNING| sched: fix data race in RED random number generation

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

**Error: Race condition on shared variable**

Line: `static int rte_red_init_done;`

This variable is checked and set without synchronization in `rte_red_config_init()`:
```c
if (!rte_red_init_done) {
    __rte_red_init_tables();
    rte_red_init_done = 1;
}
```

If multiple threads call `rte_red_config_init()` concurrently, they can race on this flag. Both threads could see `rte_red_init_done == 0`, both call `__rte_red_init_tables()`, and both set the flag. This is a classic check-then-act race condition on shared mutable state.

The variable is read and written from different threads without atomic operations or memory ordering.

**Fix:** Use `rte_atomic_*_explicit()` with appropriate ordering:
```c
static uint32_t rte_red_init_done;

// In rte_red_config_init():
if (!rte_atomic_load_explicit(&rte_red_init_done, rte_memory_order_acquire)) {
    __rte_red_init_tables();
    rte_atomic_store_explicit(&rte_red_init_done, 1, rte_memory_order_release);
}
```

Or use atomic compare-exchange for proper once-initialization:
```c
uint32_t expected = 0;
if (rte_atomic_compare_exchange_strong_explicit(&rte_red_init_done, &expected, 1,
        rte_memory_order_acq_rel, rte_memory_order_acquire)) {
    __rte_red_init_tables();
}
```

---

## WARNINGS

**Warning: Removed symbols not listed in API Changes section**

The release notes document the removal under "Removed Items" but the "API Changes" section (line 232 in the patch context) is not updated. Removing exported symbols (`rte_red_rand_val`, `rte_red_rand_seed`, `rte_fast_rand()`) is an API change and should be documented in both sections per the guidelines.

**Fix:** Add an entry to the API Changes section:
```rst
* sched: Removed ``rte_fast_rand()``, ``rte_red_rand_val``, and
  ``rte_red_rand_seed`` which were internal implementation details
  inadvertently exposed in the public header.
```

---

**Warning: Missing ABI versioning for symbol removal**

The patch removes exported symbols (`RTE_EXPORT_SYMBOL(rte_red_rand_val)` and `RTE_EXPORT_SYMBOL(rte_red_rand_seed)`) without ABI versioning. While the commit message states these "were never meant to be part of the API," they were exported symbols and their removal is an ABI break.

Per DPDK policy, ABI breaks are only allowed on LTS releases (X.11 versions). This patch targets 26.11, which would be an LTS release, so the timing is correct. However, the removal should still be mentioned in the deprecation notice section or have been announced in a prior release's deprecation.rst.

The commit message acknowledges there is no prior deprecation notice but justifies it as these being internal symbols. This is acceptable for truly internal symbols, but the presence of `RTE_EXPORT_SYMBOL` suggests they were part of the binary interface, intentionally or not.

**Recommendation:** Verify this is going into an LTS release (26.11), which allows ABI breaks. If targeting a non-LTS release, this would need deprecation notice + versioning.

---

**Warning: Release notes missing context about behavioral change**

The release note mentions removal of `rte_fast_rand()` but does not explain:
- Why it was removed (weak PRNG, thread-safety issue)
- What replaced it (`rte_rand()`)
- Any performance implications (if `rte_rand()` is slower, users should know)

The commit message provides this context but the release notes should be self-contained.

**Fix:** Expand the release note:
```rst
* sched: Removed the private random number generator used by RED.
  The inline function ``rte_fast_rand()`` and associated state variables
  ``rte_red_rand_val`` and ``rte_red_rand_seed`` have been removed.
  RED now uses the thread-safe ``rte_rand()`` function, which provides
  better randomness and eliminates data races on the shared PRNG state.
```

---

Review-Result: ERROR