Re: [dpdk-dev] [PATCH v19 7/7] app/test: add dmadev API test

[Kevin Laatz <kevin.laatz@intel.com>]

On 02/09/2021 14:13, Chengwen Feng wrote:
> This patch add dmadev API test which based on 'dma_skeleton' vdev. The
> test cases could be executed using 'dmadev_autotest' command in test
> framework.
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   MAINTAINERS                |   1 +
>   app/test/meson.build       |   4 +
>   app/test/test_dmadev.c     |  45 ++++
>   app/test/test_dmadev_api.c | 532 +++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 582 insertions(+)
>   create mode 100644 app/test/test_dmadev.c
>   create mode 100644 app/test/test_dmadev_api.c
>
<snip>


> +
> +REGISTER_TEST_COMMAND(dmadev_autotest, test_dmadev);
> diff --git a/app/test/test_dmadev_api.c b/app/test/test_dmadev_api.c
> new file mode 100644
> index 0000000..a7795eb
> --- /dev/null
> +++ b/app/test/test_dmadev_api.c
> @@ -0,0 +1,532 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 HiSilicon Limited.
> + */
> +
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include <rte_common.h>
> +#include <rte_cycles.h>
> +#include <rte_malloc.h>
> +#include <rte_test.h>
> +#include <rte_dmadev.h>
> +
> +extern int test_dmadev_api(uint16_t dev_id);
> +
> +#define SKELDMA_TEST_RUN(test) \
> +	testsuite_run_test(test, #test)
> +
> +#define TEST_MEMCPY_SIZE	1024
> +#define TEST_WAIT_US_VAL	50000
> +
> +#define TEST_SUCCESS 0
> +#define TEST_FAILED  -1
> +
> +static uint16_t test_dev_id;
> +static uint16_t invalid_dev_id;
> +
> +static int total;
> +static int passed;
> +static int failed;
> +static char *src;
> +static char *dst;
> +
> +static int
> +testsuite_setup(uint16_t dev_id)
> +{
> +	test_dev_id = dev_id;
> +	invalid_dev_id = RTE_DMADEV_MAX_DEVS;
> +
> +	src = rte_malloc("dmadev_test_src", TEST_MEMCPY_SIZE, 0);
> +	if (src == NULL)
> +		return -ENOMEM;
> +	dst = rte_malloc("dmadev_test_dst", TEST_MEMCPY_SIZE, 0);
> +	if (dst == NULL)
> +		return -ENOMEM;
> +
> +	total = 0;
> +	passed = 0;
> +	failed = 0;
> +
> +	return 0;
> +}
> +
> +static void
> +testsuite_teardown(void)
> +{
> +	rte_free(src);
> +	rte_free(dst);

These should be set to NULL after free.


> +	/* Ensure the dmadev is stopped. */
> +	rte_dmadev_stop(test_dev_id);
> +}
> +

<snip>


> +
> +int
> +test_dmadev_api(uint16_t dev_id)
> +{
> +	int ret = testsuite_setup(dev_id);
> +	if (ret) {

If testsuite setup fails, src/dst potentially need to be free'd, so 
"testsuite_teardown()" should be called here.


> +		printf("testsuite setup fail!\n");
> +		return -1;
> +	}
> +
> +	/* If the testcase exit successfully, ensure that the test dmadev exist
> +	 * and the dmadev is in the stopped state.
> +	 */
> +	SKELDMA_TEST_RUN(test_dmadev_get_dev_id);
> +	SKELDMA_TEST_RUN(test_dmadev_is_valid_dev);
> +	SKELDMA_TEST_RUN(test_dmadev_count);
> +	SKELDMA_TEST_RUN(test_dmadev_info_get);
> +	SKELDMA_TEST_RUN(test_dmadev_configure);
> +	SKELDMA_TEST_RUN(test_dmadev_vchan_setup);
> +	SKELDMA_TEST_RUN(test_dmadev_start_stop);
> +	SKELDMA_TEST_RUN(test_dmadev_stats);
> +	SKELDMA_TEST_RUN(test_dmadev_dump);
> +	SKELDMA_TEST_RUN(test_dmadev_completed);
> +	SKELDMA_TEST_RUN(test_dmadev_completed_status);
> +
> +	testsuite_teardown();
> +
> +	printf("Total tests   : %d\n", total);
> +	printf("Passed        : %d\n", passed);
> +	printf("Failed        : %d\n", failed);
> +
> +	if (failed)
> +		return -1;
> +
> +	return 0;
> +};

With the above comments addressed,

Reviewed-by: Kevin Laatz <kevin.laatz@intel.com>