diff mbox series

[v2,2/6] app/test: add basic dmadev instance tests

Message ID 20210901163216.120087-3-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add test suite for DMA drivers |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Bruce Richardson Sept. 1, 2021, 4:32 p.m. UTC 
  Run basic sanity tests for configuring, starting and stopping a dmadev
instance to help validate drivers. This also provides the framework for
future tests for data-path operation.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_dmadev.c | 81 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

Comments

Mattias Rönnblom Sept. 1, 2021, 7:24 p.m. UTC | #1 
On 2021-09-01 18:32, Bruce Richardson wrote:
> Run basic sanity tests for configuring, starting and stopping a dmadev
> instance to help validate drivers. This also provides the framework for
> future tests for data-path operation.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   app/test/test_dmadev.c | 81 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 81 insertions(+)
> 
> diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> index bb01e86483..12f7c69629 100644
> --- a/app/test/test_dmadev.c
> +++ b/app/test/test_dmadev.c
> @@ -2,6 +2,7 @@
>    * Copyright(c) 2021 HiSilicon Limited.
>    * Copyright(c) 2021 Intel Corporation.
>    */
> +#include <inttypes.h>
>   
>   #include <rte_common.h>
>   #include <rte_dev.h>
> @@ -13,6 +14,77 @@
>   /* from test_dmadev_api.c */
>   extern int test_dmadev_api(uint16_t dev_id);
>   
> +#define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
> +
> +static inline int

Remove inline.

