|WARNING| [v2] ipc: fix multi-process disabling

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

**Error 1: Missing error check for `rte_mp_disable()` in `eal_mcfg_update_from_internal()`**

File: `lib/eal/common/eal_common_mcfg.c`, lines 69-70:

```c
if (internal_conf->no_shconf)
	rte_mp_disable(); /* multi-process requires shared config */
```

`rte_mp_disable()` returns `bool` to indicate success/failure. The return value is unchecked here. If `rte_mp_disable()` fails (returns `false`), the multi-process status remains in an inconsistent state - `no_shconf` is set but multi-process is not actually disabled. This could lead to IPC operations being attempted when shared config is unavailable.

**Suggested fix:**

```c
if (internal_conf->no_shconf) {
	if (!rte_mp_disable())
		EAL_LOG(WARNING, "Failed to disable multi-process support");
}
```

Or return an error if this is a critical failure condition.

---

**Error 2: Variable declaration after statement in `rte_mp_reply()`**

File: `lib/eal/common/eal_common_proc.c`, line 1358:

```c
int
rte_mp_reply(struct rte_mp_msg *msg, const char *peer)
{
	EAL_LOG(DEBUG, "reply: %s", msg->name);

	if (check_input(msg) != 0)
```

The `EAL_LOG` statement appears before all variable declarations are complete. While C99 allows mixed declarations and statements, DPDK style from the guidelines shows declarations should appear before statements in a function, with an empty line separating them. The removed variable declaration for `internal_conf` was after the `EAL_LOG`, and now there's a statement before the first check.

However, reviewing the actual code more carefully, there are no variable declarations in this function after the removal, so this is actually compliant with C99 style. No error.

*(Self-correction: Re-reading the function, the EAL_LOG is a statement and there are no subsequent variable declarations. This is acceptable C99 style. Removing this item.)*

---

## WARNINGS

**Warning 1: Inconsistent function naming convention**

File: `lib/eal/common/eal_common_proc.c`, lines 1420-1426:

The new function `eal_mp_is_disabled()` is declared in `eal_private.h` and defined in the common code, but it follows the `eal_*` internal naming pattern rather than `__rte_mp_*` or `rte_mp_*` pattern used by related functions in this file (`__rte_mp_enable()`, `rte_mp_disable()`).

While `eal_*` is appropriate for internal functions, consider using `__rte_mp_is_disabled()` for consistency with `__rte_mp_enable()` in the same subsystem, or document why the naming differs.

**Suggested fix:** Rename to `__rte_mp_is_disabled()` for consistency with `__rte_mp_enable()`.

---

**Warning 2: Missing documentation update**

The patch changes fundamental behavior - `rte_mp_disable()` is now called automatically when `--no-shconf` is used. This is a behavior change that should be documented in release notes as it affects when the multi-process disabled state is set.

Currently the commit message explains the change, but this should also appear in `doc/guides/rel_notes/release_*.rst` to inform users that `--no-shconf` now also disables multi-process support explicitly.

---

**Warning 3: Potential confusion in comment wording**

File: `lib/eal/common/eal_common_mcfg.c`, line 70:

```c
rte_mp_disable(); /* multi-process requires shared config */
```

The comment "multi-process requires shared config" might be clearer as "disable multi-process when shared config is disabled" to match the action being taken (disabling multi-process) rather than stating the requirement.

**Suggested fix:**

```c
rte_mp_disable(); /* disable multi-process when shared config is unavailable */
```

---

Review-Result: ERROR