Re: [PATCH V2 00/15] power: unify and improve lcore ID verification

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 7 May 2026 10:42:15 +0800
Huisong Li <lihuisong@huawei.com> wrote:

> This patch series reworks the lcore ID verification logic within the
> power library to ensure consistency and improve maintainability.
> 
> Currently, various cpufreq drivers implement their own lcore ID checks,
> which are limited to simple range validation and involve significant
> code duplication. Moreover, these checks do not account for whether the
> core is actually managed by the application.
> 
> For the verification in cpufreq-related APIs and power QoS APIs, although
> service cores do not typically invoke these APIs, they may operate in
> polling modes where power management is required. To maintain compatibility
> with applications using service cores, the validation logic now explicitly
> allows both ROLE_RTE and ROLE_SERVICE.
> 
> But the lcore ID in the pmd_mgmt library must be ROLE_RTE because it is
> mainly used together with the data plane of ethdev PMD. So use
> rte_lcore_is_enabled to verify.
> 
> Key Changes:
> 1. Add lcore role verification to individual cpufreq drivers.
> 2. Introduces a unified macro in the power library to standardize lcore ID
>    checks.
> 3. Moves verification logic from individualdrivers to the framework level.
>    This reduces code duplication.
> 4. Allow the service cores to configure power QoS.
> 5. Use rte_lcore_is_enabled to verfify the lcore ID in pmd_mgmt.
> 
> ---
> v2:
>  - Allow the service cores to set power API.
> 
> ---
> 
> Huisong Li (15):
>   eal: add interface to check if lcore is EAL managed
>   power/kvm_vm: validate lcore role in cpufreq API
>   power/acpi_cpufreq: validate lcore role in cpufreq API
>   power/amd_pstate: validate lcore role in cpufreq API
>   power/cppc_cpufreq: validate lcore role in cpufreq API
>   power/intel_pstate: validate lcore role in cpufreq API
>   power: add a common macro to verify lcore ID
>   power/cpufreq: add the lcore ID verification to framework
>   power/acpi_cpufreq: remove the verification of lcore ID
>   power/amd_pstate: remove the verification of lcore ID
>   power/cppc_cpufreq: remove the verification of lcore ID
>   power/intel_pstate: remove the verification of lcore ID
>   power/kvm_vm: remove the verification of lcore ID
>   power: allow the service core to config power QoS
>   power: add lcore ID check for PMD mgmt
> 
>  drivers/power/acpi/acpi_cpufreq.c             | 65 -------------------
>  drivers/power/amd_pstate/amd_pstate_cpufreq.c | 65 -------------------
>  drivers/power/cppc/cppc_cpufreq.c             | 65 -------------------
>  .../power/intel_pstate/intel_pstate_cpufreq.c | 65 -------------------
>  drivers/power/kvm_vm/kvm_vm.c                 | 10 ---
>  lib/eal/common/eal_common_lcore.c             | 11 ++++
>  lib/eal/include/rte_lcore.h                   | 11 ++++
>  lib/power/power_common.h                      |  7 ++
>  lib/power/rte_power_cpufreq.c                 | 14 +++-
>  lib/power/rte_power_pmd_mgmt.c                | 21 +++---
>  lib/power/rte_power_qos.c                     | 10 +--
>  11 files changed, 55 insertions(+), 289 deletions(-)
> 

Lots of AI review feedback on this one:

Patch 01/15: eal: add interface to check if lcore is EAL managed
=================================================================

Warning: New public API rte_lcore_is_eal_managed() is exported as
a stable ABI symbol (RTE_EXPORT_SYMBOL), but DPDK policy requires
new public APIs to be introduced as experimental:

    +RTE_EXPORT_SYMBOL(rte_lcore_is_eal_managed)
    +int rte_lcore_is_eal_managed(unsigned int lcore_id)

The declaration in rte_lcore.h is also missing the __rte_experimental
attribute. The export should be:

    RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_lcore_is_eal_managed, 26.07)
    int rte_lcore_is_eal_managed(unsigned int lcore_id)

and the prototype in lib/eal/include/rte_lcore.h should carry
__rte_experimental.

Warning: This series adds a new public API but does not update the
release notes in doc/guides/rel_notes/release_26_07.rst (or whatever
the current target release is). New API additions must be documented
in the release notes under the "New Features" section.


Patch 07/15: power: add a common macro to verify lcore ID
=========================================================

Info: The macro evaluates lcore_id twice (once in
rte_lcore_is_eal_managed() and once in POWER_LOG). All current
call-sites pass a plain variable so there is no observable bug, but
a side-effecting argument would be evaluated twice. Same caveat
applies to the existing RTE_ETH_VALID_PORTID_OR_ERR_RET pattern, so
this is consistent with existing DPDK convention.


Patch 14/15: power: allow the service core to config power QoS
==============================================================

Info: The check changes from rte_lcore_is_enabled() (ROLE_RTE only)
to rte_lcore_is_eal_managed() (ROLE_RTE + ROLE_SERVICE), so the log
message wording changes from "is not enabled" to the macro's "is
invalid" text. For a service core that was previously rejected, the
new "invalid" wording is less informative than the prior message,
but this is minor.


Patch 15/15: power: add lcore ID check for PMD mgmt
====================================================

Info: This patch tightens the validation from "lcore_id within
RTE_MAX_LCORE range" to "lcore_id is ROLE_RTE". Combined with the
Fixes: tag and Cc: stable, this is a behavioural change in stable
branches: callers that previously passed a non-enabled lcore_id
(ROLE_OFF / ROLE_SERVICE / ROLE_NON_EAL) will now receive -EINVAL
where they previously succeeded into the function body. The commit
message correctly justifies the data-plane (ROLE_RTE) requirement,
but the behaviour change should be called out explicitly in the
commit log so stable maintainers and downstream users are aware.


Series-level observation
========================

Info: Patches 02-06 add per-driver lcore role checks, and patches
09-13 remove the same checks once patch 08 lifts validation into
the framework. The net effect is that each driver's lcore check is
modified and then deleted within the same series. The series could
be reorganized to:

  1. Patch 01: add rte_lcore_is_eal_managed()
  2. Patch 07: add the RTE_POWER_VALID_LCOREID_OR_ERR_RET macro
  3. Patch 08: add validation to the framework (rte_power_cpufreq.c)
  4. Patches 14, 15: pmd_mgmt and QoS adjustments

skipping patches 02-06 and 09-13 entirely. This would reduce review
load and avoid intermediate states where the same check exists in
two places. If the current structure is intentional for incremental
bisection, that rationale would be worth mentioning in a cover
letter.