[v10,3/4] app/dma-perf: validate copied memory

Message ID 20240227183555.3932711-4-amitprakashs@marvell.com (mailing list archive)
State Superseded, archived
Headers
Series PCI Dev and SG copy support |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Amit Prakash Shukla Feb. 27, 2024, 6:35 p.m. UTC
  From: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>

Validate copied memory to ensure DMA copy did not fail.

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test-dma-perf/benchmark.c | 21 ++++++++++++++++++++-
 app/test-dma-perf/main.c      | 16 +++++++++++-----
 app/test-dma-perf/main.h      |  2 +-
 3 files changed, 32 insertions(+), 7 deletions(-)
  

Comments

Chengwen Feng Feb. 28, 2024, 8:10 a.m. UTC | #1
Hi Gowrishankar,

On 2024/2/28 2:35, Amit Prakash Shukla wrote:
> From: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> 
> Validate copied memory to ensure DMA copy did not fail.
> 
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  app/test-dma-perf/benchmark.c | 21 ++++++++++++++++++++-
>  app/test-dma-perf/main.c      | 16 +++++++++++-----
>  app/test-dma-perf/main.h      |  2 +-
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
> index 4370d71134..0047e2f4b8 100644
> --- a/app/test-dma-perf/benchmark.c
> +++ b/app/test-dma-perf/benchmark.c
> @@ -12,6 +12,7 @@
>  #include <rte_dmadev.h>
>  #include <rte_malloc.h>
>  #include <rte_lcore.h>
> +#include <rte_random.h>
>  
>  #include "main.h"
>  
> @@ -407,6 +408,11 @@ setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs,
>  		return -1;
>  	}
>  
> +	for (i = 0; i < nr_buf; i++) {
> +		memset(rte_pktmbuf_mtod((*srcs)[i], void *), rte_rand(), buf_size);
> +		memset(rte_pktmbuf_mtod((*dsts)[i], void *), 0, buf_size);
> +	}
> +
>  	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM ||
>  	    cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) {
>  		ext_buf_info = rte_malloc(NULL, sizeof(struct rte_mbuf_ext_shared_info), 0);
> @@ -443,7 +449,7 @@ setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs,
>  	return 0;
>  }
>  
> -void
> +int
>  mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
>  {
>  	uint32_t i;
> @@ -461,6 +467,7 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
>  	uint32_t avg_cycles_total;
>  	float mops, mops_total;
>  	float bandwidth, bandwidth_total;
> +	int ret = 0;
>  
>  	if (setup_memory_env(cfg, &srcs, &dsts) < 0)
>  		goto out;
> @@ -534,6 +541,16 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
>  
>  	rte_eal_mp_wait_lcore();
>  
> +	for (i = 0; i < (nr_buf / nb_workers) * nb_workers; i++) {
> +		if (memcmp(rte_pktmbuf_mtod(srcs[i], void *),
> +			   rte_pktmbuf_mtod(dsts[i], void *),
> +			   cfg->buf_size.cur) != 0) {
> +			printf("Copy validation fails for buffer number %d\n", i);

For non-mem2mem DMA, like mem2dev or dev2mem, the device host address may not direct accessable by CPU
(if could may need mmap).

So pls restrict it only mem2mem, or drop this commit.

Thanks

> +			ret = -1;
> +			goto out;
> +		}
> +	}
> +
>  	mops_total = 0;
>  	bandwidth_total = 0;
>  	avg_cycles_total = 0;
> @@ -599,4 +616,6 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
>  			rte_dma_stop(ldm->dma_ids[i]);
>  		}
>  	}
> +
> +	return ret;
>  }
> diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
> index 051f76a6f9..df05bcd7df 100644
> --- a/app/test-dma-perf/main.c
> +++ b/app/test-dma-perf/main.c
> @@ -101,20 +101,24 @@ open_output_csv(const char *rst_path_ptr)
>  	return 0;
>  }
>  
> -static void
> +static int
>  run_test_case(struct test_configure *case_cfg)
>  {
> +	int ret = 0;
> +
>  	switch (case_cfg->test_type) {
>  	case TEST_TYPE_DMA_MEM_COPY:
> -		mem_copy_benchmark(case_cfg, true);
> +		ret = mem_copy_benchmark(case_cfg, true);
>  		break;
>  	case TEST_TYPE_CPU_MEM_COPY:
> -		mem_copy_benchmark(case_cfg, false);
> +		ret = mem_copy_benchmark(case_cfg, false);
>  		break;
>  	default:
>  		printf("Unknown test type. %s\n", case_cfg->test_type_str);
>  		break;
>  	}
> +
> +	return ret;
>  }
>  
>  static void
> @@ -159,8 +163,10 @@ run_test(uint32_t case_id, struct test_configure *case_cfg)
>  		case_cfg->scenario_id++;
>  		printf("\nRunning scenario %d\n", case_cfg->scenario_id);
>  
> -		run_test_case(case_cfg);
> -		output_csv(false);
> +		if (run_test_case(case_cfg) < 0)
> +			printf("\nTest fails! skipping this scenario.\n");
> +		else
> +			output_csv(false);
>  
>  		if (var_entry->op == OP_ADD)
>  			var_entry->cur += var_entry->incr;
> diff --git a/app/test-dma-perf/main.h b/app/test-dma-perf/main.h
> index 745c24b7fe..1123e7524a 100644
> --- a/app/test-dma-perf/main.h
> +++ b/app/test-dma-perf/main.h
> @@ -66,6 +66,6 @@ struct test_configure {
>  	struct test_vchan_dev_config vchan_dev;
>  };
>  
> -void mem_copy_benchmark(struct test_configure *cfg, bool is_dma);
> +int mem_copy_benchmark(struct test_configure *cfg, bool is_dma);
>  
>  #endif /* MAIN_H */
>
  
