[PATCH 0/2] fix the problem of dma-perf infinite loop

[PATCH 0/2] fix the problem of dma-perf infinite loop

[Dengdui Huang]

After CPU isolation is configured, an infinite loop occurs when
dma-perf is executed using the default config file.

This patchset fix it.

Dengdui Huang (2):
  eal: fix wake a incorrect lcore
  app/dma-perf: fix infinite loop

 app/test-dma-perf/benchmark.c  | 5 ++++-
 lib/eal/unix/eal_unix_thread.c | 3 +++
 lib/eal/windows/eal_thread.c   | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH 1/2] eal: fix wake a incorrect lcore

[Dengdui Huang]

If the core is not used in the rte, we can't wake it up to work.
The worker_id may come from user input. So it is necessary to
verify it.

Fixes: a95d70547c57 ("eal: factorize lcore main loop")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 lib/eal/unix/eal_unix_thread.c | 3 +++
 lib/eal/windows/eal_thread.c   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/lib/eal/unix/eal_unix_thread.c b/lib/eal/unix/eal_unix_thread.c
index ef6cbff0ee..103571cabf 100644
--- a/lib/eal/unix/eal_unix_thread.c
+++ b/lib/eal/unix/eal_unix_thread.c
@@ -17,6 +17,9 @@ eal_thread_wake_worker(unsigned int worker_id)
 	char c = 0;
 	int n;
 
+	if (m2w == 0 || w2m == 0)
+		return -EINVAL;
+
 	do {
 		n = write(m2w, &c, 1);
 	} while (n == 0 || (n < 0 && errno == EINTR));
diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c
index 9e3df200b9..82bb974868 100644
--- a/lib/eal/windows/eal_thread.c
+++ b/lib/eal/windows/eal_thread.c
@@ -24,6 +24,9 @@ eal_thread_wake_worker(unsigned int worker_id)
 	char c = 0;
 	int n;
 
+	if (m2w == 0 || w2m == 0)
+		return -EINVAL;
+
 	do {
 		n = _write(m2w, &c, 1);
 	} while (n == 0 || (n < 0 && errno == EINTR));
-- 
2.33.0

[PATCH 2/2] app/dma-perf: fix infinite loop

[Dengdui Huang]

When a core that is not used by the rte is specified in the config
for testing, the problem of infinite loop occurs. The root cause
is that the program waits for the completion of the test task when
the test worker fails to be started on the lcore. This patch fix it.

Fixes: 533d7e7f66f3 ("app/dma-perf: support config per device")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-dma-perf/benchmark.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
index 6d617ea200..351c1c966e 100644
--- a/app/test-dma-perf/benchmark.c
+++ b/app/test-dma-perf/benchmark.c
@@ -751,7 +751,10 @@ mem_copy_benchmark(struct test_configure *cfg)
 				goto out;
 		}
 