> +__rte_format_printf(3, 4)
> +print_err(const char *func, int lineno, const char *format, ...)
> +{
> +	va_list ap;
> +	int ret;
> +
> +	ret = fprintf(stderr, "In %s:%d - ", func, lineno);
Check return code here, and return on error.

> +	va_start(ap, format);
> +	ret += vfprintf(stderr, format, ap);

..and here.

> +	va_end(ap);
> +
> +	return ret;

A negative return value in one call and an valid byte count result for 
the other should produce an error, but here it might not.

You might argue this is just test code, but then I suggest not checking 
the return values at all.

> +}
> +
> +static int
> +test_dmadev_instance(uint16_t dev_id)
> +{
> +#define TEST_RINGSIZE 512
> +	struct rte_dmadev_stats stats;
> +	struct rte_dmadev_info info;
> +	const struct rte_dmadev_conf conf = { .nb_vchans = 1};
> +	const struct rte_dmadev_vchan_conf qconf = {
> +			.direction = RTE_DMA_DIR_MEM_TO_MEM,
> +			.nb_desc = TEST_RINGSIZE,
> +	};
> +	const int vchan = 0;
> +
> +	printf("\n### Test dmadev instance %u\n", dev_id);
> +
> +	rte_dmadev_info_get(dev_id, &info);
> +	if (info.max_vchans < 1) {
> +		PRINT_ERR("Error, no channels available on device id %u\n", dev_id);
> +		return -1;
> +	}
> +	if (rte_dmadev_configure(dev_id, &conf) != 0) {
> +		PRINT_ERR("Error with rte_dmadev_configure()\n");
> +		return -1;
> +	}
> +	if (rte_dmadev_vchan_setup(dev_id, vchan, &qconf) < 0) {
> +		PRINT_ERR("Error with queue configuration\n");
> +		return -1;
> +	}
> +
> +	rte_dmadev_info_get(dev_id, &info);
> +	if (info.nb_vchans != 1) {
> +		PRINT_ERR("Error, no configured queues reported on device id %u\n", dev_id);
> +		return -1;
> +	}
> +
> +	if (rte_dmadev_start(dev_id) != 0) {
> +		PRINT_ERR("Error with rte_dmadev_start()\n");
> +		return -1;
> +	}
> +	if (rte_dmadev_stats_get(dev_id, vchan, &stats) != 0) {
> +		PRINT_ERR("Error with rte_dmadev_stats_get()\n");
> +		return -1;
> +	}
> +	if (stats.completed != 0 || stats.submitted != 0 || stats.errors != 0) {
> +		PRINT_ERR("Error device stats are not all zero: completed = %"PRIu64", submitted = %"PRIu64", errors = %"PRIu64"\n",
> +				stats.completed, stats.submitted, stats.errors);
> +		return -1;
> +	}
> +
> +	rte_dmadev_stop(dev_id);
> +	rte_dmadev_stats_reset(dev_id, vchan);
> +	return 0;
> +}
> +
>   static int
>   test_apis(void)
>   {
> @@ -35,10 +107,19 @@ test_apis(void)
>   static int
>   test_dmadev(void)
>   {
> +	int i;
> +
>   	/* basic sanity on dmadev infrastructure */
>   	if (test_apis() < 0)
>   		return -1;
>   
> +	if (rte_dmadev_count() == 0)
> +		return TEST_SKIPPED;
> +
> +	for (i = 0; i < RTE_DMADEV_MAX_DEVS; i++)
> +		if (rte_dmadevices[i].state == RTE_DMADEV_ATTACHED && test_dmadev_instance(i) < 0)
> +			return -1;
> +
>   	return 0;
>   }
>   
>
Bruce Richardson Sept. 2, 2021, 10:30 a.m. UTC | #2 
On Wed, Sep 01, 2021 at 09:24:12PM +0200, Mattias Rönnblom wrote:
> On 2021-09-01 18:32, Bruce Richardson wrote:
> > Run basic sanity tests for configuring, starting and stopping a dmadev
> > instance to help validate drivers. This also provides the framework for
> > future tests for data-path operation.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >   app/test/test_dmadev.c | 81 ++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 81 insertions(+)
> > 
> > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> > index bb01e86483..12f7c69629 100644
> > --- a/app/test/test_dmadev.c
> > +++ b/app/test/test_dmadev.c
> > @@ -2,6 +2,7 @@
> >    * Copyright(c) 2021 HiSilicon Limited.
> >    * Copyright(c) 2021 Intel Corporation.
> >    */
> > +#include <inttypes.h>
> >   #include <rte_common.h>
> >   #include <rte_dev.h>
> > @@ -13,6 +14,77 @@
> >   /* from test_dmadev_api.c */
> >   extern int test_dmadev_api(uint16_t dev_id);
> > +#define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
> > +
> > +static inline int
> 
> Remove inline.
>
While I understand it's probably not doing a lot having "inline" there, any
particular reason why you think it should be removed?
 
> > +__rte_format_printf(3, 4)
> > +print_err(const char *func, int lineno, const char *format, ...)
> > +{
> > +	va_list ap;
> > +	int ret;
> > +
> > +	ret = fprintf(stderr, "In %s:%d - ", func, lineno);
> Check return code here, and return on error.
> 
> > +	va_start(ap, format);
> > +	ret += vfprintf(stderr, format, ap);
> 
> ..and here.
> 
> > +	va_end(ap);
> > +
> > +	return ret;
> 
> A negative return value in one call and an valid byte count result for the
> other should produce an error, but here it might not.
> 
> You might argue this is just test code, but then I suggest not checking the
> return values at all.
> 

Indeed the return value is never checked anywhere in the calls to PRINT_ERR
macro, and since the writes are going to stderr it's pretty low risk.
Therefore, I'll remove the return value handling completely as you suggest.

/Bruce
Conor Walsh Sept. 3, 2021, 4:07 p.m. UTC | #3 
> Run basic sanity tests for configuring, starting and stopping a dmadev
> instance to help validate drivers. This also provides the framework for
> future tests for data-path operation.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

Reviewed-by: Conor Walsh <conor.walsh@intel.com>
diff mbox series

Patch

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index bb01e86483..12f7c69629 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -2,6 +2,7 @@ 
  * Copyright(c) 2021 HiSilicon Limited.
  * Copyright(c) 2021 Intel Corporation.
  */
+#include <inttypes.h>
 
 #include <rte_common.h>
 #include <rte_dev.h>
@@ -13,6 +14,77 @@ 
 /* from test_dmadev_api.c */
 extern int test_dmadev_api(uint16_t dev_id);
 
+#define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
+
+static inline int
+__rte_format_printf(3, 4)
+print_err(const char *func, int lineno, const char *format, ...)
+{
+	va_list ap;
+	int ret;
+
+	ret = fprintf(stderr, "In %s:%d - ", func, lineno);
+	va_start(ap, format);
+	ret += vfprintf(stderr, format, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+static int
+test_dmadev_instance(uint16_t dev_id)
+{
+#define TEST_RINGSIZE 512
+	struct rte_dmadev_stats stats;
+	struct rte_dmadev_info info;
+	const struct rte_dmadev_conf conf = { .nb_vchans = 1};
+	const struct rte_dmadev_vchan_conf qconf = {
+			.direction = RTE_DMA_DIR_MEM_TO_MEM,
+			.nb_desc = TEST_RINGSIZE,
+	};
+	const int vchan = 0;
+
+	printf("\n### Test dmadev instance %u\n", dev_id);
+
+	rte_dmadev_info_get(dev_id, &info);
+	if (info.max_vchans < 1) {
+		PRINT_ERR("Error, no channels available on device id %u\n", dev_id);
+		return -1;
+	}
+	if (rte_dmadev_configure(dev_id, &conf) != 0) {
+		PRINT_ERR("Error with rte_dmadev_configure()\n");
+		return -1;
+	}
+	if (rte_dmadev_vchan_setup(dev_id, vchan, &qconf) < 0) {
+		PRINT_ERR("Error with queue configuration\n");
+		return -1;
+	}
+
+	rte_dmadev_info_get(dev_id, &info);
+	if (info.nb_vchans != 1) {
+		PRINT_ERR("Error, no configured queues reported on device id %u\n", dev_id);
+		return -1;
+	}
+
+	if (rte_dmadev_start(dev_id) != 0) {
+		PRINT_ERR("Error with rte_dmadev_start()\n");
+		return -1;
+	}
+	if (rte_dmadev_stats_get(dev_id, vchan, &stats) != 0) {
+		PRINT_ERR("Error with rte_dmadev_stats_get()\n");
+		return -1;
+	}
+	if (stats.completed != 0 || stats.submitted != 0 || stats.errors != 0) {
+		PRINT_ERR("Error device stats are not all zero: completed = %"PRIu64", submitted = %"PRIu64", errors = %"PRIu64"\n",
+				stats.completed, stats.submitted, stats.errors);
+		return -1;
+	}
+
+	rte_dmadev_stop(dev_id);
+	rte_dmadev_stats_reset(dev_id, vchan);
+	return 0;
+}
+
 static int
 test_apis(void)
 {
@@ -35,10 +107,19 @@  test_apis(void)
 static int
 test_dmadev(void)
 {
+	int i;
+
 	/* basic sanity on dmadev infrastructure */
 	if (test_apis() < 0)
 		return -1;
 
+	if (rte_dmadev_count() == 0)
+		return TEST_SKIPPED;
+
+	for (i = 0; i < RTE_DMADEV_MAX_DEVS; i++)
+		if (rte_dmadevices[i].state == RTE_DMADEV_ATTACHED && test_dmadev_instance(i) < 0)
+			return -1;
+
 	return 0;
 }