Re: [PATCH v4] eal: fix uncheck lcore ID and lcore role

[huangdengdui <huangdengdui@huawei.com>]

On 2025/7/24 22:44, Morten Brørup wrote:
>> From: Dengdui Huang [mailto:huangdengdui@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: stable@dpdk.org
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@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
> 
>