-		rte_eal_remote_launch(get_work_function(cfg), (void *)(lcores[i]), lcore_id);
+		if (rte_eal_remote_launch(get_work_function(cfg), (void *)(lcores[i]), lcore_id)) {
+			printf("Error: Fail to start the test on lcore %d\n", lcore_id);
+			goto out;
+		}
 	}
 
 	while (1) {
-- 
2.33.0

Re: [PATCH 2/2] app/dma-perf: fix infinite loop

[Stephen Hemminger]

On Fri, 21 Mar 2025 12:03:16 +0800
Dengdui Huang <huangdengdui@huawei.com> wrote:

> When a core that is not used by the rte is specified in the config
> for testing, the problem of infinite loop occurs. The root cause
> is that the program waits for the completion of the test task when
> the test worker fails to be started on the lcore. This patch fix it.
> 
> Fixes: 533d7e7f66f3 ("app/dma-perf: support config per device")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---
>  app/test-dma-perf/benchmark.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
> index 6d617ea200..351c1c966e 100644
> --- a/app/test-dma-perf/benchmark.c
> +++ b/app/test-dma-perf/benchmark.c
> @@ -751,7 +751,10 @@ mem_copy_benchmark(struct test_configure *cfg)
>  				goto out;
>  		}
>  
> -		rte_eal_remote_launch(get_work_function(cfg), (void *)(lcores[i]), lcore_id);
> +		if (rte_eal_remote_launch(get_work_function(cfg), (void *)(lcores[i]), lcore_id)) {
> +			printf("Error: Fail to start the test on lcore %d\n", lcore_id);

Convention is to log errors on stderr and lcore_id is unsigned not signed value.

Re: [PATCH 2/2] app/dma-perf: fix infinite loop

[huangdengdui]

On 2025/3/21 23:58, Stephen Hemminger wrote:
> On Fri, 21 Mar 2025 12:03:16 +0800
> Dengdui Huang <huangdengdui@huawei.com> wrote:
> 
>> When a core that is not used by the rte is specified in the config
>> for testing, the problem of infinite loop occurs. The root cause
>> is that the program waits for the completion of the test task when
>> the test worker fails to be started on the lcore. This patch fix it.
>>
>> Fixes: 533d7e7f66f3 ("app/dma-perf: support config per device")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>>  app/test-dma-perf/benchmark.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
>> index 6d617ea200..351c1c966e 100644
>> --- a/app/test-dma-perf/benchmark.c
>> +++ b/app/test-dma-perf/benchmark.c
>> @@ -751,7 +751,10 @@ mem_copy_benchmark(struct test_configure *cfg)
>>  				goto out;
>>  		}
>>  
>> -		rte_eal_remote_launch(get_work_function(cfg), (void *)(lcores[i]), lcore_id);
>> +		if (rte_eal_remote_launch(get_work_function(cfg), (void *)(lcores[i]), lcore_id)) {
>> +			printf("Error: Fail to start the test on lcore %d\n", lcore_id);
> 
> Convention is to log errors on stderr and lcore_id is unsigned not signed value.

OK, I'll fix it for the next version.

Re: [PATCH 0/2] fix the problem of dma-perf infinite loop

[David Marchand]

On Fri, Mar 21, 2025 at 5:03 AM Dengdui Huang <huangdengdui@huawei.com> wrote:
>
> After CPU isolation is configured, an infinite loop occurs when
> dma-perf is executed using the default config file.
>
> This patchset fix it.
>
> Dengdui Huang (2):
>   eal: fix wake a incorrect lcore
>   app/dma-perf: fix infinite loop
>
>  app/test-dma-perf/benchmark.c  | 5 ++++-
>  lib/eal/unix/eal_unix_thread.c | 3 +++
>  lib/eal/windows/eal_thread.c   | 3 +++
>  3 files changed, 10 insertions(+), 1 deletion(-)

We can make EAL launch API more resilient, but the dma-perf
application should not post jobs on non sense lcore id, in the first
place.
parse_lcore() should be validating that the requested lcore_id is valid.


-- 
David Marchand

Re: [PATCH 0/2] fix the problem of dma-perf infinite loop

[huangdengdui]

On 2025/3/21 15:49, David Marchand wrote:
> On Fri, Mar 21, 2025 at 5:03 AM Dengdui Huang <huangdengdui@huawei.com> wrote:
>>
>> After CPU isolation is configured, an infinite loop occurs when
>> dma-perf is executed using the default config file.
>>
>> This patchset fix it.
>>
>> Dengdui Huang (2):
>>   eal: fix wake a incorrect lcore
>>   app/dma-perf: fix infinite loop
>>
>>  app/test-dma-perf/benchmark.c  | 5 ++++-
>>  lib/eal/unix/eal_unix_thread.c | 3 +++
>>  lib/eal/windows/eal_thread.c   | 3 +++
>>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> We can make EAL launch API more resilient, but the dma-perf
> application should not post jobs on non sense lcore id, in the first
> place.
> parse_lcore() should be validating that the requested lcore_id is valid.
> 
> 

OK, I'll add a check to make sure dma-perf uses the correct lcore.

[PATCH] app/dma-perf: fix infinite loop

[Dengdui Huang]

When a core that is not used by the rte is specified in the config
for testing, the problem of infinite loop occurs. This patch fix it
by adding a check for lcore_id.

Fixes: 623dc9364dc6 ("app/dma-perf: introduce DMA performance test")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-dma-perf/main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
index 0586b3e1d0..1365456dd1 100644
--- a/app/test-dma-perf/main.c
+++ b/app/test-dma-perf/main.c
@@ -523,7 +523,7 @@ main(int argc, char *argv[])
 {
 	int ret;
 	uint16_t case_nb;
-	uint32_t i, nb_lcores;
+	uint32_t i, j, nb_lcores, lcore;
 	pid_t cpid, wpid;
 	int wstatus;
 	char args[MAX_EAL_PARAM_NB][MAX_EAL_PARAM_LEN];
@@ -603,6 +603,12 @@ main(int argc, char *argv[])
 			if (nb_lcores < 2)
 				rte_exit(EXIT_FAILURE,
 					"There should be at least 2 worker lcores.\n");
+			for (j = 0; j < test_cases[i].num_worker; j++) {
+				lcore = test_cases[i].dma_config[j].lcore_dma_map.lcore;
+				if (!rte_lcore_has_role(lcore, ROLE_RTE))
+					rte_exit(EXIT_FAILURE,
+						 "Worker lcore %u not enabled in EAL\n", lcore);
+			}
 
 			fd = fopen(rst_path_ptr, "a");
 			if (!fd) {
-- 
2.33.0

[PATCH 1/2] eal: fix uncheck worker ID

[Dengdui Huang]

The worker_id may come from user input.
So it is necessary to verify it.

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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/eal/common/eal_common_launch.c b/lib/eal/common/eal_common_launch.c
index 5320c3bd3c..76313d5cdf 100644
--- a/lib/eal/common/eal_common_launch.c
+++ b/lib/eal/common/eal_common_launch.c
@@ -35,6 +35,9 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id)
 {
 	int rc = -EBUSY;
 
+	if (!rte_lcore_has_role(worker_id, ROLE_RTE))
+		return -EINVAL;
+
 	/* Check if the worker is in 'WAIT' state. Use acquire order
 	 * since 'state' variable is used as the guard variable.
 	 */
-- 
2.33.0

[PATCH 2/2] app/dma-perf: fix infinite loop

[Dengdui Huang]

When a core that is not used by the rte is specified in the config
for testing, the problem of infinite loop occurs. This patch fix it
by adding a check for lcore_id.

Fixes: 623dc9364dc6 ("app/dma-perf: introduce DMA performance test")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-dma-perf/main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
index 0586b3e1d0..1365456dd1 100644
--- a/app/test-dma-perf/main.c
+++ b/app/test-dma-perf/main.c
@@ -523,7 +523,7 @@ main(int argc, char *argv[])
 {
 	int ret;
 	uint16_t case_nb;
-	uint32_t i, nb_lcores;
+	uint32_t i, j, nb_lcores, lcore;
 	pid_t cpid, wpid;
 	int wstatus;
 	char args[MAX_EAL_PARAM_NB][MAX_EAL_PARAM_LEN];
@@ -603,6 +603,12 @@ main(int argc, char *argv[])
 			if (nb_lcores < 2)
 				rte_exit(EXIT_FAILURE,
 					"There should be at least 2 worker lcores.\n");
+			for (j = 0; j < test_cases[i].num_worker; j++) {
+				lcore = test_cases[i].dma_config[j].lcore_dma_map.lcore;
+				if (!rte_lcore_has_role(lcore, ROLE_RTE))
+					rte_exit(EXIT_FAILURE,
+						 "Worker lcore %u not enabled in EAL\n", lcore);
+			}
 
 			fd = fopen(rst_path_ptr, "a");
 			if (!fd) {
-- 
2.33.0

[PATCH v2 0/2] fix the problem of dma-perf infinite loop

[Dengdui Huang]

After CPU isolation is configured, an infinite loop occurs when
dma-perf is executed using the default config file.

This patchset fix it.

Dengdui Huang (2):
  eal: fix uncheck worker ID
  app/dma-perf: fix infinite loop

 app/test-dma-perf/main.c           | 8 +++++++-
 lib/eal/common/eal_common_launch.c | 3 +++
 2 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v2 1/2] eal: fix uncheck worker ID

[Dengdui Huang]

The worker_id may come from user input.
So it is necessary to verify it.

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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/eal/common/eal_common_launch.c b/lib/eal/common/eal_common_launch.c
index 5320c3bd3c..76313d5cdf 100644
--- a/lib/eal/common/eal_common_launch.c
+++ b/lib/eal/common/eal_common_launch.c
@@ -35,6 +35,9 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id)
 {
 	int rc = -EBUSY;
 
+	if (!rte_lcore_has_role(worker_id, ROLE_RTE))
+		return -EINVAL;
+
 	/* Check if the worker is in 'WAIT' state. Use acquire order
 	 * since 'state' variable is used as the guard variable.
 	 */
-- 
2.33.0

RE: [PATCH v2 1/2] eal: fix uncheck worker ID

[Morten Brørup]

> From: Dengdui Huang [mailto:huangdengdui@huawei.com]
> Sent: Thursday, 27 March 2025 10.01
> 
> The worker_id may come from user input.
> So it is necessary to verify it.
> 
> 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 | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/eal/common/eal_common_launch.c
> b/lib/eal/common/eal_common_launch.c
> index 5320c3bd3c..76313d5cdf 100644
> --- a/lib/eal/common/eal_common_launch.c
> +++ b/lib/eal/common/eal_common_launch.c
> @@ -35,6 +35,9 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg,
> unsigned int worker_id)
>  {
>  	int rc = -EBUSY;
> 

Also check the lcore_id:
+	if (unlikely(lcore_id >= RTE_MAX_LCORE))
+		return -EINVAL;
+

> +	if (!rte_lcore_has_role(worker_id, ROLE_RTE))
> +		return -EINVAL;
> +

Although rte_lcore_has_role() checks the lcore_id and returns -EINVAL if invalid, "if (!-EINVAL))" will not take the branch to return -EINVAL here. So this check alone does not suffice. You could change it to "if (rte_lcore_has_role(worker_id, ROLE_RTE) <= 0) return -EINVAL;".
However, I prefer the separate lcore_id check, as mentioned above, rather than changing this check.

>  	/* Check if the worker is in 'WAIT' state. Use acquire order
>  	 * since 'state' variable is used as the guard variable.
>  	 */
> --
> 2.33.0

<feature creep>
While you are at it, please also add "if (unlikely(lcore_id >= RTE_MAX_LCORE)) return -EINVAL;" checks in the other functions in eal_common_lcore.c where lcore_id comes from user input and the check is missing.
</feature creep>

[PATCH v2 2/2] app/dma-perf: fix infinite loop

[Dengdui Huang]

When a core that is not used by the rte is specified in the config
for testing, the problem of infinite loop occurs. This patch fix it
by adding a check for lcore_id.

Fixes: 623dc9364dc6 ("app/dma-perf: introduce DMA performance test")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 app/test-dma-perf/main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
index 0586b3e1d0..1365456dd1 100644
--- a/app/test-dma-perf/main.c
+++ b/app/test-dma-perf/main.c
@@ -523,7 +523,7 @@ main(int argc, char *argv[])
 {
 	int ret;
 	uint16_t case_nb;
-	uint32_t i, nb_lcores;
+	uint32_t i, j, nb_lcores, lcore;
 	pid_t cpid, wpid;
 	int wstatus;
 	char args[MAX_EAL_PARAM_NB][MAX_EAL_PARAM_LEN];
@@ -603,6 +603,12 @@ main(int argc, char *argv[])
 			if (nb_lcores < 2)
 				rte_exit(EXIT_FAILURE,
 					"There should be at least 2 worker lcores.\n");
+			for (j = 0; j < test_cases[i].num_worker; j++) {
+				lcore = test_cases[i].dma_config[j].lcore_dma_map.lcore;
+				if (!rte_lcore_has_role(lcore, ROLE_RTE))
+					rte_exit(EXIT_FAILURE,
+						 "Worker lcore %u not enabled in EAL\n", lcore);
+			}
 
 			fd = fopen(rst_path_ptr, "a");
 			if (!fd) {
-- 
2.33.0