|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: eal: add API to get CPU socket ID

## Summary
This patch adds `rte_cpu_socket_id()` to map OS logical CPU IDs to NUMA socket IDs. The implementation is straightforward, delegating to the existing internal `eal_cpu_socket_id()` function.

---

## Errors

None found.

---

## Warnings

**1. Missing test coverage (required for new API)**

New API functions must have tests in the functional test suite (`app/test`).
This patch adds a public experimental API but includes no test to exercise it.

Add a test case to `app/test/test_lcore.c` or similar that:
- Calls `rte_cpu_socket_id()` on valid CPU IDs
- Verifies the return value is reasonable (0 or a valid socket ID)
- Tests edge cases (invalid CPU ID, CPU ID beyond system max)

**2. Missing testpmd integration**

New API functions should have hooks in `app/testpmd` for validation and demonstration.

Consider adding a command to query socket ID by CPU ID, similar to how testpmd displays lcore information.

**3. API return value for invalid input**

The documentation states "The ID of cpu_id's NUMA node, or 0 if unavailable."
Returning `0` for both "socket 0" and "error/unavailable" is ambiguous.
The caller cannot distinguish whether CPU ID 0 is genuinely on socket 0
or whether the lookup failed.

Suggest either:
- Return `SOCKET_ID_ANY` (already defined as `(unsigned int)-1`) for invalid/unavailable
- Document that the caller must validate `cpu_id` is within the system's CPU range before calling

If this behavior matches `eal_cpu_socket_id()` by design, document that explicitly.

**4. Missing API usage example in documentation**

While the Doxygen comment explains *what* the function returns,
it does not show *when* or *how* an application would use it.

Consider adding a usage note explaining the intended use case (e.g., building memory policies, querying topology for non-EAL threads).

---

## Info

**1. Release notes wording**

The release notes entry is functional but could be more specific about the use case:

Current:
```
* eal: Added experimental ``rte_cpu_socket_id()`` to map an OS logical
  CPU ID to the NUMA socket containing that CPU.
```

Suggested (more actionable):
```
* eal: Added experimental ``rte_cpu_socket_id()`` to query NUMA topology
  by OS logical CPU ID, enabling socket lookups without requiring an EAL
  lcore ID or thread affinity.
```

**2. Doxygen `@param` formatting**

The Doxygen comment is clear, but the parameter description could be more explicit:

Current:
```c
 * @param cpu_id
 *   The OS logical CPU ID.
```

Suggested:
```c
 * @param cpu_id
 *   The OS logical CPU ID (as from sched_getcpu() or pthread_getaffinity_np()).
```

This clarifies what "OS logical CPU ID" means in practice.

---

## Conclusion

The patch is **functionally correct** but incomplete per DPDK contribution guidelines:
- **Missing tests** (Warning - required for new API)
- **Missing testpmd integration** (Warning - required for new API)
- **Ambiguous error return value** (Warning - design issue)

These should be addressed before the patch is merged.