|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