On 3 September 2014 06:43, Savolainen, Petri (NSN - FI/Espoo) <
[email protected]> wrote:

>
>
> From: ext Mike Holmes [mailto:[email protected]]
> Sent: Tuesday, September 02, 2014 11:53 PM
> To: Savolainen, Petri (NSN - FI/Espoo)
> Cc: [email protected]
> Subject: Re: [lng-odp] [PATCH v2] test/unit: add odp_buffer_test
>
>
>
> On 1 September 2014 03:44, Savolainen, Petri (NSN - FI/Espoo) <
> [email protected]> wrote:
>
>
> > -----Original Message-----
> > From: [email protected] [mailto:lng-odp-
> > [email protected]] On Behalf Of ext Mike Holmes
> > Sent: Friday, August 29, 2014 5:41 PM
> > To: [email protected]
> > Subject: [lng-odp] [PATCH v2] test/unit: add odp_buffer_test
> >
> > Signed-off-by: Mike Holmes <[email protected]>
> > Signed-off-by: Anders Roxell <[email protected]>
> > ---
> >
> > v2:
> >  Add dependencies
> >  Add cross compile
> >  Add comments on test cases
> >
> >  .gitignore                  |   1 +
> >  DEPENDENCIES                |  19 +++-
> >  configure.ac                |  20 ++++
> >  test/Makefile.am            |   2 +-
> >  test/api_test/Makefile.am   |   1 +
> >  test/unit/Makefile.am       |  11 +++
> >  test/unit/odp_buffer_test.c | 222
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  7 files changed, 274 insertions(+), 2 deletions(-)
> >  create mode 100644 test/unit/Makefile.am
> >  create mode 100644 test/unit/odp_buffer_test.c
> >
> > diff --git a/.gitignore b/.gitignore
> > index 6a97f17..1000401 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -42,4 +42,5 @@ odp_pktio
> >  odp_timer_test
> >  odp_generator
> >  odp_l2fwd
> > +odp_buffer
> >  doxygen-doc
> > diff --git a/DEPENDENCIES b/DEPENDENCIES
> > index 9597511..f54724d 100644
> > --- a/DEPENDENCIES
> > +++ b/DEPENDENCIES
> > @@ -75,4 +75,21 @@ Prerequisites for building the OpenDataPlane (ODP) API
> >     # Or build 64 bit version of ODP
> >     $ ./configure --host=aarch64-linux-gnu \
> >       --with-openssl-path=/home/user/src/install-openssl-aarch64
> > -   $ make
> > \ No newline at end of file
> > +   $ make
> > +
> > +4. packages needed to build API tests
> > +
> > +4.1 Native compile
> > +
> > +   # Debian/Ubuntu
> > +   $ apt-get install libcunit1-dev
> > +
> > +   On CentOS/RedHat/Fedora systems:
> > +   $ su -c 'yum update CUnit'
> > +
> > +4.2 Cross compile
> > +
> > +   $ git svn clone http://svn.code.sf.net/p/cunit/code/trunk cunit-code
> > +   $ cd cunit-code
> > +   $ ./bootstrap
> > +   $ ./configure --host=arm-linux-gnueabihf --
> > prefix=/home/user/src/install-cunit
> > diff --git a/configure.ac b/configure.ac
> > index 5574f82..d63f9ef 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -73,6 +73,25 @@ AC_ARG_ENABLE([netmap],
> >  AM_CONDITIONAL([ODP_NETMAP_ENABLED], [test x$netmap_support = xyes ])
> >
> >
> >
> ##########################################################################
> > +# Enable/disable Unit tests
> >
> +#########################################################################
> > #
> > +AC_ARG_ENABLE([cunit],
> > +    [  --enable-cunit         Enable/Disable cunit. See cunit-path],
> > +    [if test x$enableval = xyes; then
> > +        unit_support=yes
> > +    fi])
> > +
> > +AC_ARG_WITH([cunit-path],
> > +AC_HELP_STRING([--with-cunit-path=DIR Path to Cunit libs and headers],
> > +               [(automaticly implies --enable-cunit).]),
> > +[CUNIT_PATH=$withval unit_support=yes
> > +],[ AC_MSG_WARN([Cunit not found - continuing without Cunit support])
> > +])
> > +
> > +AC_SUBST(CUNIT_PATH)
> > +AM_CONDITIONAL([ODP_UNIT_ENABLED], [test x$unit_support = xyes ])
> > +
> >
> +#########################################################################
> > #
> >  # Enable/disable ODP_DEBUG_PRINT
> >
> >
> ##########################################################################
> >  ODP_DEBUG=1
> > @@ -152,6 +171,7 @@ AC_CONFIG_FILES([Makefile
> >                example/timer/Makefile
> >                test/Makefile
> >                test/api_test/Makefile
> > +                 test/unit/Makefile
> >                pkgconfig/libodp.pc])
> >
> >  AC_SEARCH_LIBS([timer_create],[rt posix4])
> > diff --git a/test/Makefile.am b/test/Makefile.am
> > index 9bd7db1..dda7d5e 100644
> > --- a/test/Makefile.am
> > +++ b/test/Makefile.am
> > @@ -1 +1 @@
> > -SUBDIRS = api_test
> > +SUBDIRS = api_test unit
> > diff --git a/test/api_test/Makefile.am b/test/api_test/Makefile.am
> > index 5104454..d76bc26 100644
> > --- a/test/api_test/Makefile.am
> > +++ b/test/api_test/Makefile.am
> > @@ -1,6 +1,7 @@
> >  include $(top_srcdir)/test/Makefile.inc
> >
> >  bin_PROGRAMS = odp_atomic odp_shm odp_ring odp_timer_ping
> > +
> >  odp_atomic_LDFLAGS = $(AM_LDFLAGS) -static
> >  odp_shm_LDFLAGS = $(AM_LDFLAGS) -static
> >  odp_ring_LDFLAGS = $(AM_LDFLAGS) -static
> > diff --git a/test/unit/Makefile.am b/test/unit/Makefile.am
> > new file mode 100644
> > index 0000000..9554ed9
> > --- /dev/null
> > +++ b/test/unit/Makefile.am
> > @@ -0,0 +1,11 @@
> > +include $(top_srcdir)/test/Makefile.inc
> > +
> > +AM_CFLAGS += -I$(CUNIT_PATH)/include
> > +AM_LDFLAGS += -L$(CUNIT_PATH)/lib
> > +
> > +if ODP_UNIT_ENABLED
> > +bin_PROGRAMS = odp_buffer
> > +odp_buffer_LDFLAGS = $(AM_LDFLAGS) -static -lcunit
> > +endif
> > +
> > +dist_odp_buffer_SOURCES = odp_buffer_test.c
> > diff --git a/test/unit/odp_buffer_test.c b/test/unit/odp_buffer_test.c
> > new file mode 100644
> > index 0000000..5bc990e
> > --- /dev/null
> > +++ b/test/unit/odp_buffer_test.c
> > @@ -0,0 +1,222 @@
> > +/* Copyright (c) 2014, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier:     BSD-3-Clause
> > + */
> > +
> > +#include "odp.h"
> > +#include "CUnit/Basic.h"
> > +
> > +#define DEFAULT_MSG_POOL_SIZE         (4*1024*1024)
> > +#define DEFAULT_MSG_SIZE         (8)
> > +
> > +static void test_odp_buffer_addr(void)
> > +{
> > +     void *startaddress;
> > +     odp_buffer_pool_t msg_pool;
> > +     odp_buffer_t mybuffer;
> > +     void *pool_base;
> > +     /* correct address returned */
> > +     pool_base = odp_shm_reserve("msg_pool1",
> > +                                 DEFAULT_MSG_POOL_SIZE,
> > +                                 ODP_CACHE_LINE_SIZE);
> > +     msg_pool = odp_buffer_pool_create("msg_pool1",
> > +                                       pool_base,
> > +                                       DEFAULT_MSG_POOL_SIZE,
> > +                                       DEFAULT_MSG_SIZE,
> > +                                       ODP_CACHE_LINE_SIZE,
> > +                                       ODP_BUFFER_TYPE_RAW);
> > +     mybuffer = odp_buffer_alloc(msg_pool);
> > +     startaddress = odp_buffer_addr(mybuffer);
> > +     odp_buffer_free(mybuffer);
> > +     CU_ASSERT_TRUE(startaddress != NULL);
> No error checks for invalid handles ... Test passes also if invalid
> handles are accepted and odp_buffer_addr() returns garbage.
> Thank you for the comments.
>
> This test case is only for buffer_addr, my thinking was that if we add all
> the checks for all the supporting APIs every time each test case will get
> very large and obscure the tests purpose. Previous tests will have already
> proven those APIs work won't they ?
> We can add them for every set up call but I expect the global_init unit
> test to have already been run - what do people think ?
>
> > +}
> > +
> > +static void test_odp_buffer_size(void)
> > +{
> > +     size_t size;
> > +     odp_buffer_pool_t msg_pool;
> > +     odp_buffer_t mybuffer;
> > +     void *pool_base;
> > +     /* correct size returned */
> > +     pool_base = odp_shm_reserve("msg_pool2",
> > +                                 DEFAULT_MSG_POOL_SIZE,
> > +                                 ODP_CACHE_LINE_SIZE);
> > +     msg_pool = odp_buffer_pool_create("msg_pool2", pool_base,
> > +                                       DEFAULT_MSG_POOL_SIZE,
> > +                                       DEFAULT_MSG_SIZE,
> > +                                       ODP_CACHE_LINE_SIZE,
> > +                                       ODP_BUFFER_TYPE_RAW);
> Are you sure you want to do a shm and pool allocation per test. An
> implementation may run out of shm segments or pools (especially when more
> test cases are added).
>
> I wanted to start as close to clean as possible for each API,  so that if
> there is a failure it is with the specific API under test and not due to
> any artifacts from a previous case. But to support this ODP really needs a
> clean up API so that the same buffer can be re-made each time, then we
> don't consume them.
>
> [Petri]
> Already now if an implementation has e.g. only 8 pools (which is
> reasonable number), this test suite would not pass because of running out
> of pools. Before we have shm/pool release calls, you have to reuse shm
> memory/pools across test cases.
>
> Ok, I will modify the flow, I'd like to get people's feedback on if we
should make the tests more autonomous as a general rule however.

>
>
> > +     mybuffer = odp_buffer_alloc(msg_pool);
> > +     size = odp_buffer_size(mybuffer);
> > +     odp_buffer_free(mybuffer);
> > +     CU_ASSERT_TRUE(size == DEFAULT_MSG_SIZE);
> > +     msg_pool = odp_buffer_pool_create("msg_pool2_1", pool_base,
> > +                                       DEFAULT_MSG_POOL_SIZE,
> > +                                       -1,
> > +                                       ODP_CACHE_LINE_SIZE,
> > +                                       ODP_BUFFER_TYPE_RAW);
> Same pool_base should not be used twice. So the failure may be also caused
> by that,
>
> The docs don't say this so we should add that information ?
> Basically you can only create one buffer pool per odp shared memory
> reservation.
>
> [Petri] You give the resource (shm memory) to the pool create function,
> it's now pool memory and you cannot touch it any more or use it for another
> purpose. Similarly you cannot enqueue the same buffer into two different
> queues, and so


>
> -Petri
>
>
>


-- 
*Mike Holmes*
Linaro Technical Manager / Lead
LNG - ODP
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to