|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

### 1. Missing `RTE_EXPORT_SYMBOL` removal causes linker errors

**File:** `lib/sched/rte_red.c`  
**Lines:** 11-14 (removed lines)

The patch removes the variable definitions for `rte_red_rand_val` and `rte_red_rand_seed` along with their `RTE_EXPORT_SYMBOL()` macros, but the header file `rte_red.h` still declares these as `extern` variables (lines 38-39 in the original). This will cause linker errors for any code that references these symbols.

**Fix:** The patch must also remove the `extern` declarations from `lib/sched/rte_red.h`:
```c
-extern uint32_t rte_red_rand_val;
-extern uint32_t rte_red_rand_seed;
```

### 2. ABI break without proper versioning or deprecation notice

The removal of exported symbols `rte_red_rand_val` and `rte_red_rand_seed` is an ABI break. The patch acknowledges in the commit message that "There is no prior deprecation notice" but proceeds anyway. According to DPDK policy:

- Removing exported symbols requires a deprecation notice in a prior release
- ABI changes require proper versioning for LTS compatibility
- The release notes should document this as an ABI change, not just mention removal of `rte_fast_rand()`

**Fix:** Either:
1. Add a deprecation notice in the current release and defer removal to the next release, OR
2. Provide clear justification in the commit message for why the emergency removal without deprecation is acceptable (e.g., these were never intended to be public API and have no known external users)

---

## WARNINGS

### 1. Incomplete release notes

**File:** `doc/guides/rel_notes/release_26_11.rst`  
**Lines:** 229-230

The release notes only mention removal of `rte_fast_rand()` but omit the removal of the two exported global variables `rte_red_rand_val` and `rte_red_rand_seed`. The note should be comprehensive.

**Suggested fix:**
```rst
* sched: Removed the private random number generator used by RED: the
  inline function ``rte_fast_rand()`` and the global variables
  ``rte_red_rand_val`` and ``rte_red_rand_seed``.
```

### 2. Release notes placement

The removal is documented under "Removed Items" but should also have an entry under "API Changes" since it removes exported symbols. Both the function and variables were exported (via inline and `RTE_EXPORT_SYMBOL`).

---

## INFO

### 1. Improved randomness quality

The change from a weak custom LCG (`rte_fast_rand()`) to `rte_rand()` improves randomness quality. The old generator used LCG parameters from a Microsoft implementation with a right-shift of 10 bits, producing only 22 bits of randomness. `rte_rand()` uses a proper seeded LFSR with better distribution, making the RED drop probability more statistically sound.

### 2. Thread safety improvement

The old implementation had a data race: `rte_red_rand_seed` and `rte_red_rand_val` were global variables modified without synchronization. Multiple threads calling `__rte_red_drop()` would race on updates to `rte_red_rand_seed`. The new `rte_rand()` is thread-safe (uses per-lcore state), eliminating this race condition. This justifies the "data race" claim in the commit title.

### 3. Initialization cleanup

The removal of initialization code in `rte_red_config_init()` (lines setting `rte_red_rand_seed` and `rte_red_rand_val`) is correct since `rte_rand()` handles its own initialization.

---

Review-Result: ERROR