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

  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(-)
  

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 */
>
  

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