[RFC,4/7] app/test: add basic dmadev copy tests
Checks
Commit Message
For each dmadev instance, perform some basic copy tests to validate that
functionality.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
app/test/test_dmadev.c | 157 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 157 insertions(+)
Comments
On Fri, Aug 27, 2021 at 12:03 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> For each dmadev instance, perform some basic copy tests to validate that
> functionality.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> app/test/test_dmadev.c | 157 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 157 insertions(+)
>
> diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> index f895556d29..a9f7d34a94 100644
> --- a/app/test/test_dmadev.c
> +++ b/app/test/test_dmadev.c
> @@ -1,11 +1,14 @@
> /* SPDX-License-Identifier: BSD-3-Clause
> * Copyright(c) 2021 HiSilicon Limited.
> */
> +#include <unistd.h>
>
> #include <rte_common.h>
> #include <rte_dev.h>
> #include <rte_dmadev.h>
> #include <rte_bus_vdev.h>
> +#include <rte_mbuf.h>
> +#include <rte_random.h>
>
> #include "test.h"
>
> @@ -14,6 +17,11 @@ extern int test_dmadev_api(uint16_t dev_id);
>
> #define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
>
> +#define COPY_LEN 1024
> +
> +static struct rte_mempool *pool;
> +static uint16_t id_count;
> +
> static inline int
> __rte_format_printf(3, 4)
> print_err(const char *func, int lineno, const char *format, ...)
> @@ -29,10 +37,123 @@ print_err(const char *func, int lineno, const char *format, ...)
> return ret;
> }
>
> +static int
> +test_enqueue_copies(int dev_id, uint16_t vchan)
> +{
> + unsigned int i;
> + uint16_t id;
> +
> + /* test doing a single copy */
> + do {
> + struct rte_mbuf *src, *dst;
> + char *src_data, *dst_data;
> +
> + src = rte_pktmbuf_alloc(pool);
> + dst = rte_pktmbuf_alloc(pool);
> + src_data = rte_pktmbuf_mtod(src, char *);
> + dst_data = rte_pktmbuf_mtod(dst, char *);
> +
> + for (i = 0; i < COPY_LEN; i++)
> + src_data[i] = rte_rand() & 0xFF;
> +
> + id = rte_dmadev_copy(dev_id, vchan, src->buf_iova + src->data_off,
> + dst->buf_iova + dst->data_off, COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);
> + if (id != id_count) {
> + PRINT_ERR("Error with rte_dmadev_copy, got %u, expected %u\n",
> + id, id_count);
> + return -1;
> + }
> +
> + /* give time for copy to finish, then check it was done */
> + usleep(10);
Across series, We have this pattern. IMHO, It is not portable.
Can we have a helper function either common lib code or test code to
busy poll for completion with timeout? and in test code, we have a much bigger
timeout to accommodate all the devices. That way if the driver completes
early it can continue to execute and makes it portable.
> +
> + for (i = 0; i < COPY_LEN; i++) {
> + if (dst_data[i] != src_data[i]) {
> + PRINT_ERR("Data mismatch at char %u [Got %02x not %02x]\n", i,
> + dst_data[i], src_data[i]);
> + rte_dmadev_dump(dev_id, stderr);
> + return -1;
> + }
> + }
> +
> + /* now check completion works */
> + if (rte_dmadev_completed(dev_id, vchan, 1, &id, NULL) != 1) {
> + PRINT_ERR("Error with rte_dmadev_completed\n");
> + return -1;
> + }
> + if (id != id_count) {
> + PRINT_ERR("Error:incorrect job id received, %u [expected %u]\n",
> + id, id_count);
> + return -1;
> + }
> +
On Fri, Aug 27, 2021 at 12:44:17PM +0530, Jerin Jacob wrote:
> On Fri, Aug 27, 2021 at 12:03 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > For each dmadev instance, perform some basic copy tests to validate that
> > functionality.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > app/test/test_dmadev.c | 157 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 157 insertions(+)
> >
> > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> > index f895556d29..a9f7d34a94 100644
> > --- a/app/test/test_dmadev.c
> > +++ b/app/test/test_dmadev.c
> > @@ -1,11 +1,14 @@
> > /* SPDX-License-Identifier: BSD-3-Clause
> > * Copyright(c) 2021 HiSilicon Limited.
> > */
> > +#include <unistd.h>
> >
> > #include <rte_common.h>
> > #include <rte_dev.h>
> > #include <rte_dmadev.h>
> > #include <rte_bus_vdev.h>
> > +#include <rte_mbuf.h>
> > +#include <rte_random.h>
> >
> > #include "test.h"
> >
> > @@ -14,6 +17,11 @@ extern int test_dmadev_api(uint16_t dev_id);
> >
> > #define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
> >
> > +#define COPY_LEN 1024
> > +
> > +static struct rte_mempool *pool;
> > +static uint16_t id_count;
> > +
> > static inline int
> > __rte_format_printf(3, 4)
> > print_err(const char *func, int lineno, const char *format, ...)
> > @@ -29,10 +37,123 @@ print_err(const char *func, int lineno, const char *format, ...)
> > return ret;
> > }
> >
> > +static int
> > +test_enqueue_copies(int dev_id, uint16_t vchan)
> > +{
> > + unsigned int i;
> > + uint16_t id;
> > +
> > + /* test doing a single copy */
> > + do {
> > + struct rte_mbuf *src, *dst;
> > + char *src_data, *dst_data;
> > +
> > + src = rte_pktmbuf_alloc(pool);
> > + dst = rte_pktmbuf_alloc(pool);
> > + src_data = rte_pktmbuf_mtod(src, char *);
> > + dst_data = rte_pktmbuf_mtod(dst, char *);
> > +
> > + for (i = 0; i < COPY_LEN; i++)
> > + src_data[i] = rte_rand() & 0xFF;
> > +
> > + id = rte_dmadev_copy(dev_id, vchan, src->buf_iova + src->data_off,
> > + dst->buf_iova + dst->data_off, COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);
> > + if (id != id_count) {
> > + PRINT_ERR("Error with rte_dmadev_copy, got %u, expected %u\n",
> > + id, id_count);
> > + return -1;
> > + }
> > +
> > + /* give time for copy to finish, then check it was done */
> > + usleep(10);
>
> Across series, We have this pattern. IMHO, It is not portable.
> Can we have a helper function either common lib code or test code to
> busy poll for completion with timeout? and in test code, we have a much bigger
> timeout to accommodate all the devices. That way if the driver completes
> early it can continue to execute and makes it portable.
>
It's less that ideal, I admit, but I'm not sure it's not portable. The main
concern here for these unit tests is to try and ensure that at all times
the state of the device is fully known, so the delays are there to ensure
that the device has completely finished all work given to it. The
suggestion of having a polling loop to gather completions won't work for
all scenarios are it doesn't give us the same degree of control in that we
cannot know the exact state of job completion when running each poll -
meaning that running tests such as for gathering all completions in one go,
or in parts of a fixed size are hard to do.
The other alternative is to provide an API in dmadev to await quiescence of
a DMA device. This would be a better alternative to having the fixed
delays, but means a new API for testing only. I can investigate this for
the next version of the patchset but it means more work for driver writers.
[This could be perhaps worked around by having dmadev implement a
"usleep()" call for any drivers that don't implement the function, which
would provide a quicker way for driver writers to test incomplete drivers]
Also, if having "large" delays is a concern, I don't think it's a major
one. With all delays as usleep(10) - and 10 microseconds should be a long
time for a HW device - an test run of an optimized build of DPDK takes ~1.5
seconds for 2 dmadev instances, and a debug build only ~2.5 seconds.
Increasing the delay tenfold to 100 usec, brings the optimized test run to
<2.5 secs and a debug build run to ~4 seconds i.e. 2 seconds per device.
Therefore I don't believe having delays in this code is a real problem
other than being a less-elegant solution than a polling-based one.
In any case, I'll see about adding an "all-jobs-done" API to dmadev for v2
to remove these delays as much as possible.
Regards,
/Bruce
@@ -1,11 +1,14 @@
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright(c) 2021 HiSilicon Limited.
*/
+#include <unistd.h>
#include <rte_common.h>
#include <rte_dev.h>
#include <rte_dmadev.h>
#include <rte_bus_vdev.h>
+#include <rte_mbuf.h>
+#include <rte_random.h>
#include "test.h"
@@ -14,6 +17,11 @@ extern int test_dmadev_api(uint16_t dev_id);
#define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
+#define COPY_LEN 1024
+
+static struct rte_mempool *pool;
+static uint16_t id_count;
+
static inline int
__rte_format_printf(3, 4)
print_err(const char *func, int lineno, const char *format, ...)
@@ -29,10 +37,123 @@ print_err(const char *func, int lineno, const char *format, ...)
return ret;
}
+static int
+test_enqueue_copies(int dev_id, uint16_t vchan)
+{
+ unsigned int i;
+ uint16_t id;
+
+ /* test doing a single copy */
+ do {
+ struct rte_mbuf *src, *dst;
+ char *src_data, *dst_data;
+
+ src = rte_pktmbuf_alloc(pool);
+ dst = rte_pktmbuf_alloc(pool);
+ src_data = rte_pktmbuf_mtod(src, char *);
+ dst_data = rte_pktmbuf_mtod(dst, char *);
+
+ for (i = 0; i < COPY_LEN; i++)
+ src_data[i] = rte_rand() & 0xFF;
+
+ id = rte_dmadev_copy(dev_id, vchan, src->buf_iova + src->data_off,
+ dst->buf_iova + dst->data_off, COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);
+ if (id != id_count) {
+ PRINT_ERR("Error with rte_dmadev_copy, got %u, expected %u\n",
+ id, id_count);
+ return -1;
+ }
+
+ /* give time for copy to finish, then check it was done */
+ usleep(10);
+
+ for (i = 0; i < COPY_LEN; i++) {
+ if (dst_data[i] != src_data[i]) {
+ PRINT_ERR("Data mismatch at char %u [Got %02x not %02x]\n", i,
+ dst_data[i], src_data[i]);
+ rte_dmadev_dump(dev_id, stderr);
+ return -1;
+ }
+ }
+
+ /* now check completion works */
+ if (rte_dmadev_completed(dev_id, vchan, 1, &id, NULL) != 1) {
+ PRINT_ERR("Error with rte_dmadev_completed\n");
+ return -1;
+ }
+ if (id != id_count) {
+ PRINT_ERR("Error:incorrect job id received, %u [expected %u]\n",
+ id, id_count);
+ return -1;
+ }
+
+ rte_pktmbuf_free(src);
+ rte_pktmbuf_free(dst);
+
+ /* now check completion works */
+ if (rte_dmadev_completed(dev_id, 0, 1, NULL, NULL) != 0) {
+ PRINT_ERR("Error with rte_dmadev_completed in empty check\n");
+ return -1;
+ }
+ id_count++;
+
+ } while (0);
+
+ /* test doing a multiple single copies */
+ do {
+ const uint16_t max_ops = 4;
+ struct rte_mbuf *src, *dst;
+ char *src_data, *dst_data;
+
+ src = rte_pktmbuf_alloc(pool);
+ dst = rte_pktmbuf_alloc(pool);
+ src_data = rte_pktmbuf_mtod(src, char *);
+ dst_data = rte_pktmbuf_mtod(dst, char *);
+
+ for (i = 0; i < COPY_LEN; i++)
+ src_data[i] = rte_rand() & 0xFF;
+
+ /* perform the same copy <max_ops> times */
+ for (i = 0; i < max_ops; i++) {
+ if (rte_dmadev_copy(dev_id, vchan,
+ src->buf_iova + src->data_off,
+ dst->buf_iova + dst->data_off,
+ COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT) != id_count++) {
+ PRINT_ERR("Error with rte_dmadev_copy\n");
+ return -1;
+ }
+ }
+ usleep(10);
+
+ if ((i = rte_dmadev_completed(dev_id, vchan, max_ops * 2, &id, NULL)) != max_ops) {
+ PRINT_ERR("Error with rte_dmadev_completed, got %u not %u\n", i, max_ops);
+ return -1;
+ }
+ if (id != id_count - 1) {
+ PRINT_ERR("Error, incorrect job id returned: got %u not %u\n",
+ id, id_count - 1);
+ return -1;
+ }
+ for (i = 0; i < COPY_LEN; i++) {
+ if (dst_data[i] != src_data[i]) {
+ PRINT_ERR("Data mismatch at char %u\n", i);
+ return -1;
+ }
+ }
+ rte_pktmbuf_free(src);
+ rte_pktmbuf_free(dst);
+ } while (0);
+
+ return 0;
+}
+
static int
test_dmadev_instance(uint16_t dev_id)
{
#define TEST_RINGSIZE 512
+ struct rte_dmadev_stats stats;
+ int i;
+
/* Setup of the dmadev device. 8< */
struct rte_dmadev_info info;
const struct rte_dmadev_conf conf = { .nb_vchans = 1};
@@ -68,9 +189,45 @@ test_dmadev_instance(uint16_t dev_id)
PRINT_ERR("Error with rte_rawdev_start()\n");
return -1;
}
+ id_count = 0;
+ /* create a mempool for running tests */
+ pool = rte_pktmbuf_pool_create("TEST_DMADEV_POOL",
+ TEST_RINGSIZE * 2, /* n == num elements */
+ 32, /* cache size */
+ 0, /* priv size */
+ 2048, /* data room size */
+ info.device->numa_node);
+ if (pool == NULL) {
+ PRINT_ERR("Error with mempool creation\n");
+ return -1;
+ }
+
+ /* run the test cases, use many iterations to ensure UINT16_MAX id wraparound */
+ printf("DMA Dev: %u, Running Copy Tests\n", dev_id);
+ for (i = 0; i < 640; i++) {
+
+ if (test_enqueue_copies(dev_id, vchan) != 0) {
+ printf("Error with iteration %d\n", i);
+ rte_dmadev_dump(dev_id, stdout);
+ goto err;
+ }
+
+ rte_dmadev_stats_get(dev_id, 0, &stats);
+ printf("Ops submitted: %"PRIu64"\t", stats.submitted);
+ printf("Ops completed: %"PRIu64"\r", stats.completed);
+ }
+ printf("\n");
+
+
+ rte_mempool_free(pool);
rte_dmadev_stop(dev_id);
return 0;
+
+err:
+ rte_mempool_free(pool);
+ rte_dmadev_stop(dev_id);
+ return -1;
}
static int