|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