|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

## Summary

This patch adds a new experimental API `rte_cpu_socket_id()` that maps an OS logical CPU ID to its NUMA socket. The implementation delegates to the existing internal `eal_cpu_socket_id()` function.

---

## Errors

None found.

---

## Warnings

### 1. Missing test coverage

**Issue:** New public API function lacks test coverage.

**Guideline:** "New API functions must have hooks in `app/testpmd` and tests in the functional test suite."

**Recommendation:** Add a test case in `app/test/` that exercises `rte_cpu_socket_id()`. The test should:
- Call `rte_cpu_socket_id()` with valid CPU IDs
- Verify the returned socket ID matches expected topology
- Test edge cases (invalid CPU ID, CPU not present)

Example test structure:
```c
static int
test_cpu_socket_id(void)
{
	unsigned int cpu_id, socket_id;
	
	/* Test valid CPU ID */
	cpu_id = 0;
	socket_id = rte_cpu_socket_id(cpu_id);
	TEST_ASSERT(socket_id != SOCKET_ID_ANY,
		"Failed to get socket for CPU 0");
	
	return TEST_SUCCESS;
}

REGISTER_FAST_TEST(cpu_socket_id_autotest, NOHUGE_OK, ASAN_OK,
	test_cpu_socket_id);
```

### 2. Potential testpmd integration

**Issue:** The patch does not add any testpmd hooks for the new API.

**Guideline:** "New API functions must have hooks in `app/testpmd`."

**Recommendation:** Consider adding a testpmd command to expose this functionality for interactive testing, such as:
```
testpmd> show cpu <cpu_id> socket
```
This would allow users to verify CPU-to-socket mapping at runtime.

---

## Info

### 1. API design consideration

The function returns `0` when the socket ID is unavailable, which could be ambiguous since `0` is also a valid socket ID on many systems. The existing `rte_lcore_to_socket_id()` has the same behavior, so this is consistent with DPDK conventions. No change required, but future API could consider returning `int` with negative values for errors.

### 2. Documentation clarity

The Doxygen comment states "return ... 0 if unavailable" but does not specify what "unavailable" means (invalid CPU ID, CPU offline, topology detection failed). Consider expanding the documentation to clarify the error semantics.

### 3. Implementation delegates correctly

The implementation correctly reuses the existing `eal_cpu_socket_id()` internal function, avoiding code duplication. Good practice.

---

## Checklist Review

- [x] Lines <=100 characters
- [x] Proper include order (no includes added)
- [x] `rte_` prefix on external symbols
- [x] No forbidden tokens
- [x] No usage of deprecated API
- [x] Function return type on own line
- [x] `__rte_experimental` alone on line, only in header
- [x] New public function has `RTE_EXPORT_EXPERIMENTAL_SYMBOL` macro in `.c` file
- [x] Release notes updated for new API
- [ ] New API has testpmd hooks (missing)
- [ ] New API has functional tests (missing)
- [x] Code and docs updated together
- [x] Documentation matches code behavior
- [x] Each commit compiles independently (single commit)

---

## Conclusion

The patch is **technically correct** with no correctness bugs or style violations. The primary gap is **test coverage** - the new API needs functional tests and ideally testpmd integration per DPDK contribution guidelines. These are **Warning**-level issues that should be addressed before merging.