On 2025/7/24 22:44, Morten Brørup wrote:
>> From: Dengdui Huang [mailto:huangdeng...@huawei.com]
>> Sent: Thursday, 24 July 2025 13.26
>>
>> The worker_id may come from user input. So it is necessary to verify it.
>>
>> In addition, only the lcore whose role is ROLE_RTE or ROLE_SERVICE can
>> be
>> launched. This patch adds the lcore role check.
>>
>> Fixes: a95d70547c57 ("eal: factorize lcore main loop")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Dengdui Huang <huangdeng...@huawei.com>
>> ---
>> lib/eal/common/eal_common_launch.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/lib/eal/common/eal_common_launch.c
>> b/lib/eal/common/eal_common_launch.c
>> index a7deac6ecd..09366ea404 100644
>> --- a/lib/eal/common/eal_common_launch.c
>> +++ b/lib/eal/common/eal_common_launch.c
>> @@ -20,6 +20,9 @@ RTE_EXPORT_SYMBOL(rte_eal_wait_lcore)
>> int
>> rte_eal_wait_lcore(unsigned worker_id)
>> {
>> + if (unlikely(worker_id >= RTE_MAX_LCORE))
>> + return -EINVAL;
>> +
>> while (rte_atomic_load_explicit(&lcore_config[worker_id].state,
>> rte_memory_order_acquire) != WAIT)
>> rte_pause();
>> @@ -37,6 +40,18 @@ int
>> rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int
>> worker_id)
>> {
>> int rc = -EBUSY;
>> + uint8_t role;
>
> Use the correct type:
> enum rte_lcore_role_t role;
OK, I made a mistake, and I will fix it in the next version.
>
>> +
>> + if (unlikely(worker_id >= RTE_MAX_LCORE)) {
>> + rc = -EINVAL;
>> + goto finish;
>> + }
>> +
>> + role = lcore_config[worker_id].core_role;
>> + if (role != ROLE_RTE && role != ROLE_SERVICE) {
>> + rc = -EINVAL;
>> + goto finish;
>> + }
>>
>> /* Check if the worker is in 'WAIT' state. Use acquire order
>> * since 'state' variable is used as the guard variable.
>> @@ -98,6 +113,9 @@ RTE_EXPORT_SYMBOL(rte_eal_get_lcore_state)
>> enum rte_lcore_state_t
>> rte_eal_get_lcore_state(unsigned lcore_id)
>> {
>> + if (unlikely(lcore_id >= RTE_MAX_LCORE))
>> + return -EINVAL;
>
> The return type of this function is enum rte_lcore_state_t, so it cannot
> return (int)-EINVAL.
>
> Maybe it's better to update the function's documentation instead of its
> implementation.
> Like the documentation of the lcore_id parameter passed to
> rte_eal_lcore_role():
> https://elixir.bootlin.com/dpdk/v25.07/source/lib/eal/include/rte_lcore.h#L44
>
> After further consideration, updating the documentation might also be a
> better solution for the other functions.
This is a better solution, and I will do it in the next version.
>
> If you add a new return value (-EINVAL) to a function, it must be listed in
> function's documentation, and in the documentation of functions calling that
> function.
>
>> +
>> return lcore_config[lcore_id].state;
>> }
>>
>> --
>> 2.33.0
>
>
