Message ID | 1462984942-53326-1-git-send-email-christophe.milard@linaro.org |
---|---|
State | New |
Headers | show |
Reviewed-by: Brian Brooks <brian.brooks@linaro.org> for the series. On 05/11 18:41:44, Christophe Milard wrote: > Pull can be performed from the URL at the end of this cover letter > All patches sent to the list following Maxim's request. `git merge --squash remote_branch' may be helpful to those reviewing pull requests from remotes. I support this distributed workflow over flooding the list for changes which morph the code like this series does. > Since v5 > -rebased > -patch removing old unused helper code dropped. (Petri) > -Doc update to discourage usage of the old functions. (Petri, Mike) Since this code is not officially part of the ODP API, it may be silly to mark deprecated, but an __attribute__((deprecated)) wouldn't hurt. Is there an existing policy around deprecated behavior? E.g. one ODP release's worth of notice for apps to adapt accordingly. > 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! > > > [GIT PULL ODPv6] running things in process mode: > > The following changes since commit 24fdbb5060e8702a808a8e2e674521df50e8e830: > > doc: userguide: trim images for better fit (2016-05-10 06:34:34 -0400) > > are available in the git repository at: > > https://git.linaro.org/people/christophe.milard/odp.git test_in_process_mode_v6 > > for you to fetch changes up to f4f4e9e4a1d4304902b50b0d08719c4805ac0a3e: > > doc: implementers-guide: adding helper recommendations (2016-05-11 18:10:54 +0200) > > ---------------------------------------------------------------- > Christophe Milard (38): > 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 > linux-generic: test: ringtest: using agnostic function for ODP threads > linux-generic: test: ringtest: adding helper cmd line parsing > doc: implementers-guide: adding helper recommendations > > doc/implementers-guide/implementers-guide.adoc | 32 ++ > 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 | 136 +++++++- > helper/linux.c | 385 +++++++++++++++++++++ > helper/test/.gitignore | 1 + > helper/test/Makefile.am | 8 +- > helper/test/odpthreads.c | 100 ++++++ > helper/test/odpthreads_as_mixed | 15 + > helper/test/odpthreads_as_processes | 15 + > helper/test/odpthreads_as_pthreads | 15 + > 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 +- > platform/linux-generic/test/ring/ringtest.c | 24 +- > 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/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 +- > 100 files changed, 1336 insertions(+), 409 deletions(-) > create mode 100644 helper/test/odpthreads.c > create mode 100755 helper/test/odpthreads_as_mixed > create mode 100755 helper/test/odpthreads_as_processes > create mode 100755 helper/test/odpthreads_as_pthreads > -- > 2.5.0 >
On 4 May 2016 at 20:38, Brian Brooks <brian.brooks@linaro.org> wrote: > Reviewed-by: Brian Brooks <brian.brooks@linaro.org> > > for the series. > > On 05/11 18:41:44, Christophe Milard wrote: > > Pull can be performed from the URL at the end of this cover letter > > All patches sent to the list following Maxim's request. > > `git merge --squash remote_branch' may be helpful to those reviewing > pull requests from remotes. > > I support this distributed workflow over flooding the list for changes > which morph the code like this series does. > > > Since v5 > > -rebased > > -patch removing old unused helper code dropped. (Petri) > > -Doc update to discourage usage of the old functions. (Petri, Mike) > > Since this code is not officially part of the ODP API, it may be silly > to mark deprecated, but an __attribute__((deprecated)) wouldn't hurt. > > Is there an existing policy around deprecated behavior? E.g. one ODP > release's worth of notice for apps to adapt accordingly. > yes, there is something already http://docs.opendataplane.org/snapshots/master/linux-generic/output/release-guide.html#_deprecating_part_of_the_api > > > 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! > > > > > > [GIT PULL ODPv6] running things in process mode: > > > > The following changes since commit > 24fdbb5060e8702a808a8e2e674521df50e8e830: > > > > doc: userguide: trim images for better fit (2016-05-10 06:34:34 -0400) > > > > are available in the git repository at: > > > > https://git.linaro.org/people/christophe.milard/odp.git > test_in_process_mode_v6 > > > > for you to fetch changes up to f4f4e9e4a1d4304902b50b0d08719c4805ac0a3e: > > > > doc: implementers-guide: adding helper recommendations (2016-05-11 > 18:10:54 +0200) > > > > ---------------------------------------------------------------- > > Christophe Milard (38): > > 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 > > linux-generic: test: ringtest: using agnostic function for ODP > threads > > linux-generic: test: ringtest: adding helper cmd line parsing > > doc: implementers-guide: adding helper recommendations > > > > doc/implementers-guide/implementers-guide.adoc | 32 ++ > > 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 | 136 +++++++- > > helper/linux.c | 385 > +++++++++++++++++++++ > > helper/test/.gitignore | 1 + > > helper/test/Makefile.am | 8 +- > > helper/test/odpthreads.c | 100 ++++++ > > helper/test/odpthreads_as_mixed | 15 + > > helper/test/odpthreads_as_processes | 15 + > > helper/test/odpthreads_as_pthreads | 15 + > > 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 +- > > platform/linux-generic/test/ring/ringtest.c | 24 +- > > 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/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 +- > > 100 files changed, 1336 insertions(+), 409 deletions(-) > > create mode 100644 helper/test/odpthreads.c > > create mode 100755 helper/test/odpthreads_as_mixed > > create mode 100755 helper/test/odpthreads_as_processes > > create mode 100755 helper/test/odpthreads_as_pthreads > > -- > > 2.5.0 > > > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"