<snip>
> On Wed, Jan 15, 2020 at 11:25:08PM -0600, Honnappa Nagarahalli wrote:
> > Add basic infrastructure to test rte_ring_xxx_elem APIs.
> > Adjust the existing test cases to test for various ring element sizes.
> >
> > Signed-off-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> > Reviewed-by: Gavin Hu <gavin...@arm.com>
> > ---
> > app/test/test_ring.c | 1342
> > +++++++++++++++++++++---------------------
> > app/test/test_ring.h | 187 ++++++
> > 2 files changed, 850 insertions(+), 679 deletions(-) create mode
> > 100644 app/test/test_ring.h
> >
> > diff --git a/app/test/test_ring.c b/app/test/test_ring.c index
> > aaf1e70ad..c08500eca 100644
> > --- a/app/test/test_ring.c
> > +++ b/app/test/test_ring.c
> > @@ -23,11 +23,13 @@
> > #include <rte_branch_prediction.h>
> > #include <rte_malloc.h>
> > #include <rte_ring.h>
> > +#include <rte_ring_elem.h>
> > #include <rte_random.h>
> > #include <rte_errno.h>
> > #include <rte_hexdump.h>
> >
> > #include "test.h"
> > +#include "test_ring.h"
> >
> > /*
> > * Ring
>
> As you are changing a lot of things, maybe it's an opportunity to update or
> remove the comment at the beginning of the file.
I have removed specific comments. I have converted it into generic comments.
>
>
> > @@ -55,8 +57,6 @@
> > #define RING_SIZE 4096
> > #define MAX_BULK 32
> >
> > -static rte_atomic32_t synchro;
> > -
> > #define TEST_RING_VERIFY(exp)
> \
> > if (!(exp)) { \
> > printf("error at %s:%d\tcondition " #exp " failed\n", \
> > @@ -67,808 +67,792 @@ static rte_atomic32_t synchro;
> >
> > #define TEST_RING_FULL_EMTPY_ITER 8
> >
> > -/*
> > - * helper routine for test_ring_basic
> > - */
> > -static int
> > -test_ring_basic_full_empty(struct rte_ring *r, void * const src[],
> > void *dst[])
> > +static int esize[] = {-1, 4, 8, 16, 20};
>
> it could be const
Yes
>
> [...]
>
> > +/*
> > + * Burst and bulk operations with sp/sc, mp/mc and default (during
> > +creation)
> > + * Random number of elements are enqueued and dequeued.
> > + */
> > +static int
> > +test_ring_burst_bulk_tests1(unsigned int api_type) {
> > + struct rte_ring *r;
> > + void **src = NULL, **cur_src = NULL, **dst = NULL, **cur_dst = NULL;
> > + int ret;
> > + unsigned int i, j;
> > + int rand;
> > + const unsigned int rsz = RING_SIZE - 1;
> >
> > - /* check data */
> > - if (memcmp(src, dst, cur_dst - dst)) {
> > - rte_hexdump(stdout, "src", src, cur_src - src);
> > - rte_hexdump(stdout, "dst", dst, cur_dst - dst);
> > - printf("data after dequeue is not the same\n");
> > - goto fail;
> > - }
> > + for (i = 0; i < RTE_DIM(esize); i++) {
> > + test_ring_print_test_string("Test standard ring", api_type,
> > + esize[i]);
> >
> > - cur_src = src;
> > - cur_dst = dst;
> > + /* Create the ring */
> > + r = test_ring_create("test_ring_burst_bulk_tests", esize[i],
> > + RING_SIZE, SOCKET_ID_ANY, 0);
> >
> > - ret = rte_ring_mp_enqueue(r, cur_src);
> > - if (ret != 0)
> > - goto fail;
> > + /* alloc dummy object pointers */
> > + src = test_ring_calloc(RING_SIZE * 2, esize[i]);
> > + if (src == NULL)
> > + goto fail;
> > + test_ring_mem_init(src, RING_SIZE * 2, esize[i]);
> > + cur_src = src;
> >
> > - ret = rte_ring_mc_dequeue(r, cur_dst);
> > - if (ret != 0)
> > - goto fail;
> > + /* alloc some room for copied objects */
> > + dst = test_ring_calloc(RING_SIZE * 2, esize[i]);
> > + if (dst == NULL)
> > + goto fail;
> > + cur_dst = dst;
> > +
> > + printf("Random full/empty test\n");
> > +
> > + for (j = 0; j != TEST_RING_FULL_EMTPY_ITER; j++) {
> > + /* random shift in the ring */
> > + rand = RTE_MAX(rte_rand() % RING_SIZE, 1UL);
> > + printf("%s: iteration %u, random shift: %u;\n",
> > + __func__, i, rand);
> > + ret = test_ring_enqueue(r, cur_src, esize[i], rand,
> > + api_type);
> > + TEST_RING_VERIFY(ret != 0);
> > +
> > + ret = test_ring_dequeue(r, cur_dst, esize[i], rand,
> > + api_type);
> > + TEST_RING_VERIFY(ret == rand);
> > +
> > + /* fill the ring */
> > + ret = test_ring_enqueue(r, cur_src, esize[i], rsz,
> > + api_type);
> > + TEST_RING_VERIFY(ret != 0);
> > +
> > + TEST_RING_VERIFY(rte_ring_free_count(r) == 0);
> > + TEST_RING_VERIFY(rsz == rte_ring_count(r));
> > + TEST_RING_VERIFY(rte_ring_full(r));
> > + TEST_RING_VERIFY(rte_ring_empty(r) == 0);
> > +
> > + /* empty the ring */
> > + ret = test_ring_dequeue(r, cur_dst, esize[i], rsz,
> > + api_type);
> > + TEST_RING_VERIFY(ret == (int)rsz);
> > + TEST_RING_VERIFY(rsz == rte_ring_free_count(r));
> > + TEST_RING_VERIFY(rte_ring_count(r) == 0);
> > + TEST_RING_VERIFY(rte_ring_full(r) == 0);
> > + TEST_RING_VERIFY(rte_ring_empty(r));
> > +
> > + /* check data */
> > + TEST_RING_VERIFY(memcmp(src, dst, rsz) == 0);
> > + }
> > +
> > + /* Free memory before test completed */
> > + rte_ring_free(r);
> > + rte_free(src);
> > + rte_free(dst);
>
> I think they should be reset to NULL to avoid a double free if next iteration
> fails.
>
> There are several places like this, I think it can be done even if not really
> needed.
Will change all of them
>
> [...]
>
> > diff --git a/app/test/test_ring.h b/app/test/test_ring.h new file mode
> > 100644 index 000000000..26716e4f8
> > --- /dev/null
> > +++ b/app/test/test_ring.h
> > @@ -0,0 +1,187 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2019 Arm Limited
> > + */
> > +
> > +#include <rte_malloc.h>
> > +#include <rte_ring.h>
> > +#include <rte_ring_elem.h>
> > +
> > +/* API type to call
> > + * rte_ring_<sp/mp or sc/mc>_enqueue_<bulk/burst>
> > + * TEST_RING_THREAD_DEF - Uses configured SPSC/MPMC calls
> > + * TEST_RING_THREAD_SPSC - Calls SP or SC API
> > + * TEST_RING_THREAD_MPMC - Calls MP or MC API */ #define
> > +TEST_RING_THREAD_DEF 1 #define TEST_RING_THREAD_SPSC 2 #define
> > +TEST_RING_THREAD_MPMC 4
> > +
> > +/* API type to call
> > + * SL - Calls single element APIs
> > + * BL - Calls bulk APIs
> > + * BR - Calls burst APIs
> > + */
>
> The comment was not updated according to macro name.
Will fix
>
