|WARNING| [v4,9/9] power: add lcore ID check for PMD mgmt
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Jun 15 10:26:37 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/165460
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-15
# DPDK Patch Review
## Patch 1/9: power: add a common macro to verify lcore ID
### Errors
**RTE_POWER_VALID_LCOREID_OR_ERR_RET macro has implicit comparison on lcore role**
The macro uses implicit truthiness checks on `rte_eal_lcore_role()` return values rather than explicit comparison.
DPDK coding style requires explicit comparison against enum/integer values:
```c
/* Current code */
#define RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, retval) do { \
if (rte_eal_lcore_role(lcore_id) != ROLE_RTE && \
rte_eal_lcore_role(lcore_id) != ROLE_SERVICE) { \
POWER_LOG(ERR, "lcore id %u is invalid", lcore_id); \
return retval; \
} \
} while (0)
/* Should be */
#define RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, retval) do { \
enum rte_lcore_role_t role = rte_eal_lcore_role(lcore_id); \
if (role != ROLE_RTE && role != ROLE_SERVICE) { \
POWER_LOG(ERR, "lcore id %u is invalid", lcore_id); \
return retval; \
} \
} while (0)
```
Wait - reviewing this again, `rte_eal_lcore_role()` returns `enum rte_lcore_role_t` and the comparisons `!= ROLE_RTE` and `!= ROLE_SERVICE` are explicit enum comparisons. The condition is a boolean combination of two explicit comparisons. This is correct under DPDK style - the result of `!=` is `bool` and using `&&` on boolean values does not require further explicit comparison.
No issue here after all. Removing this item.
### Warnings
None.
---
## Patch 2/9: power/cpufreq: add the lcore ID verification to framework
### Errors
None.
### Warnings
None.
---
## Patch 3/9: power/acpi: remove redundant lcore ID checks
### Errors
None.
### Warnings
None.
---
## Patch 4/9: power/amd_pstate: remove redundant lcore ID checks
### Errors
None.
### Warnings
None.
---
## Patch 5/9: power/cppc: remove redundant lcore ID checks
### Errors
None.
### Warnings
None.
---
## Patch 6/9: power/intel_pstate: remove redundant lcore ID checks
### Errors
None.
### Warnings
None.
---
## Patch 7/9: power/kvm_vm: remove redundant lcore ID checks
### Errors
None.
### Warnings
None.
---
## Patch 8/9: power: allow the service core to config power QoS
### Errors
None.
### Warnings
**Release notes formatting: extra blank lines**
The release notes diff shows:
```rst
+ in power core. The power QoS library also updated its lcore validation to
+ use this macro, so service cores (``ROLE_SERVICE``) are now permitted.
+
+
```
There are two extra blank lines after the last sentence. RST files should have only one blank line between sections/paragraphs.
---
## Patch 9/9: power: add lcore ID check for PMD mgmt
### Errors
None.
### Warnings
None.
---
## Summary
**Patches with issues:**
- Patch 8/9: Release notes formatting (Warning)
**Clean patches:**
- Patch 1/9
- Patch 2/9
- Patch 3/9
- Patch 4/9
- Patch 5/9
- Patch 6/9
- Patch 7/9
- Patch 9/9
The series correctly consolidates lcore ID validation into a common macro and removes redundant checks across all cpufreq drivers. The approach is sound and consistent. The only issue is a minor formatting problem in the release notes.
More information about the test-report
mailing list