Message ID | 20160503094445.GA24275@erachmi-ericsson.ki.sw.ericsson.se |
---|---|
State | New |
Headers | show |
v5 LGTM (looks good to me). Thanks Christophe! Reviewed-by: Brian Brooks <brian.brooks@linaro.org> On 05/03 11:44:47, Christophe Milard wrote: > [GIT PULL ODPv5] running things in process mode > > Since v4: > -updates following Brian's comments > > Since v3: > -fixed rebase error (Christophe) > -rebased > > Since v2: > -serious rebase following clash with dba05a28 (api: init: add instance handle) > -squashing the validation changes. I am not very keen on squashing > small isolated modifs to larger patches. don't really see the gain. > Squashing more will put even more unrelated things together. > > Since v1: > -variable declaration gathered in function 's head (Petri) > -linux prefix removed from helper's types and function names (Mike, Christophe) > > Hi, > > This patch series adds the ability to run tests/ exemples / perf-test > in "process mode" (i.e letting OPD threads being linux processes) > It it hence tackling ODP-171. > > This is achieved in 2 main steps: > > A] > The 2 pairs of helper functions: > odph_linux_pthread_create(), odph_linux_pthread_join() > and > odph_linux_process_fork_n(), odph_linux_process_wait_n() > are replaced by: > odph_linux_odpthreads_create() and odph_linux_odpthreads_join() > The latter's callers are unaware of the actual implementation of the ODP > thread (making test truly platform agnostic). > The helper functions decide at run time whether an odp thread is to be > a linux process or pthread based on the command line argument. > > B] each test/example now calls a new helper function, > odph_linux_parse_options(), so that the helper can get its own arguments > out of the command line. > Currentely supported args are: --odph_proc, --odph_thread. > Defaults assumes thread. specifying both options runs in mixed mode. > > The changed are first done on the shmem tests, and thereafter propagated > to other helper users. > Note that this patch series enable the option but does not affect > make check at this time: make check still calls the tests with no options > which default to thread mode. > > This patch series nicely splits it two groups (you can split your review > there): > 1) up to "validation: pktio: adding command line argument parsing", the new > helper functions are introduced, and used in the validation tests. > 2) from "helper: adding a function to merge getopt parameters" the ability > to parse command line arguments in subset in added and applied to > the example and performance tests. > > Hope this makes sence for you too! > > ---- > > The following changes since commit 04149e64e897d79229cf06539cda4d5f29bc7a90: > > configure.ac: remove duplicated test example configure (2016-04-29 18:38:14 +0300) > > are available in the git repository at: > > https://git.linaro.org/people/christophe.milard/odp.git test_in_process_mode_v5 > > for you to fetch changes up to f66d32f2bba5ceca85a894588a3262f75a1e5783: > > helper: removing dead code (2016-05-03 11:39:44 +0200) > > ---------------------------------------------------------------- > Christophe Milard (36): > helpers: adding command line argument parsing > validation: common: adding command line argument parsing > validation: shmem: adding command line argument parsing > helpers: linux: creating common entry for process and thread > helpers: linux: creating functions to handle odpthreads > helper: test: adding odpthread functions tests > validation: using implementation agnostic function for ODP threads > validation: most tests: adding command line argument parsing > validation: init: adding command line argument parsing > validation: pktio: adding command line argument parsing > helper: adding a function to merge getopt parameters > helper: parsing the complete set of options > performance: odp_scheduling: proc mode done by helper > performance: odp_pktio_perf: using agnostic function for ODP threads > performance: odp_pktio_perf: adding helper cmd line parsing > performance: odp_l2fwd: using agnostic function for ODP threads > performance: odp_l2fwd: adding helper cmd line parsing > performance: crypto: using agnostic function for ODP threads > performance: crypto: adding helper cmd line parsing > example: classifier: using agnostic function for ODP threads > example: classifier: adding helper cmd line parsing > example: generator: using agnostic function for ODP threads > example: generator: adding helper cmd line parsing > example: ipsec: using agnostic function for ODP threads > example: ipsec: adding helper cmd line parsing > example: l2fwd_simple: using agnostic function for ODP threads > example: l2fwd_simple: adding helper cmd line parsing > example: pktio: using agnostic function for ODP threads > example: pktio: adding helper cmd line parsing > example: time: using agnostic function for ODP threads > example: time: adding helper cmd line parsing > example: timer: using agnostic function for ODP threads > example: timer: adding helper cmd line parsing > example: switch: using agnostic function for ODP threads > example: switch: adding helper cmd line parsing > helper: removing dead code > > example/classifier/odp_classifier.c | 39 +- > example/generator/odp_generator.c | 45 ++- > example/ipsec/odp_ipsec.c | 27 +- > example/l2fwd_simple/odp_l2fwd_simple.c | 47 ++- > example/packet/odp_pktio.c | 45 ++- > example/switch/odp_switch.c | 26 +- > example/time/time_global_test.c | 17 +- > example/timer/odp_timer_test.c | 26 +- > helper/include/odp/helper/linux.h | 181 +++++---- > helper/linux.c | 433 ++++++++++++++------- > helper/test/.gitignore | 3 +- > helper/test/Makefile.am | 14 +- > helper/test/{thread.c => odpthreads.c} | 33 +- > helper/test/odpthreads_as_mixed | 15 + > helper/test/odpthreads_as_processes | 15 + > helper/test/odpthreads_as_pthreads | 15 + > helper/test/process.c | 92 ----- > platform/linux-generic/test/pktio/pktio_run | 21 +- > platform/linux-generic/test/pktio/pktio_run_dpdk | 17 +- > platform/linux-generic/test/pktio/pktio_run_netmap | 5 +- > platform/linux-generic/test/pktio/pktio_run_pcap | 5 +- > platform/linux-generic/test/pktio/pktio_run_tap | 6 +- > test/performance/odp_crypto.c | 24 +- > test/performance/odp_l2fwd.c | 37 +- > test/performance/odp_pktio_perf.c | 34 +- > test/performance/odp_scheduling.c | 91 ++--- > test/validation/atomic/atomic.c | 40 +- > test/validation/atomic/atomic.h | 2 +- > test/validation/atomic/atomic_main.c | 4 +- > test/validation/barrier/barrier.c | 14 +- > test/validation/barrier/barrier.h | 2 +- > test/validation/barrier/barrier_main.c | 4 +- > test/validation/buffer/buffer.c | 10 +- > test/validation/buffer/buffer.h | 2 +- > test/validation/buffer/buffer_main.c | 4 +- > test/validation/classification/classification.c | 10 +- > test/validation/classification/classification.h | 2 +- > .../classification/classification_main.c | 4 +- > test/validation/common/odp_cunit_common.c | 24 +- > test/validation/common/odp_cunit_common.h | 6 +- > test/validation/config/config.c | 10 +- > test/validation/config/config.h | 2 +- > test/validation/config/config_main.c | 4 +- > test/validation/cpumask/cpumask.c | 10 +- > test/validation/cpumask/cpumask.h | 2 +- > test/validation/cpumask/cpumask_main.c | 4 +- > test/validation/crypto/crypto.c | 6 +- > test/validation/crypto/crypto.h | 2 +- > test/validation/crypto/crypto_main.c | 4 +- > test/validation/errno/errno.c | 10 +- > test/validation/errno/errno.h | 2 +- > test/validation/errno/errno_main.c | 4 +- > test/validation/hash/hash.c | 10 +- > test/validation/hash/hash.h | 2 +- > test/validation/hash/hash_main.c | 4 +- > test/validation/init/init.c | 18 +- > test/validation/init/init.h | 6 +- > test/validation/init/init_main_abort.c | 4 +- > test/validation/init/init_main_log.c | 4 +- > test/validation/init/init_main_ok.c | 4 +- > test/validation/lock/lock.c | 50 +-- > test/validation/lock/lock.h | 2 +- > test/validation/lock/lock_main.c | 4 +- > test/validation/packet/packet.c | 10 +- > test/validation/packet/packet.h | 2 +- > test/validation/packet/packet_main.c | 4 +- > test/validation/pktio/pktio.c | 10 +- > test/validation/pktio/pktio.h | 2 +- > test/validation/pktio/pktio_main.c | 4 +- > test/validation/pool/pool.c | 10 +- > test/validation/pool/pool.h | 2 +- > test/validation/pool/pool_main.c | 4 +- > test/validation/queue/queue.c | 10 +- > test/validation/queue/queue.h | 2 +- > test/validation/queue/queue_main.c | 4 +- > test/validation/random/random.c | 10 +- > test/validation/random/random.h | 2 +- > test/validation/random/random_main.c | 4 +- > test/validation/scheduler/scheduler.c | 18 +- > test/validation/scheduler/scheduler.h | 2 +- > test/validation/scheduler/scheduler_main.c | 4 +- > test/validation/shmem/shmem.c | 16 +- > test/validation/shmem/shmem.h | 2 +- > test/validation/shmem/shmem_main.c | 4 +- > test/validation/std_clib/std_clib.c | 10 +- > test/validation/std_clib/std_clib.h | 2 +- > test/validation/std_clib/std_clib_main.c | 4 +- > test/validation/system/system.c | 10 +- > test/validation/system/system.h | 2 +- > test/validation/system/system_main.c | 4 +- > test/validation/thread/thread.c | 14 +- > test/validation/thread/thread.h | 2 +- > test/validation/thread/thread_main.c | 4 +- > test/validation/time/time.c | 10 +- > test/validation/time/time.h | 2 +- > test/validation/time/time_main.c | 4 +- > test/validation/timer/timer.c | 10 +- > test/validation/timer/timer.h | 2 +- > test/validation/timer/timer_main.c | 4 +- > test/validation/traffic_mngr/traffic_mngr.c | 6 +- > test/validation/traffic_mngr/traffic_mngr.h | 2 +- > test/validation/traffic_mngr/traffic_mngr_main.c | 4 +- > 102 files changed, 1113 insertions(+), 718 deletions(-) > rename helper/test/{thread.c => odpthreads.c} (71%) > create mode 100755 helper/test/odpthreads_as_mixed > create mode 100755 helper/test/odpthreads_as_processes > create mode 100755 helper/test/odpthreads_as_pthreads > delete mode 100644 helper/test/process.c
Thanks for the time you took for it, Brian! On 3 May 2016 at 15:56, Brian Brooks <brian.brooks@linaro.org> wrote: > v5 LGTM (looks good to me). Thanks Christophe! > > Reviewed-by: Brian Brooks <brian.brooks@linaro.org> > > On 05/03 11:44:47, Christophe Milard wrote: > > [GIT PULL ODPv5] running things in process mode > > > > Since v4: > > -updates following Brian's comments > > > > Since v3: > > -fixed rebase error (Christophe) > > -rebased > > > > Since v2: > > -serious rebase following clash with dba05a28 (api: init: add instance > handle) > > -squashing the validation changes. I am not very keen on squashing > > small isolated modifs to larger patches. don't really see the gain. > > Squashing more will put even more unrelated things together. > > > > Since v1: > > -variable declaration gathered in function 's head (Petri) > > -linux prefix removed from helper's types and function names (Mike, > Christophe) > > > > Hi, > > > > This patch series adds the ability to run tests/ exemples / perf-test > > in "process mode" (i.e letting OPD threads being linux processes) > > It it hence tackling ODP-171. > > > > This is achieved in 2 main steps: > > > > A] > > The 2 pairs of helper functions: > > odph_linux_pthread_create(), odph_linux_pthread_join() > > and > > odph_linux_process_fork_n(), odph_linux_process_wait_n() > > are replaced by: > > odph_linux_odpthreads_create() and odph_linux_odpthreads_join() > > The latter's callers are unaware of the actual implementation of the ODP > > thread (making test truly platform agnostic). > > The helper functions decide at run time whether an odp thread is to be > > a linux process or pthread based on the command line argument. > > > > B] each test/example now calls a new helper function, > > odph_linux_parse_options(), so that the helper can get its own arguments > > out of the command line. > > Currentely supported args are: --odph_proc, --odph_thread. > > Defaults assumes thread. specifying both options runs in mixed mode. > > > > The changed are first done on the shmem tests, and thereafter propagated > > to other helper users. > > Note that this patch series enable the option but does not affect > > make check at this time: make check still calls the tests with no options > > which default to thread mode. > > > > This patch series nicely splits it two groups (you can split your review > > there): > > 1) up to "validation: pktio: adding command line argument parsing", the > new > > helper functions are introduced, and used in the validation tests. > > 2) from "helper: adding a function to merge getopt parameters" the > ability > > to parse command line arguments in subset in added and applied to > > the example and performance tests. > > > > Hope this makes sence for you too! > > > > ---- > > > > The following changes since commit > 04149e64e897d79229cf06539cda4d5f29bc7a90: > > > > configure.ac: remove duplicated test example configure (2016-04-29 > 18:38:14 +0300) > > > > are available in the git repository at: > > > > https://git.linaro.org/people/christophe.milard/odp.git > test_in_process_mode_v5 > > > > for you to fetch changes up to f66d32f2bba5ceca85a894588a3262f75a1e5783: > > > > helper: removing dead code (2016-05-03 11:39:44 +0200) > > > > ---------------------------------------------------------------- > > Christophe Milard (36): > > helpers: adding command line argument parsing > > validation: common: adding command line argument parsing > > validation: shmem: adding command line argument parsing > > helpers: linux: creating common entry for process and thread > > helpers: linux: creating functions to handle odpthreads > > helper: test: adding odpthread functions tests > > validation: using implementation agnostic function for ODP threads > > validation: most tests: adding command line argument parsing > > validation: init: adding command line argument parsing > > validation: pktio: adding command line argument parsing > > helper: adding a function to merge getopt parameters > > helper: parsing the complete set of options > > performance: odp_scheduling: proc mode done by helper > > performance: odp_pktio_perf: using agnostic function for ODP > threads > > performance: odp_pktio_perf: adding helper cmd line parsing > > performance: odp_l2fwd: using agnostic function for ODP threads > > performance: odp_l2fwd: adding helper cmd line parsing > > performance: crypto: using agnostic function for ODP threads > > performance: crypto: adding helper cmd line parsing > > example: classifier: using agnostic function for ODP threads > > example: classifier: adding helper cmd line parsing > > example: generator: using agnostic function for ODP threads > > example: generator: adding helper cmd line parsing > > example: ipsec: using agnostic function for ODP threads > > example: ipsec: adding helper cmd line parsing > > example: l2fwd_simple: using agnostic function for ODP threads > > example: l2fwd_simple: adding helper cmd line parsing > > example: pktio: using agnostic function for ODP threads > > example: pktio: adding helper cmd line parsing > > example: time: using agnostic function for ODP threads > > example: time: adding helper cmd line parsing > > example: timer: using agnostic function for ODP threads > > example: timer: adding helper cmd line parsing > > example: switch: using agnostic function for ODP threads > > example: switch: adding helper cmd line parsing > > helper: removing dead code > > > > example/classifier/odp_classifier.c | 39 +- > > example/generator/odp_generator.c | 45 ++- > > example/ipsec/odp_ipsec.c | 27 +- > > example/l2fwd_simple/odp_l2fwd_simple.c | 47 ++- > > example/packet/odp_pktio.c | 45 ++- > > example/switch/odp_switch.c | 26 +- > > example/time/time_global_test.c | 17 +- > > example/timer/odp_timer_test.c | 26 +- > > helper/include/odp/helper/linux.h | 181 +++++---- > > helper/linux.c | 433 > ++++++++++++++------- > > helper/test/.gitignore | 3 +- > > helper/test/Makefile.am | 14 +- > > helper/test/{thread.c => odpthreads.c} | 33 +- > > helper/test/odpthreads_as_mixed | 15 + > > helper/test/odpthreads_as_processes | 15 + > > helper/test/odpthreads_as_pthreads | 15 + > > helper/test/process.c | 92 ----- > > platform/linux-generic/test/pktio/pktio_run | 21 +- > > platform/linux-generic/test/pktio/pktio_run_dpdk | 17 +- > > platform/linux-generic/test/pktio/pktio_run_netmap | 5 +- > > platform/linux-generic/test/pktio/pktio_run_pcap | 5 +- > > platform/linux-generic/test/pktio/pktio_run_tap | 6 +- > > test/performance/odp_crypto.c | 24 +- > > test/performance/odp_l2fwd.c | 37 +- > > test/performance/odp_pktio_perf.c | 34 +- > > test/performance/odp_scheduling.c | 91 ++--- > > test/validation/atomic/atomic.c | 40 +- > > test/validation/atomic/atomic.h | 2 +- > > test/validation/atomic/atomic_main.c | 4 +- > > test/validation/barrier/barrier.c | 14 +- > > test/validation/barrier/barrier.h | 2 +- > > test/validation/barrier/barrier_main.c | 4 +- > > test/validation/buffer/buffer.c | 10 +- > > test/validation/buffer/buffer.h | 2 +- > > test/validation/buffer/buffer_main.c | 4 +- > > test/validation/classification/classification.c | 10 +- > > test/validation/classification/classification.h | 2 +- > > .../classification/classification_main.c | 4 +- > > test/validation/common/odp_cunit_common.c | 24 +- > > test/validation/common/odp_cunit_common.h | 6 +- > > test/validation/config/config.c | 10 +- > > test/validation/config/config.h | 2 +- > > test/validation/config/config_main.c | 4 +- > > test/validation/cpumask/cpumask.c | 10 +- > > test/validation/cpumask/cpumask.h | 2 +- > > test/validation/cpumask/cpumask_main.c | 4 +- > > test/validation/crypto/crypto.c | 6 +- > > test/validation/crypto/crypto.h | 2 +- > > test/validation/crypto/crypto_main.c | 4 +- > > test/validation/errno/errno.c | 10 +- > > test/validation/errno/errno.h | 2 +- > > test/validation/errno/errno_main.c | 4 +- > > test/validation/hash/hash.c | 10 +- > > test/validation/hash/hash.h | 2 +- > > test/validation/hash/hash_main.c | 4 +- > > test/validation/init/init.c | 18 +- > > test/validation/init/init.h | 6 +- > > test/validation/init/init_main_abort.c | 4 +- > > test/validation/init/init_main_log.c | 4 +- > > test/validation/init/init_main_ok.c | 4 +- > > test/validation/lock/lock.c | 50 +-- > > test/validation/lock/lock.h | 2 +- > > test/validation/lock/lock_main.c | 4 +- > > test/validation/packet/packet.c | 10 +- > > test/validation/packet/packet.h | 2 +- > > test/validation/packet/packet_main.c | 4 +- > > test/validation/pktio/pktio.c | 10 +- > > test/validation/pktio/pktio.h | 2 +- > > test/validation/pktio/pktio_main.c | 4 +- > > test/validation/pool/pool.c | 10 +- > > test/validation/pool/pool.h | 2 +- > > test/validation/pool/pool_main.c | 4 +- > > test/validation/queue/queue.c | 10 +- > > test/validation/queue/queue.h | 2 +- > > test/validation/queue/queue_main.c | 4 +- > > test/validation/random/random.c | 10 +- > > test/validation/random/random.h | 2 +- > > test/validation/random/random_main.c | 4 +- > > test/validation/scheduler/scheduler.c | 18 +- > > test/validation/scheduler/scheduler.h | 2 +- > > test/validation/scheduler/scheduler_main.c | 4 +- > > test/validation/shmem/shmem.c | 16 +- > > test/validation/shmem/shmem.h | 2 +- > > test/validation/shmem/shmem_main.c | 4 +- > > test/validation/std_clib/std_clib.c | 10 +- > > test/validation/std_clib/std_clib.h | 2 +- > > test/validation/std_clib/std_clib_main.c | 4 +- > > test/validation/system/system.c | 10 +- > > test/validation/system/system.h | 2 +- > > test/validation/system/system_main.c | 4 +- > > test/validation/thread/thread.c | 14 +- > > test/validation/thread/thread.h | 2 +- > > test/validation/thread/thread_main.c | 4 +- > > test/validation/time/time.c | 10 +- > > test/validation/time/time.h | 2 +- > > test/validation/time/time_main.c | 4 +- > > test/validation/timer/timer.c | 10 +- > > test/validation/timer/timer.h | 2 +- > > test/validation/timer/timer_main.c | 4 +- > > test/validation/traffic_mngr/traffic_mngr.c | 6 +- > > test/validation/traffic_mngr/traffic_mngr.h | 2 +- > > test/validation/traffic_mngr/traffic_mngr_main.c | 4 +- > > 102 files changed, 1113 insertions(+), 718 deletions(-) > > rename helper/test/{thread.c => odpthreads.c} (71%) > > create mode 100755 helper/test/odpthreads_as_mixed > > create mode 100755 helper/test/odpthreads_as_processes > > create mode 100755 helper/test/odpthreads_as_pthreads > > delete mode 100644 helper/test/process.c >