|WARNING| eal: fix multi-process disabling
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Nov 3 21:07:33 CET 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172602
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-03
# DPDK Patch Review
## ERRORS
**1. Missing `static` on internal helper function**
Line: `lib/eal/common/eal_common_proc.c:1420`
```c
bool
eal_mp_is_enabled(void)
```
This new function `eal_mp_is_enabled()` is declared in `eal_private.h` as an internal EAL function but is exported without any annotation. In `lib/eal/windows/eal.c` at line 84, the same function is defined without `static` and without any export annotation.
Since this is an internal function shared between EAL components (used across `lib/eal/common/` and `lib/eal/windows/`), it must use `RTE_EXPORT_INTERNAL_SYMBOL` in the `.c` file where it is defined, or be made `static` if it's file-local. The Linux implementation in `eal_common_proc.c` needs the export macro; the Windows stub in `eal.c` also needs it for consistency.
**Suggested fix:**
```c
/* In lib/eal/common/eal_common_proc.c */
RTE_EXPORT_INTERNAL_SYMBOL(eal_mp_is_enabled)
bool
eal_mp_is_enabled(void)
{
/* ... */
}
/* In lib/eal/windows/eal.c */
RTE_EXPORT_INTERNAL_SYMBOL(eal_mp_is_enabled)
bool
eal_mp_is_enabled(void)
{
return false;
}
```
---
**2. Race condition: multi-process status checked without synchronization against concurrent `rte_mp_disable()`**
Lines: `lib/eal/common/eal_common_dev.c:270`, `lib/eal/common/eal_common_dev.c:427`
```c
bool do_mp = eal_mp_is_enabled();
```
The new `eal_mp_is_enabled()` reads `mcfg->mp_status` with `rte_memory_order_relaxed`. However, `rte_mp_disable()` writes to the same field with `rte_memory_order_release` (via `set_mp_status()` in `eal_common_proc.c`). A thread calling `rte_dev_probe()` or `rte_dev_remove()` can read a stale `mp_status` value before the release-store from `rte_mp_disable()` becomes visible.
This is a correctness issue: if multi-process is disabled concurrently with a hotplug operation, the hotplug code may attempt IPC on a closed channel or skip IPC when it should still be attempted, depending on the race outcome.
**Suggested fix:**
The load in `eal_mp_is_enabled()` should use `rte_memory_order_acquire` to synchronize with the release-store in `set_mp_status()`:
```c
bool
eal_mp_is_enabled(void)
{
struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
return rte_atomic_load_explicit(&mcfg->mp_status,
rte_memory_order_acquire) != MP_STATUS_DISABLED;
}
```
---
## WARNINGS
**3. `eal_mp_is_enabled()` missing Doxygen documentation**
Line: `lib/eal/common/eal_private.h:746`
```c
/**
* Check whether multi-process is enabled.
*/
bool eal_mp_is_enabled(void);
```
This comment does not document the return value or provide sufficient detail for maintainers. Internal functions should have complete Doxygen documentation.
**Suggested improvement:**
```c
/**
* Check whether multi-process IPC is enabled.
*
* Multi-process support is disabled when --no-shconf is used,
* or after a call to rte_mp_disable().
*
* @return
* true if multi-process is enabled, false otherwise.
*/
bool eal_mp_is_enabled(void);
```
---
**4. Release notes missing**
This patch fixes user-visible behavior: hotplug devargs are no longer subject to IPC message size limits when multi-process is disabled, and `rte_mp_disable()` now prevents IPC attempts in hotplug operations.
The fix should be documented in `doc/guides/rel_notes/release_*.rst` under "Fixed Issues in This Release" (or equivalent section for the target release).
**Suggested addition:**
```rst
* **eal: fixed multi-process checks in hotplug operations**
Hotplug operations (``rte_dev_probe()``, ``rte_dev_remove()``) now correctly
respect the multi-process disabled state set by ``rte_mp_disable()`` or
``--no-shconf``, avoiding IPC attempts when secondary processes cannot attach.
```
---
Review-Result: ERROR
More information about the test-report
mailing list