Comments inline. Thanks, Alex
On 10 November 2014 18:00, Taras Kondratiuk <[email protected]> wrote: > Do you plan to add tests for async out of place? > > Please check comments below. > > On 11/10/2014 05:31 PM, Alexandru Badicioiu wrote: > >> Ping. >> >> On 5 November 2014 11:00, <[email protected] >> <mailto:[email protected]>> wrote: >> >> From: Alexandru Badicioiu <[email protected] >> <mailto:[email protected]>> >> >> This patch adds a suite for sync and async inplace mode of crypto >> APIs. >> Correctness of crypto operation output is verified with known test >> vectors. Various options and functionalities like use session IV >> or operation IV for ciphering are exercised for both modes. >> For async mode there are options to use input packet buffer or a >> separate buffer as the completion event and to set and retrieve the >> context associated with an operation from the completion event. >> >> Signed-off-by: Alexandru Badicioiu <[email protected] >> <mailto:[email protected]>> >> --- >> configure.ac <http://configure.ac> >> | 1 + >> test/cunit/Makefile.am | 2 + >> test/cunit/crypto/Makefile.am | 10 + >> test/cunit/crypto/odp_crypto_test.c | 114 ++++++++ >> test/cunit/crypto/odp_crypto_test_async_inp.c | 371 >> +++++++++++++++++++++++++ >> test/cunit/crypto/odp_crypto_test_async_inp.h | 17 ++ >> test/cunit/crypto/odp_crypto_test_sync_inp.c | 260 >> +++++++++++++++++ >> test/cunit/crypto/odp_crypto_test_sync_inp.h | 17 ++ >> test/cunit/crypto/test_vectors.h | 94 +++++++ >> 9 files changed, 886 insertions(+), 0 deletions(-) >> create mode 100644 test/cunit/crypto/Makefile.am >> create mode 100644 test/cunit/crypto/odp_crypto_test.c >> create mode 100644 test/cunit/crypto/odp_crypto_test_async_inp.c >> create mode 100644 test/cunit/crypto/odp_crypto_test_async_inp.h >> create mode 100644 test/cunit/crypto/odp_crypto_test_sync_inp.c >> create mode 100644 test/cunit/crypto/odp_crypto_test_sync_inp.h >> create mode 100644 test/cunit/crypto/test_vectors.h >> >> diff --git a/configure.ac <http://configure.ac> b/configure.ac >> <http://configure.ac> >> index 1c061e9..298d50b 100644 >> --- a/configure.ac <http://configure.ac> >> +++ b/configure.ac <http://configure.ac> >> >> @@ -177,6 +177,7 @@ AC_CONFIG_FILES([Makefile >> test/Makefile >> test/api_test/Makefile >> test/cunit/Makefile >> + test/cunit/crypto/Makefile >> pkgconfig/libodp.pc]) >> >> AC_SEARCH_LIBS([timer_create],[rt posix4]) >> diff --git a/test/cunit/Makefile.am b/test/cunit/Makefile.am >> index 927a5a5..7611145 100644 >> --- a/test/cunit/Makefile.am >> +++ b/test/cunit/Makefile.am >> @@ -3,6 +3,8 @@ include $(top_srcdir)/test/Makefile.inc >> AM_CFLAGS += -I$(CUNIT_PATH)/include >> AM_LDFLAGS += -L$(CUNIT_PATH)/lib >> >> +SUBDIRS = crypto >> + >> if ODP_CUNIT_ENABLED >> TESTS = ${bin_PROGRAMS} >> check_PROGRAMS = ${bin_PROGRAMS} >> diff --git a/test/cunit/crypto/Makefile.am >> b/test/cunit/crypto/Makefile.am >> new file mode 100644 >> index 0000000..0eb06df >> --- /dev/null >> +++ b/test/cunit/crypto/Makefile.am >> @@ -0,0 +1,10 @@ >> +include $(top_srcdir)/test/Makefile.inc >> + >> +if ODP_CUNIT_ENABLED >> +bin_PROGRAMS = odp_crypto >> +odp_crypto_LDFLAGS = $(AM_LDFLAGS) -static -lcunit >> +endif >> + >> +dist_odp_crypto_SOURCES = odp_crypto_test_async_inp.c \ >> + odp_crypto_test_sync_inp.c \ >> + odp_crypto_test.c >> diff --git a/test/cunit/crypto/odp_crypto_test.c >> b/test/cunit/crypto/odp_crypto_test.c >> new file mode 100644 >> index 0000000..dbf245a >> --- /dev/null >> +++ b/test/cunit/crypto/odp_crypto_test.c >> @@ -0,0 +1,114 @@ >> +/* Copyright (c) 2014, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +#include <odp.h> >> +#include "CUnit/Headers/Basic.h" >> +#include "CUnit/Headers/TestDB.h" >> > > It seems you are using not installed CUnit headers. > This should be <CUnit/Basic.h> and <CUnit/TestDB.h> > The same for all other instances of these headers. > [Alex] Agree. I'll make the changes. > > +#include "odp_crypto_test_async_inp.h" >> +#include "odp_crypto_test_sync_inp.h" >> + >> +#define SHM_PKT_POOL_SIZE (512*2048*2) >> +#define SHM_PKT_POOL_BUF_SIZE (1024 * 32) >> + >> +#define SHM_COMPL_POOL_SIZE (128*1024) >> +#define SHM_COMPL_POOL_BUF_SIZE 128 >> + >> + >> +/* Suites init/finalize funcs */ >> +/* ODP global/local initialization >> + * Packet/Completion event pool creation >> + * Crypto output queue creation >> +*/ >> + >> +static int init(void) >> +{ >> + odp_shm_t shm; >> + void *pool_base = NULL; >> + odp_buffer_pool_t pool; >> + odp_queue_t out_queue; >> + >> + if (odp_init_global(NULL, NULL)) { >> + ODP_ERR("ODP global init failed.\n"); >> + return -1; >> + } >> + odp_init_local(); >> > > Maybe it makes sense to call odp_init_*() from main function. > They are not part of testsuite initialization. [Alex] This is debatable. According to CUnit documentation "This allows the suite to set up and tear down temporary fixtures to support running the tests." so it looks like it supports your view; ODP initialization is not temporary during the execution of all tests. But the other resources (pools, queues) are not really temporary, they are reused by all currently defined suites. I think they should be reused because otherwise we would have a test dependency on something else - e.g. supporting creation/cleanup of the same thing more than once (this would be a separate test I think). We may move to main() any resource which lives for the entire test run outside the init/finalize - in fact CUnit says that init/finalize are optional. > > + >> + shm = odp_shm_reserve("shm_packet_pool", >> + SHM_PKT_POOL_SIZE, >> + ODP_CACHE_LINE_SIZE, 0); >> + >> + pool_base = odp_shm_addr(shm); >> + if (!pool_base) { >> + ODP_ERR("Packet pool allocation failed.\n"); >> + return -1; >> + } >> + >> + pool = odp_buffer_pool_create("packet_pool", pool_base, >> + SHM_PKT_POOL_SIZE, >> + SHM_PKT_POOL_BUF_SIZE, >> + ODP_CACHE_LINE_SIZE, >> + ODP_BUFFER_TYPE_PACKET); >> + if (pool == ODP_BUFFER_POOL_INVALID) { >> + ODP_ERR("Packet pool creation failed.\n"); >> + /* TODO - cleanup */ >> + return -1; >> + } >> + out_queue = odp_queue_create("crypto-out", >> + ODP_QUEUE_TYPE_POLL, NULL); >> + if (out_queue == ODP_QUEUE_INVALID) { >> + ODP_ERR("Crypto outq creation failed.\n"); >> + /* TODO - cleanup */ >> + return -1; >> + } >> + shm = odp_shm_reserve("shm_compl_pool", >> + SHM_COMPL_POOL_SIZE, >> + ODP_CACHE_LINE_SIZE, >> + ODP_SHM_SW_ONLY); >> + pool_base = odp_shm_addr(shm); >> + if (!pool_base) { >> + ODP_ERR("Completion pool allocation failed.\n"); >> + return -1; >> + } >> + pool = odp_buffer_pool_create("compl_pool", pool_base, >> + SHM_COMPL_POOL_SIZE, >> + SHM_COMPL_POOL_BUF_SIZE, >> + ODP_CACHE_LINE_SIZE, >> + ODP_BUFFER_TYPE_RAW); >> + if (pool == ODP_BUFFER_POOL_INVALID) { >> + ODP_ERR("Completion pool creation failed.\n"); >> + return -1; >> + } >> + >> + printf("\tODP version: %s\n", odp_version_api_str()); >> + return 0; >> +} >> +static int finalize(void) >> +{ >> + return 0; >> +} >> + >> + >> +CU_SuiteInfo suites[] = { >> + { ODP_CRYPTO_SYNC_INP , init, NULL, NULL, NULL, >> test_array_sync }, >> + { ODP_CRYPTO_ASYNC_INP , NULL, finalize, NULL, NULL, >> test_array_async }, >> > > You assume that testsuites are called in order. We may add interactive > mode later. It would be better to add init/finalize for each testsuite and > cleanup resources after each testsuite in finalize. [Alex] Regarding the interactive mode, it was agreed (or at least this is what I understood) to run all tests on a given platform, even if the platform does not support a certain mode. > > > + CU_SUITE_INFO_NULL, >> +}; >> + >> +int main(void) >> +{ >> + /* initialize the CUnit test registry */ >> + if (CUE_SUCCESS != CU_initialize_registry()) >> + return CU_get_error(); >> + >> + /* register suites */ >> + CU_register_suites(suites); >> + /* Run all tests using the CUnit Basic interface */ >> + CU_basic_set_mode(CU_BRM_VERBOSE); >> + CU_basic_run_tests(); >> + CU_cleanup_registry(); >> + >> + return CU_get_error(); >> +} >> > >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
