|WARNING| [1/1] eal: add API to get CPU socket ID
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Sep 15 11:25:49 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/169560
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-15
# DPDK Patch Review
## Summary
This patch adds a new public API `rte_cpu_socket_id()` to map OS logical CPU IDs to NUMA socket IDs. The implementation is straightforward and reuses existing internal EAL infrastructure. However, there are several issues that need to be addressed.
---
## Errors
### 1. Missing `__rte_experimental` tag on new public API
New public API functions must be marked as `__rte_experimental` in the header file declaration. The function declaration in `rte_lcore.h` is missing this annotation.
**Fix:** Add `__rte_experimental` on the line immediately before the return type:
```c
/**
* Get the ID of the NUMA node for a CPU.
*
* This function maps an OS logical CPU ID to the NUMA node containing
* that CPU.
*
* @param cpu_id
* The OS logical CPU ID.
* @return
* The ID of cpu_id's NUMA node, or 0 if unavailable.
*/
__rte_experimental
unsigned int
rte_cpu_socket_id(unsigned int cpu_id);
```
### 2. Wrong `RTE_EXPORT_SYMBOL` variant for experimental API
The implementation uses `RTE_EXPORT_SYMBOL(rte_cpu_socket_id)` but should use `RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_cpu_socket_id, 26.11)` for experimental functions, including the DPDK version when first added.
**Fix:** In `lib/eal/common/eal_common_lcore.c`:
```c
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_cpu_socket_id, 26.11)
unsigned int
rte_cpu_socket_id(unsigned int cpu_id)
{
return eal_cpu_socket_id(cpu_id);
}
```
---
## Warnings
### 1. No bounds checking on input parameter
The function does not validate that `cpu_id` is within a valid range before passing it to `eal_cpu_socket_id()`. While `eal_cpu_socket_id()` may have internal bounds checking, the public API should document the expected valid range and what happens with out-of-bounds values.
The documentation states "or 0 if unavailable", but it's unclear whether this means:
- The CPU exists but socket ID is unknown
- The CPU ID is invalid/out of range
- Both
**Suggested improvement:** Either add explicit bounds checking and document the behavior, or clarify in the Doxygen comment what "unavailable" means. For example:
```c
/**
* Get the ID of the NUMA node for a CPU.
*
* This function maps an OS logical CPU ID to the NUMA node containing
* that CPU.
*
* @param cpu_id
* The OS logical CPU ID. Must be less than the number of CPUs on the system.
* @return
* The ID of cpu_id's NUMA node. Returns 0 if the cpu_id is invalid or
* if socket information is unavailable.
*/
```
### 2. Missing test coverage
New API functions should have tests added to `app/test` and ideally hooks in testpmd. No test additions are included in this patch.
**Recommended action:** Add a unit test in `app/test/test_lcore.c` (or create a new test file) that validates:
- Valid CPU IDs return expected socket IDs
- Behavior with edge cases (CPU 0, max valid CPU, invalid CPU)
- Consistency with existing `rte_lcore_to_socket_id()` for EAL-configured lcores
---
## Info
### 1. Release notes placement
The release notes addition is correctly placed in the "API Changes" section. This is appropriate for a new public API.
### 2. Implementation simplicity
The implementation correctly delegates to the existing `eal_cpu_socket_id()` internal function, maintaining consistency with EAL's existing socket lookup infrastructure. This is good design.
---
## Summary of Required Changes
**Must fix (Errors):**
1. Add `__rte_experimental` tag to header declaration
2. Change to `RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_cpu_socket_id, 26.11)` in implementation
**Should fix (Warnings):**
1. Clarify documentation for input validation and return value semantics
2. Add unit tests in `app/test/`
More information about the test-report
mailing list