Gowrishankar Muthukrishnan Feb. 28, 2024, 9:09 a.m. UTC | #2
Hi Fengchengwen,

> > +	for (i = 0; i < (nr_buf / nb_workers) * nb_workers; i++) {
> > +		if (memcmp(rte_pktmbuf_mtod(srcs[i], void *),
> > +			   rte_pktmbuf_mtod(dsts[i], void *),
> > +			   cfg->buf_size.cur) != 0) {
> > +			printf("Copy validation fails for buffer number %d\n",
> i);
> 
> For non-mem2mem DMA, like mem2dev or dev2mem, the device host
> address may not direct accessable by CPU (if could may need mmap).
> 

This has been checked in 4/4 patch as (cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_MEM). Would you still need it in this patch only ?.

Thanks,
Gowrishankar

> So pls restrict it only mem2mem, or drop this commit.
> 
> Thanks
  

Patch

diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
index 4370d71134..0047e2f4b8 100644
--- a/app/test-dma-perf/benchmark.c
+++ b/app/test-dma-perf/benchmark.c
@@ -12,6 +12,7 @@ 
 #include <rte_dmadev.h>
 #include <rte_malloc.h>
 #include <rte_lcore.h>
+#include <rte_random.h>
 
 #include "main.h"
 
@@ -407,6 +408,11 @@  setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs,
 		return -1;
 	}
 
+	for (i = 0; i < nr_buf; i++) {
+		memset(rte_pktmbuf_mtod((*srcs)[i], void *), rte_rand(), buf_size);
+		memset(rte_pktmbuf_mtod((*dsts)[i], void *), 0, buf_size);
+	}
+
 	if (cfg->transfer_dir == RTE_DMA_DIR_DEV_TO_MEM ||
 	    cfg->transfer_dir == RTE_DMA_DIR_MEM_TO_DEV) {
 		ext_buf_info = rte_malloc(NULL, sizeof(struct rte_mbuf_ext_shared_info), 0);
@@ -443,7 +449,7 @@  setup_memory_env(struct test_configure *cfg, struct rte_mbuf ***srcs,
 	return 0;
 }
 
-void
+int
 mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
 {
 	uint32_t i;
@@ -461,6 +467,7 @@  mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
 	uint32_t avg_cycles_total;
 	float mops, mops_total;
 	float bandwidth, bandwidth_total;
+	int ret = 0;
 
 	if (setup_memory_env(cfg, &srcs, &dsts) < 0)
 		goto out;
@@ -534,6 +541,16 @@  mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
 
 	rte_eal_mp_wait_lcore();
 
+	for (i = 0; i < (nr_buf / nb_workers) * nb_workers; i++) {
+		if (memcmp(rte_pktmbuf_mtod(srcs[i], void *),
+			   rte_pktmbuf_mtod(dsts[i], void *),
+			   cfg->buf_size.cur) != 0) {
+			printf("Copy validation fails for buffer number %d\n", i);
+			ret = -1;
+			goto out;
+		}
+	}
+
 	mops_total = 0;
 	bandwidth_total = 0;
 	avg_cycles_total = 0;
@@ -599,4 +616,6 @@  mem_copy_benchmark(struct test_configure *cfg, bool is_dma)
 			rte_dma_stop(ldm->dma_ids[i]);
 		}
 	}
+
+	return ret;
 }
diff --git a/app/test-dma-perf/main.c b/app/test-dma-perf/main.c
index 051f76a6f9..df05bcd7df 100644
--- a/app/test-dma-perf/main.c
+++ b/app/test-dma-perf/main.c
@@ -101,20 +101,24 @@  open_output_csv(const char *rst_path_ptr)
 	return 0;
 }
 
-static void
+static int
 run_test_case(struct test_configure *case_cfg)
 {
+	int ret = 0;
+
 	switch (case_cfg->test_type) {
 	case TEST_TYPE_DMA_MEM_COPY:
-		mem_copy_benchmark(case_cfg, true);
+		ret = mem_copy_benchmark(case_cfg, true);
 		break;
 	case TEST_TYPE_CPU_MEM_COPY:
-		mem_copy_benchmark(case_cfg, false);
+		ret = mem_copy_benchmark(case_cfg, false);
 		break;
 	default:
 		printf("Unknown test type. %s\n", case_cfg->test_type_str);
 		break;
 	}
+
+	return ret;
 }
 
 static void
@@ -159,8 +163,10 @@  run_test(uint32_t case_id, struct test_configure *case_cfg)
 		case_cfg->scenario_id++;
 		printf("\nRunning scenario %d\n", case_cfg->scenario_id);
 
-		run_test_case(case_cfg);
-		output_csv(false);
+		if (run_test_case(case_cfg) < 0)
+			printf("\nTest fails! skipping this scenario.\n");
+		else
+			output_csv(false);
 
 		if (var_entry->op == OP_ADD)
 			var_entry->cur += var_entry->incr;
diff --git a/app/test-dma-perf/main.h b/app/test-dma-perf/main.h
index 745c24b7fe..1123e7524a 100644
--- a/app/test-dma-perf/main.h
+++ b/app/test-dma-perf/main.h
@@ -66,6 +66,6 @@  struct test_configure {
 	struct test_vchan_dev_config vchan_dev;
 };
 
-void mem_copy_benchmark(struct test_configure *cfg, bool is_dma);
+int mem_copy_benchmark(struct test_configure *cfg, bool is_dma);
 
 #endif /* MAIN_H */