|WARNING| [v2,1/1] eal: add API to get CPU socket ID

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch: eal: add API to get CPU socket ID

### Summary
This patch adds `rte_cpu_socket_id()` to allow applications to map an OS logical CPU ID to its NUMA socket without requiring an EAL lcore ID. The implementation delegates to the existing internal `eal_cpu_socket_id()` helper.

---

## Errors

None found.

---

## Warnings

### 1. Missing Doxygen `@param` return value range

**File:** `lib/eal/include/rte_lcore.h`

The Doxygen comment states the function "return[s] The ID of cpu_id's NUMA node, or 0 if unavailable."
This implies that socket ID 0 is ambiguous -- it could mean either "socket 0" or "unavailable."
The documentation should clarify what happens when `cpu_id` is out of range or the topology cannot be determined,
and whether socket 0 is a valid return value on systems where socket 0 exists.

**Suggested fix:**
```c
/**
 * @return
 *   The ID of cpu_id's NUMA node.
 *   Returns SOCKET_ID_ANY if the socket ID cannot be determined
 *   or cpu_id is invalid.
 */
```
Alternatively, if the function truly returns 0 for unavailable cases
and this is distinguishable from socket 0,
the documentation must explicitly state that socket ID 0 is never used on DPDK systems
or that the caller must validate `cpu_id` separately.

### 2. No validation of `cpu_id` parameter documented

**File:** `lib/eal/include/rte_lcore.h`

The function does not document what happens when `cpu_id` is out of range
(e.g., larger than the number of CPUs on the system).
Does `eal_cpu_socket_id()` validate the input?
If not, the API should either:
- Document that passing an invalid `cpu_id` is undefined behavior, or
- Return a sentinel value (e.g., `SOCKET_ID_ANY` or `(unsigned int)-1`) for invalid inputs

**Suggested fix:**
Add a note to the Doxygen:
```c
/**
 * @param cpu_id
 *   The OS logical CPU ID.
 *   The behavior is undefined if cpu_id does not correspond to a valid CPU.
 */
```
Or add bounds checking if `eal_cpu_socket_id()` does not already do so.

### 3. Missing testpmd hook

**File:** Not applicable (testpmd not modified)

New API functions should have hooks in `app/testpmd` for manual verification.
A simple command like `show cpu socket <cpu_id>` would allow developers to test the function
and verify topology detection on various platforms.

**Suggested action:**
Add a testpmd command in a follow-up patch or in this series
to exercise `rte_cpu_socket_id()`.

### 4. Missing functional test

**File:** Not applicable (app/test not modified)

The patch does not add a test in `app/test` to verify `rte_cpu_socket_id()`.
At minimum, the test should:
- Call `rte_cpu_socket_id()` with a valid CPU ID and verify the result matches `eal_cpu_socket_id()`
- Call it with an out-of-range CPU ID and verify it does not crash

**Suggested action:**
Add a test in `app/test/test_lcore.c` (or a new test file) that validates the new function.

---

## Info

### 1. Consider adding `SOCKET_ID_ANY` return for invalid input

**File:** `lib/eal/common/eal_common_lcore.c`

The existing `rte_lcore_to_socket_id()` returns `SOCKET_ID_ANY` for invalid lcore IDs
(see `eal_common_lcore.c`).
For consistency, `rte_cpu_socket_id()` could do the same for invalid CPU IDs.

This would require adding bounds checking:
```c
unsigned int
rte_cpu_socket_id(unsigned int cpu_id)
{
	if (cpu_id >= /* max CPUs */)
		return SOCKET_ID_ANY;
	return eal_cpu_socket_id(cpu_id);
}
```

This is not strictly required if the API contract is "undefined behavior for invalid `cpu_id`,"
but it would be more defensive and user-friendly.

### 2. Release notes wording

**File:** `doc/guides/rel_notes/release_26_11.rst`

The release notes entry is concise and correct.
Minor wording suggestion for clarity:

```rst
* eal: Added experimental ``rte_cpu_socket_id()`` to map an OS logical
  CPU ID to its NUMA socket, independent of EAL lcore configuration.
```

This emphasizes the key benefit (no lcore dependency) mentioned in the commit message.

---

## Overall Assessment

**Correctness:** The implementation appears correct.
It properly delegates to the existing internal function and is marked experimental.

**Style:** Code style is compliant. The `RTE_EXPORT_EXPERIMENTAL_SYMBOL` macro is correctly placed.

**Process:** Mailmap, release notes, and experimental marking are all correct.

**Main concerns:**
- Ambiguous return value documentation (socket 0 vs unavailable)
- No testpmd hook or functional test (per guidelines, new API should have both)
- Unclear behavior for invalid `cpu_id`

These are **Warnings**, not **Errors**, but should be addressed before the API is promoted from experimental.