| Message ID | 1409681155-49091-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
On 09/02/2014 10:05 PM, Mike Holmes wrote: > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > paramiters -> parameters
> -----Original Message----- > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > Sent: Tuesday, September 02, 2014 9:06 PM > To: lng-odp@lists.linaro.org > Subject: [lng-odp] [PATCH v5] Add-global_init-paramiters > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > > v2: > Factor out odp_platform_init so that odp_global_init is only defined > for linux_generic and resued. > > v3: > Use -M to simplify the diff > Fix missing doxygen comments > > v4: > Improve the documentation. > > v5: > rename typedef struct names > > This touches all platforms to get crypto working with generic init, DPDK > should > be using generic crypto but obviously KS2 will replace this. > A platform may replace global init entirely but this patch starts out > assuming > that the sub module inits can be called from a central global_init as a > starting > point. > > example/generator/odp_generator.c | 2 +- > example/l2fwd/odp_l2fwd.c | 2 +- > example/odp_example/odp_example.c | 2 +- > example/packet/odp_pktio.c | 2 +- > example/timer/odp_timer_test.c | 2 +- > platform/linux-dpdk/Makefile.am | 4 +- > platform/linux-dpdk/{odp_init.c => odp_platform.c} | 66 +---------------- > -- > platform/linux-generic/Makefile.am | 1 + > platform/linux-generic/include/api/odp.h | 1 + > platform/linux-generic/include/api/odp_init.h | 34 +++++++--- > platform/linux-generic/include/odp_internal.h | 3 + > platform/linux-generic/odp_init.c | 9 ++- > platform/linux-generic/odp_platform.c | 14 ++++ > platform/linux-keystone2/Makefile.am | 4 +- > .../linux-keystone2/{odp_init.c => odp_platform.c} | 74 ++--------------- > ----- > test/api_test/odp_common.c | 2 +- > 16 files changed, 73 insertions(+), 149 deletions(-) > rename platform/linux-dpdk/{odp_init.c => odp_platform.c} (50%) > create mode 100644 platform/linux-generic/odp_platform.c > rename platform/linux-keystone2/{odp_init.c => odp_platform.c} (72%) > > diff --git a/example/generator/odp_generator.c > b/example/generator/odp_generator.c > index 9fa9b37..ba7aa68 100644 > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -525,7 +525,7 @@ int main(int argc, char *argv[]) > int core_count; > > /* Init ODP before calling anything else */ > - if (odp_init_global()) { > + if (odp_init_global(NULL, NULL)) { > ODP_ERR("Error: ODP global init failed.\n"); > exit(EXIT_FAILURE); > } > diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c > index d74449a..f8d6729 100644 > --- a/example/l2fwd/odp_l2fwd.c > +++ b/example/l2fwd/odp_l2fwd.c > @@ -323,7 +323,7 @@ int main(int argc, char *argv[]) > odp_pktio_t pktio; > > /* Init ODP before calling anything else */ > - if (odp_init_global()) { > + if (odp_init_global(NULL, NULL)) { > ODP_ERR("Error: ODP global init failed.\n"); > exit(EXIT_FAILURE); > } > diff --git a/example/odp_example/odp_example.c > b/example/odp_example/odp_example.c > index f0bdf29..3d38ac7 100644 > --- a/example/odp_example/odp_example.c > +++ b/example/odp_example/odp_example.c > @@ -951,7 +951,7 @@ int main(int argc, char *argv[]) > > memset(thread_tbl, 0, sizeof(thread_tbl)); > > - if (odp_init_global()) { > + if (odp_init_global(NULL, NULL)) { > printf("ODP global init failed.\n"); > return -1; > } > diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c > index f247bd0..f3543a0 100644 > --- a/example/packet/odp_pktio.c > +++ b/example/packet/odp_pktio.c > @@ -311,7 +311,7 @@ int main(int argc, char *argv[]) > int core_count; > > /* Init ODP before calling anything else */ > - if (odp_init_global()) { > + if (odp_init_global(NULL, NULL)) { > ODP_ERR("Error: ODP global init failed.\n"); > exit(EXIT_FAILURE); > } > diff --git a/example/timer/odp_timer_test.c > b/example/timer/odp_timer_test.c > index bf1d7df..abf90aa 100644 > --- a/example/timer/odp_timer_test.c > +++ b/example/timer/odp_timer_test.c > @@ -260,7 +260,7 @@ int main(int argc, char *argv[]) > > memset(thread_tbl, 0, sizeof(thread_tbl)); > > - if (odp_init_global()) { > + if (odp_init_global(NULL, NULL)) { > printf("ODP global init failed.\n"); > return -1; > } > diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux- > dpdk/Makefile.am > index ff49b7d..48fc9d7 100644 > --- a/platform/linux-dpdk/Makefile.am > +++ b/platform/linux-dpdk/Makefile.am > @@ -67,13 +67,15 @@ __LIB__libodp_la_SOURCES = \ > odp_buffer.c \ > odp_buffer_pool.c \ > ../linux-generic/odp_coremask.c \ > - odp_init.c \ > + ../linux-generic/odp_init.c \ > odp_linux.c \ > odp_packet.c \ > odp_packet_dpdk.c \ > ../linux-generic/odp_packet_flags.c \ > odp_packet_io.c \ > ../linux-generic/odp_packet_socket.c \ > + ../linux-generic/odp_crypto.c \ > + odp_platform.c \ > odp_queue.c \ > ../linux-generic/odp_ring.c \ > ../linux-generic/odp_rwlock.c \ > diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux- > dpdk/odp_platform.c > similarity index 50% > rename from platform/linux-dpdk/odp_init.c > rename to platform/linux-dpdk/odp_platform.c > index ecc2066..3162c05 100644 > --- a/platform/linux-dpdk/odp_init.c > +++ b/platform/linux-dpdk/odp_platform.c > @@ -1,4 +1,4 @@ > -/* Copyright (c) 2013, Linaro Limited > +/* Copyright (c) 2014, Linaro Limited > * All rights reserved. > * > * SPDX-License-Identifier: BSD-3-Clause > @@ -9,7 +9,7 @@ > #include <odp_debug.h> > #include <odp_packet_dpdk.h> > > -int odp_init_dpdk(void) > +int odp_init_platform(odp_global_platform_init_t *platform_params > ODP_UNUSED) > { > int test_argc = 5; > char *test_argv[6]; > @@ -49,65 +49,3 @@ int odp_init_dpdk(void) > > return 0; > } > - > -int odp_init_global(void) > -{ > - odp_thread_init_global(); > - > - odp_system_info_init(); > - > - if (odp_init_dpdk()) { > - ODP_ERR("ODP dpdk init failed.\n"); > - return -1; > - } > - > - if (odp_shm_init_global()) { > - ODP_ERR("ODP shm init failed.\n"); > - return -1; > - } > - > - if (odp_buffer_pool_init_global()) { > - ODP_ERR("ODP buffer pool init failed.\n"); > - return -1; > - } > - > - if (odp_queue_init_global()) { > - ODP_ERR("ODP queue init failed.\n"); > - return -1; > - } > - > - if (odp_schedule_init_global()) { > - ODP_ERR("ODP schedule init failed.\n"); > - return -1; > - } > - > - if (odp_pktio_init_global()) { > - ODP_ERR("ODP packet io init failed.\n"); > - return -1; > - } > - > - if (odp_timer_init_global()) { > - ODP_ERR("ODP timer init failed.\n"); > - return -1; > - } > - > - return 0; > -} > - > - > -int odp_init_local(int thr_id) > -{ > - odp_thread_init_local(thr_id); > - > - if (odp_pktio_init_local()) { > - ODP_ERR("ODP packet io local init failed.\n"); > - return -1; > - } > - > - if (odp_schedule_init_local()) { > - ODP_ERR("ODP schedule local init failed.\n"); > - return -1; > - } > - > - return 0; > -} > diff --git a/platform/linux-generic/Makefile.am b/platform/linux- > generic/Makefile.am > index f4dfdc1..c4aae9d 100644 > --- a/platform/linux-generic/Makefile.am > +++ b/platform/linux-generic/Makefile.am > @@ -62,6 +62,7 @@ __LIB__libodp_la_SOURCES = \ > odp_packet_flags.c \ > odp_packet_io.c \ > odp_packet_socket.c \ > + odp_platform.c \ > odp_queue.c \ > odp_ring.c \ > odp_rwlock.c \ > diff --git a/platform/linux-generic/include/api/odp.h b/platform/linux- > generic/include/api/odp.h > index 0ee3faf..99e2ae0 100644 > --- a/platform/linux-generic/include/api/odp.h > +++ b/platform/linux-generic/include/api/odp.h > @@ -48,6 +48,7 @@ extern "C" { > #include <odp_packet.h> > #include <odp_packet_flags.h> > #include <odp_packet_io.h> > +#include <odp_crypto.h> > > #ifdef __cplusplus > } > diff --git a/platform/linux-generic/include/api/odp_init.h > b/platform/linux-generic/include/api/odp_init.h > index 490324a..8cdce46 100644 > --- a/platform/linux-generic/include/api/odp_init.h > +++ b/platform/linux-generic/include/api/odp_init.h > @@ -22,24 +22,42 @@ extern "C" { > > #include <odp_std_types.h> > > - > - > +/** ODP initialisation data. > + * Data that is required to initialize the ODP API with the > + * application specific data such as specifying a logging callback, the > log > + * level etc. > + */ > +typedef struct odp_global_init_t { > +} odp_global_init_t; > + > +/** ODP platform initialization data. > + * @note ODP API does nothing with this data. It is the underlying > + * implementation requires it and any data passed here is not portable. > + * It is required that the application takes care of identifying and > + * passing any required platform specific data. > + */ > +typedef struct odp_global_platform_init_t { > +} odp_global_platform_init_t; > > /** > - * Perform global ODP initalisation. > - * > - * This function must be called once before calling > - * any other ODP API functions. > + * Perform global ODP initialisation. > * > + * This function must be called once before calling any other ODP API > + * functions. > + * @param[in] params Those parameters that are interpreted by the ODP API > + * @param[in] platform_params Those parameters that are passed without > + * interpretation by the ODP API to the implementation. > * @return 0 if successful > */ > -int odp_init_global(void); > +int odp_init_global(odp_global_init_t *params, > + odp_global_platform_init_t *platform_params); > > > /** > - * Perform thread local ODP initalisation. > + * Perform thread local ODP initialisation. > * > * All threads must call this function before calling > + * Extra line feed. > * any other ODP API functions. > * @param thr_id Thread id > * @return 0 if successful > diff --git a/platform/linux-generic/include/odp_internal.h > b/platform/linux-generic/include/odp_internal.h > index aa79493..6c71fc9 100644 > --- a/platform/linux-generic/include/odp_internal.h > +++ b/platform/linux-generic/include/odp_internal.h > @@ -4,6 +4,7 @@ > * SPDX-License-Identifier: BSD-3-Clause > */ > > +#include <odp_init.h> > > /** > * @file > @@ -42,6 +43,8 @@ int odp_schedule_init_local(void); > int odp_timer_init_global(void); > int odp_timer_disarm_all(void); > > +int odp_init_platform(odp_global_platform_init_t *platform_params); > + > #ifdef __cplusplus > } > #endif > diff --git a/platform/linux-generic/odp_init.c b/platform/linux- > generic/odp_init.c > index 5b7e192..f595def 100644 > --- a/platform/linux-generic/odp_init.c > +++ b/platform/linux-generic/odp_init.c > @@ -8,13 +8,18 @@ > #include <odp_internal.h> > #include <odp_debug.h> > > - > -int odp_init_global(void) > +int odp_init_global(odp_global_init_t *params ODP_UNUSED, > + odp_global_platform_init_t *platform_params ODP_UNUSED) > { > odp_thread_init_global(); > > odp_system_info_init(); > > + if (odp_init_platform(platform_params)) { platform_params is used here -> remove ODP_UNUSED ...OR actually you can remove odp_init_platform() since it's doing nothing. For the same reason, there are no calls to e.g. odp_barrier_init(), odp_coremask_init(), odp_spinlock_init() either - those do not need global initialization (in linux-generic implementation). Also if some linux-generic platform init data would be added to odp_global_platform_init_t, it would be consumed by individual init functions e.g. odp_schedule_init_global(platform_params), etc. This is the platform specific init ... there's not need for odp_init_platform(). -Petri > + ODP_ERR("ODP platform init failed.\n"); > + return -1; > + } > + > if (odp_shm_init_global()) { > ODP_ERR("ODP shm init failed.\n"); > return -1; > diff --git a/platform/linux-generic/odp_platform.c b/platform/linux- > generic/odp_platform.c > new file mode 100644 > index 0000000..a9efa7f > --- /dev/null > +++ b/platform/linux-generic/odp_platform.c > @@ -0,0 +1,14 @@ > +/* Copyright (c) 2014, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +#include <odp_init.h> > +#include <odp_internal.h> > +#include <odp_debug.h> > + > +int odp_init_platform(odp_global_platform_init_t *platform_params > ODP_UNUSED) > +{ > + return 0; > +} > http://lists.linaro.org/mailman/listinfo/lng-odp
On 3 September 2014 15:48, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com> wrote: > > >> -----Original Message----- >> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >> bounces@lists.linaro.org] On Behalf Of ext Mike Holmes >> Sent: Tuesday, September 02, 2014 9:06 PM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCH v5] Add-global_init-paramiters >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> --- >> >> v2: >> Factor out odp_platform_init so that odp_global_init is only defined >> for linux_generic and resued. >> >> v3: >> Use -M to simplify the diff >> Fix missing doxygen comments >> >> v4: >> Improve the documentation. >> >> v5: >> rename typedef struct names >> >> This touches all platforms to get crypto working with generic init, DPDK >> should >> be using generic crypto but obviously KS2 will replace this. >> A platform may replace global init entirely but this patch starts out >> assuming >> that the sub module inits can be called from a central global_init as a >> starting >> point. >> >> example/generator/odp_generator.c | 2 +- >> example/l2fwd/odp_l2fwd.c | 2 +- >> example/odp_example/odp_example.c | 2 +- >> example/packet/odp_pktio.c | 2 +- >> example/timer/odp_timer_test.c | 2 +- >> platform/linux-dpdk/Makefile.am | 4 +- >> platform/linux-dpdk/{odp_init.c => odp_platform.c} | 66 +---------------- >> -- >> platform/linux-generic/Makefile.am | 1 + >> platform/linux-generic/include/api/odp.h | 1 + >> platform/linux-generic/include/api/odp_init.h | 34 +++++++--- >> platform/linux-generic/include/odp_internal.h | 3 + >> platform/linux-generic/odp_init.c | 9 ++- >> platform/linux-generic/odp_platform.c | 14 ++++ >> platform/linux-keystone2/Makefile.am | 4 +- >> .../linux-keystone2/{odp_init.c => odp_platform.c} | 74 ++--------------- >> ----- >> test/api_test/odp_common.c | 2 +- >> 16 files changed, 73 insertions(+), 149 deletions(-) >> rename platform/linux-dpdk/{odp_init.c => odp_platform.c} (50%) >> create mode 100644 platform/linux-generic/odp_platform.c >> rename platform/linux-keystone2/{odp_init.c => odp_platform.c} (72%) >> >> diff --git a/example/generator/odp_generator.c >> b/example/generator/odp_generator.c >> index 9fa9b37..ba7aa68 100644 >> --- a/example/generator/odp_generator.c >> +++ b/example/generator/odp_generator.c >> @@ -525,7 +525,7 @@ int main(int argc, char *argv[]) >> int core_count; >> >> /* Init ODP before calling anything else */ >> - if (odp_init_global()) { >> + if (odp_init_global(NULL, NULL)) { >> ODP_ERR("Error: ODP global init failed.\n"); >> exit(EXIT_FAILURE); >> } >> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c >> index d74449a..f8d6729 100644 >> --- a/example/l2fwd/odp_l2fwd.c >> +++ b/example/l2fwd/odp_l2fwd.c >> @@ -323,7 +323,7 @@ int main(int argc, char *argv[]) >> odp_pktio_t pktio; >> >> /* Init ODP before calling anything else */ >> - if (odp_init_global()) { >> + if (odp_init_global(NULL, NULL)) { >> ODP_ERR("Error: ODP global init failed.\n"); >> exit(EXIT_FAILURE); >> } >> diff --git a/example/odp_example/odp_example.c >> b/example/odp_example/odp_example.c >> index f0bdf29..3d38ac7 100644 >> --- a/example/odp_example/odp_example.c >> +++ b/example/odp_example/odp_example.c >> @@ -951,7 +951,7 @@ int main(int argc, char *argv[]) >> >> memset(thread_tbl, 0, sizeof(thread_tbl)); >> >> - if (odp_init_global()) { >> + if (odp_init_global(NULL, NULL)) { >> printf("ODP global init failed.\n"); >> return -1; >> } >> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c >> index f247bd0..f3543a0 100644 >> --- a/example/packet/odp_pktio.c >> +++ b/example/packet/odp_pktio.c >> @@ -311,7 +311,7 @@ int main(int argc, char *argv[]) >> int core_count; >> >> /* Init ODP before calling anything else */ >> - if (odp_init_global()) { >> + if (odp_init_global(NULL, NULL)) { >> ODP_ERR("Error: ODP global init failed.\n"); >> exit(EXIT_FAILURE); >> } >> diff --git a/example/timer/odp_timer_test.c >> b/example/timer/odp_timer_test.c >> index bf1d7df..abf90aa 100644 >> --- a/example/timer/odp_timer_test.c >> +++ b/example/timer/odp_timer_test.c >> @@ -260,7 +260,7 @@ int main(int argc, char *argv[]) >> >> memset(thread_tbl, 0, sizeof(thread_tbl)); >> >> - if (odp_init_global()) { >> + if (odp_init_global(NULL, NULL)) { >> printf("ODP global init failed.\n"); >> return -1; >> } >> diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux- >> dpdk/Makefile.am >> index ff49b7d..48fc9d7 100644 >> --- a/platform/linux-dpdk/Makefile.am >> +++ b/platform/linux-dpdk/Makefile.am >> @@ -67,13 +67,15 @@ __LIB__libodp_la_SOURCES = \ >> odp_buffer.c \ >> odp_buffer_pool.c \ >> ../linux-generic/odp_coremask.c \ >> - odp_init.c \ >> + ../linux-generic/odp_init.c \ >> odp_linux.c \ >> odp_packet.c \ >> odp_packet_dpdk.c \ >> ../linux-generic/odp_packet_flags.c \ >> odp_packet_io.c \ >> ../linux-generic/odp_packet_socket.c \ >> + ../linux-generic/odp_crypto.c \ >> + odp_platform.c \ >> odp_queue.c \ >> ../linux-generic/odp_ring.c \ >> ../linux-generic/odp_rwlock.c \ >> diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux- >> dpdk/odp_platform.c >> similarity index 50% >> rename from platform/linux-dpdk/odp_init.c >> rename to platform/linux-dpdk/odp_platform.c >> index ecc2066..3162c05 100644 >> --- a/platform/linux-dpdk/odp_init.c >> +++ b/platform/linux-dpdk/odp_platform.c >> @@ -1,4 +1,4 @@ >> -/* Copyright (c) 2013, Linaro Limited >> +/* Copyright (c) 2014, Linaro Limited >> * All rights reserved. >> * >> * SPDX-License-Identifier: BSD-3-Clause >> @@ -9,7 +9,7 @@ >> #include <odp_debug.h> >> #include <odp_packet_dpdk.h> >> >> -int odp_init_dpdk(void) >> +int odp_init_platform(odp_global_platform_init_t *platform_params >> ODP_UNUSED) >> { >> int test_argc = 5; >> char *test_argv[6]; >> @@ -49,65 +49,3 @@ int odp_init_dpdk(void) >> >> return 0; >> } >> - >> -int odp_init_global(void) >> -{ >> - odp_thread_init_global(); >> - >> - odp_system_info_init(); >> - >> - if (odp_init_dpdk()) { >> - ODP_ERR("ODP dpdk init failed.\n"); >> - return -1; >> - } >> - >> - if (odp_shm_init_global()) { >> - ODP_ERR("ODP shm init failed.\n"); >> - return -1; >> - } >> - >> - if (odp_buffer_pool_init_global()) { >> - ODP_ERR("ODP buffer pool init failed.\n"); >> - return -1; >> - } >> - >> - if (odp_queue_init_global()) { >> - ODP_ERR("ODP queue init failed.\n"); >> - return -1; >> - } >> - >> - if (odp_schedule_init_global()) { >> - ODP_ERR("ODP schedule init failed.\n"); >> - return -1; >> - } >> - >> - if (odp_pktio_init_global()) { >> - ODP_ERR("ODP packet io init failed.\n"); >> - return -1; >> - } >> - >> - if (odp_timer_init_global()) { >> - ODP_ERR("ODP timer init failed.\n"); >> - return -1; >> - } >> - >> - return 0; >> -} >> - >> - >> -int odp_init_local(int thr_id) >> -{ >> - odp_thread_init_local(thr_id); >> - >> - if (odp_pktio_init_local()) { >> - ODP_ERR("ODP packet io local init failed.\n"); >> - return -1; >> - } >> - >> - if (odp_schedule_init_local()) { >> - ODP_ERR("ODP schedule local init failed.\n"); >> - return -1; >> - } >> - >> - return 0; >> -} >> diff --git a/platform/linux-generic/Makefile.am b/platform/linux- >> generic/Makefile.am >> index f4dfdc1..c4aae9d 100644 >> --- a/platform/linux-generic/Makefile.am >> +++ b/platform/linux-generic/Makefile.am >> @@ -62,6 +62,7 @@ __LIB__libodp_la_SOURCES = \ >> odp_packet_flags.c \ >> odp_packet_io.c \ >> odp_packet_socket.c \ >> + odp_platform.c \ >> odp_queue.c \ >> odp_ring.c \ >> odp_rwlock.c \ >> diff --git a/platform/linux-generic/include/api/odp.h b/platform/linux- >> generic/include/api/odp.h >> index 0ee3faf..99e2ae0 100644 >> --- a/platform/linux-generic/include/api/odp.h >> +++ b/platform/linux-generic/include/api/odp.h >> @@ -48,6 +48,7 @@ extern "C" { >> #include <odp_packet.h> >> #include <odp_packet_flags.h> >> #include <odp_packet_io.h> >> +#include <odp_crypto.h> >> >> #ifdef __cplusplus >> } >> diff --git a/platform/linux-generic/include/api/odp_init.h >> b/platform/linux-generic/include/api/odp_init.h >> index 490324a..8cdce46 100644 >> --- a/platform/linux-generic/include/api/odp_init.h >> +++ b/platform/linux-generic/include/api/odp_init.h >> @@ -22,24 +22,42 @@ extern "C" { >> >> #include <odp_std_types.h> >> >> - >> - >> +/** ODP initialisation data. >> + * Data that is required to initialize the ODP API with the >> + * application specific data such as specifying a logging callback, the >> log >> + * level etc. >> + */ >> +typedef struct odp_global_init_t { >> +} odp_global_init_t; >> + >> +/** ODP platform initialization data. >> + * @note ODP API does nothing with this data. It is the underlying >> + * implementation requires it and any data passed here is not portable. >> + * It is required that the application takes care of identifying and >> + * passing any required platform specific data. >> + */ >> +typedef struct odp_global_platform_init_t { >> +} odp_global_platform_init_t; >> >> /** >> - * Perform global ODP initalisation. >> - * >> - * This function must be called once before calling >> - * any other ODP API functions. >> + * Perform global ODP initialisation. >> * >> + * This function must be called once before calling any other ODP API >> + * functions. >> + * @param[in] params Those parameters that are interpreted by the ODP API >> + * @param[in] platform_params Those parameters that are passed without >> + * interpretation by the ODP API to the implementation. >> * @return 0 if successful >> */ >> -int odp_init_global(void); >> +int odp_init_global(odp_global_init_t *params, >> + odp_global_platform_init_t *platform_params); >> >> >> /** >> - * Perform thread local ODP initalisation. >> + * Perform thread local ODP initialisation. >> * >> * All threads must call this function before calling >> + * > > > Extra line feed. > > >> * any other ODP API functions. >> * @param thr_id Thread id >> * @return 0 if successful >> diff --git a/platform/linux-generic/include/odp_internal.h >> b/platform/linux-generic/include/odp_internal.h >> index aa79493..6c71fc9 100644 >> --- a/platform/linux-generic/include/odp_internal.h >> +++ b/platform/linux-generic/include/odp_internal.h >> @@ -4,6 +4,7 @@ >> * SPDX-License-Identifier: BSD-3-Clause >> */ >> >> +#include <odp_init.h> >> >> /** >> * @file >> @@ -42,6 +43,8 @@ int odp_schedule_init_local(void); >> int odp_timer_init_global(void); >> int odp_timer_disarm_all(void); >> >> +int odp_init_platform(odp_global_platform_init_t *platform_params); >> + >> #ifdef __cplusplus >> } >> #endif >> diff --git a/platform/linux-generic/odp_init.c b/platform/linux- >> generic/odp_init.c >> index 5b7e192..f595def 100644 >> --- a/platform/linux-generic/odp_init.c >> +++ b/platform/linux-generic/odp_init.c >> @@ -8,13 +8,18 @@ >> #include <odp_internal.h> >> #include <odp_debug.h> >> >> - >> -int odp_init_global(void) >> +int odp_init_global(odp_global_init_t *params ODP_UNUSED, >> + odp_global_platform_init_t *platform_params ODP_UNUSED) >> { >> odp_thread_init_global(); >> >> odp_system_info_init(); >> >> + if (odp_init_platform(platform_params)) { > > > platform_params is used here -> remove ODP_UNUSED > > ...OR actually you can remove odp_init_platform() since it's doing nothing. For the same reason, > there are no calls to e.g. odp_barrier_init(), odp_coremask_init(), odp_spinlock_init() either - those do not need global initialization (in linux-generic implementation). Also if some linux-generic platform init data would be added to odp_global_platform_init_t, it would be consumed by individual init functions e.g. odp_schedule_init_global(platform_params), etc. (I thought, I sent this reply to list, But it went only to petri (: - ) Petri, I agree what you pointed out. However, By separating/removing odp_init_platform () from odp_global_init() call will lead to segfault for other platform specific resource init. For example, I tried removing odp_platfrom_init() and call separately from application layer, got this (gdb) bt #0 0x0000000000443e3a in memzone_reserve_aligned_thread_unsafe () #1 0x0000000000444250 in rte_memzone_reserve_aligned () #2 0x000000000044ceff in rte_ring_create () #3 0x000000000042986e in rte_mempool_xmem_create () #4 0x0000000000429c8b in rte_mempool_create () #5 0x0000000000403963 in odp_buffer_pool_create (name=name@entry=0x46bba2 "odp_sched_pool", base_addr=<optimized out>, size=size@entry=262144, buf_size=buf_size@entry=4, buf_align=buf_align@entry=64, buf_type=buf_type@entry=1) at odp_buffer_pool.c:113 #6 0x0000000000406931 in odp_schedule_init_global () at ../linux-generic/odp_schedule.c:103 #7 0x000000000040445b in odp_init_global (params=params@entry=0x0, platform_params=platform_params@entry=0x0) at ../linux-generic/odp_init.c:38 #8 0x0000000000402940 in main (argc=7, argv=0x7fffffffe338) at odp_l2fwd.c:341 My concern : - odp_global_init (arg1, agr2), agr2 : is platform centric, Most likely to receive data from cmdline. cmdline argument processing happens after odp resource init, So in that case arg2 is redundant. IIUC, intent of this patch to provide method by which application can send command from application layer to odp-platform-centric-generric-layer. I believe this patch NOT addressing that requirement. > > This is the platform specific init ... there's not need for odp_init_platform(). > > -Petri > > >> + ODP_ERR("ODP platform init failed.\n"); >> + return -1; >> + } >> + >> if (odp_shm_init_global()) { >> ODP_ERR("ODP shm init failed.\n"); >> return -1; >> diff --git a/platform/linux-generic/odp_platform.c b/platform/linux- >> generic/odp_platform.c >> new file mode 100644 >> index 0000000..a9efa7f >> --- /dev/null >> +++ b/platform/linux-generic/odp_platform.c >> @@ -0,0 +1,14 @@ >> +/* Copyright (c) 2014, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +#include <odp_init.h> >> +#include <odp_internal.h> >> +#include <odp_debug.h> >> + >> +int odp_init_platform(odp_global_platform_init_t *platform_params >> ODP_UNUSED) >> +{ >> + return 0; >> +} >> http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 4 September 2014 10:18, Santosh Shukla <santosh.shukla@linaro.org> wrote: > On 3 September 2014 15:48, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com> wrote: >> >> >>> -----Original Message----- >>> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >>> bounces@lists.linaro.org] On Behalf Of ext Mike Holmes >>> Sent: Tuesday, September 02, 2014 9:06 PM >>> To: lng-odp@lists.linaro.org >>> Subject: [lng-odp] [PATCH v5] Add-global_init-paramiters >>> >>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>> --- >>> >>> v2: >>> Factor out odp_platform_init so that odp_global_init is only defined >>> for linux_generic and resued. >>> >>> v3: >>> Use -M to simplify the diff >>> Fix missing doxygen comments >>> >>> v4: >>> Improve the documentation. >>> >>> v5: >>> rename typedef struct names >>> >>> This touches all platforms to get crypto working with generic init, DPDK >>> should >>> be using generic crypto but obviously KS2 will replace this. >>> A platform may replace global init entirely but this patch starts out >>> assuming >>> that the sub module inits can be called from a central global_init as a >>> starting >>> point. >>> >>> example/generator/odp_generator.c | 2 +- >>> example/l2fwd/odp_l2fwd.c | 2 +- >>> example/odp_example/odp_example.c | 2 +- >>> example/packet/odp_pktio.c | 2 +- >>> example/timer/odp_timer_test.c | 2 +- >>> platform/linux-dpdk/Makefile.am | 4 +- >>> platform/linux-dpdk/{odp_init.c => odp_platform.c} | 66 +---------------- >>> -- >>> platform/linux-generic/Makefile.am | 1 + >>> platform/linux-generic/include/api/odp.h | 1 + >>> platform/linux-generic/include/api/odp_init.h | 34 +++++++--- >>> platform/linux-generic/include/odp_internal.h | 3 + >>> platform/linux-generic/odp_init.c | 9 ++- >>> platform/linux-generic/odp_platform.c | 14 ++++ >>> platform/linux-keystone2/Makefile.am | 4 +- >>> .../linux-keystone2/{odp_init.c => odp_platform.c} | 74 ++--------------- >>> ----- >>> test/api_test/odp_common.c | 2 +- >>> 16 files changed, 73 insertions(+), 149 deletions(-) >>> rename platform/linux-dpdk/{odp_init.c => odp_platform.c} (50%) >>> create mode 100644 platform/linux-generic/odp_platform.c >>> rename platform/linux-keystone2/{odp_init.c => odp_platform.c} (72%) >>> >>> diff --git a/example/generator/odp_generator.c >>> b/example/generator/odp_generator.c >>> index 9fa9b37..ba7aa68 100644 >>> --- a/example/generator/odp_generator.c >>> +++ b/example/generator/odp_generator.c >>> @@ -525,7 +525,7 @@ int main(int argc, char *argv[]) >>> int core_count; >>> >>> /* Init ODP before calling anything else */ >>> - if (odp_init_global()) { >>> + if (odp_init_global(NULL, NULL)) { >>> ODP_ERR("Error: ODP global init failed.\n"); >>> exit(EXIT_FAILURE); >>> } >>> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c >>> index d74449a..f8d6729 100644 >>> --- a/example/l2fwd/odp_l2fwd.c >>> +++ b/example/l2fwd/odp_l2fwd.c >>> @@ -323,7 +323,7 @@ int main(int argc, char *argv[]) >>> odp_pktio_t pktio; >>> >>> /* Init ODP before calling anything else */ >>> - if (odp_init_global()) { >>> + if (odp_init_global(NULL, NULL)) { >>> ODP_ERR("Error: ODP global init failed.\n"); >>> exit(EXIT_FAILURE); >>> } >>> diff --git a/example/odp_example/odp_example.c >>> b/example/odp_example/odp_example.c >>> index f0bdf29..3d38ac7 100644 >>> --- a/example/odp_example/odp_example.c >>> +++ b/example/odp_example/odp_example.c >>> @@ -951,7 +951,7 @@ int main(int argc, char *argv[]) >>> >>> memset(thread_tbl, 0, sizeof(thread_tbl)); >>> >>> - if (odp_init_global()) { >>> + if (odp_init_global(NULL, NULL)) { >>> printf("ODP global init failed.\n"); >>> return -1; >>> } >>> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c >>> index f247bd0..f3543a0 100644 >>> --- a/example/packet/odp_pktio.c >>> +++ b/example/packet/odp_pktio.c >>> @@ -311,7 +311,7 @@ int main(int argc, char *argv[]) >>> int core_count; >>> >>> /* Init ODP before calling anything else */ >>> - if (odp_init_global()) { >>> + if (odp_init_global(NULL, NULL)) { >>> ODP_ERR("Error: ODP global init failed.\n"); >>> exit(EXIT_FAILURE); >>> } >>> diff --git a/example/timer/odp_timer_test.c >>> b/example/timer/odp_timer_test.c >>> index bf1d7df..abf90aa 100644 >>> --- a/example/timer/odp_timer_test.c >>> +++ b/example/timer/odp_timer_test.c >>> @@ -260,7 +260,7 @@ int main(int argc, char *argv[]) >>> >>> memset(thread_tbl, 0, sizeof(thread_tbl)); >>> >>> - if (odp_init_global()) { >>> + if (odp_init_global(NULL, NULL)) { >>> printf("ODP global init failed.\n"); >>> return -1; >>> } >>> diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux- >>> dpdk/Makefile.am >>> index ff49b7d..48fc9d7 100644 >>> --- a/platform/linux-dpdk/Makefile.am >>> +++ b/platform/linux-dpdk/Makefile.am >>> @@ -67,13 +67,15 @@ __LIB__libodp_la_SOURCES = \ >>> odp_buffer.c \ >>> odp_buffer_pool.c \ >>> ../linux-generic/odp_coremask.c \ >>> - odp_init.c \ >>> + ../linux-generic/odp_init.c \ >>> odp_linux.c \ >>> odp_packet.c \ >>> odp_packet_dpdk.c \ >>> ../linux-generic/odp_packet_flags.c \ >>> odp_packet_io.c \ >>> ../linux-generic/odp_packet_socket.c \ >>> + ../linux-generic/odp_crypto.c \ >>> + odp_platform.c \ >>> odp_queue.c \ >>> ../linux-generic/odp_ring.c \ >>> ../linux-generic/odp_rwlock.c \ >>> diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux- >>> dpdk/odp_platform.c >>> similarity index 50% >>> rename from platform/linux-dpdk/odp_init.c >>> rename to platform/linux-dpdk/odp_platform.c >>> index ecc2066..3162c05 100644 >>> --- a/platform/linux-dpdk/odp_init.c >>> +++ b/platform/linux-dpdk/odp_platform.c >>> @@ -1,4 +1,4 @@ >>> -/* Copyright (c) 2013, Linaro Limited >>> +/* Copyright (c) 2014, Linaro Limited >>> * All rights reserved. >>> * >>> * SPDX-License-Identifier: BSD-3-Clause >>> @@ -9,7 +9,7 @@ >>> #include <odp_debug.h> >>> #include <odp_packet_dpdk.h> >>> >>> -int odp_init_dpdk(void) >>> +int odp_init_platform(odp_global_platform_init_t *platform_params >>> ODP_UNUSED) >>> { >>> int test_argc = 5; >>> char *test_argv[6]; >>> @@ -49,65 +49,3 @@ int odp_init_dpdk(void) >>> >>> return 0; >>> } >>> - >>> -int odp_init_global(void) >>> -{ >>> - odp_thread_init_global(); >>> - >>> - odp_system_info_init(); >>> - >>> - if (odp_init_dpdk()) { >>> - ODP_ERR("ODP dpdk init failed.\n"); >>> - return -1; >>> - } >>> - >>> - if (odp_shm_init_global()) { >>> - ODP_ERR("ODP shm init failed.\n"); >>> - return -1; >>> - } >>> - >>> - if (odp_buffer_pool_init_global()) { >>> - ODP_ERR("ODP buffer pool init failed.\n"); >>> - return -1; >>> - } >>> - >>> - if (odp_queue_init_global()) { >>> - ODP_ERR("ODP queue init failed.\n"); >>> - return -1; >>> - } >>> - >>> - if (odp_schedule_init_global()) { >>> - ODP_ERR("ODP schedule init failed.\n"); >>> - return -1; >>> - } >>> - >>> - if (odp_pktio_init_global()) { >>> - ODP_ERR("ODP packet io init failed.\n"); >>> - return -1; >>> - } >>> - >>> - if (odp_timer_init_global()) { >>> - ODP_ERR("ODP timer init failed.\n"); >>> - return -1; >>> - } >>> - >>> - return 0; >>> -} >>> - >>> - >>> -int odp_init_local(int thr_id) >>> -{ >>> - odp_thread_init_local(thr_id); >>> - >>> - if (odp_pktio_init_local()) { >>> - ODP_ERR("ODP packet io local init failed.\n"); >>> - return -1; >>> - } >>> - >>> - if (odp_schedule_init_local()) { >>> - ODP_ERR("ODP schedule local init failed.\n"); >>> - return -1; >>> - } >>> - >>> - return 0; >>> -} >>> diff --git a/platform/linux-generic/Makefile.am b/platform/linux- >>> generic/Makefile.am >>> index f4dfdc1..c4aae9d 100644 >>> --- a/platform/linux-generic/Makefile.am >>> +++ b/platform/linux-generic/Makefile.am >>> @@ -62,6 +62,7 @@ __LIB__libodp_la_SOURCES = \ >>> odp_packet_flags.c \ >>> odp_packet_io.c \ >>> odp_packet_socket.c \ >>> + odp_platform.c \ >>> odp_queue.c \ >>> odp_ring.c \ >>> odp_rwlock.c \ >>> diff --git a/platform/linux-generic/include/api/odp.h b/platform/linux- >>> generic/include/api/odp.h >>> index 0ee3faf..99e2ae0 100644 >>> --- a/platform/linux-generic/include/api/odp.h >>> +++ b/platform/linux-generic/include/api/odp.h >>> @@ -48,6 +48,7 @@ extern "C" { >>> #include <odp_packet.h> >>> #include <odp_packet_flags.h> >>> #include <odp_packet_io.h> >>> +#include <odp_crypto.h> >>> >>> #ifdef __cplusplus >>> } >>> diff --git a/platform/linux-generic/include/api/odp_init.h >>> b/platform/linux-generic/include/api/odp_init.h >>> index 490324a..8cdce46 100644 >>> --- a/platform/linux-generic/include/api/odp_init.h >>> +++ b/platform/linux-generic/include/api/odp_init.h >>> @@ -22,24 +22,42 @@ extern "C" { >>> >>> #include <odp_std_types.h> >>> >>> - >>> - >>> +/** ODP initialisation data. >>> + * Data that is required to initialize the ODP API with the >>> + * application specific data such as specifying a logging callback, the >>> log >>> + * level etc. >>> + */ >>> +typedef struct odp_global_init_t { >>> +} odp_global_init_t; >>> + >>> +/** ODP platform initialization data. >>> + * @note ODP API does nothing with this data. It is the underlying >>> + * implementation requires it and any data passed here is not portable. >>> + * It is required that the application takes care of identifying and >>> + * passing any required platform specific data. >>> + */ >>> +typedef struct odp_global_platform_init_t { >>> +} odp_global_platform_init_t; >>> >>> /** >>> - * Perform global ODP initalisation. >>> - * >>> - * This function must be called once before calling >>> - * any other ODP API functions. >>> + * Perform global ODP initialisation. >>> * >>> + * This function must be called once before calling any other ODP API >>> + * functions. >>> + * @param[in] params Those parameters that are interpreted by the ODP API >>> + * @param[in] platform_params Those parameters that are passed without >>> + * interpretation by the ODP API to the implementation. >>> * @return 0 if successful >>> */ >>> -int odp_init_global(void); >>> +int odp_init_global(odp_global_init_t *params, >>> + odp_global_platform_init_t *platform_params); >>> >>> >>> /** >>> - * Perform thread local ODP initalisation. >>> + * Perform thread local ODP initialisation. >>> * >>> * All threads must call this function before calling >>> + * >> >> >> Extra line feed. >> >> >>> * any other ODP API functions. >>> * @param thr_id Thread id >>> * @return 0 if successful >>> diff --git a/platform/linux-generic/include/odp_internal.h >>> b/platform/linux-generic/include/odp_internal.h >>> index aa79493..6c71fc9 100644 >>> --- a/platform/linux-generic/include/odp_internal.h >>> +++ b/platform/linux-generic/include/odp_internal.h >>> @@ -4,6 +4,7 @@ >>> * SPDX-License-Identifier: BSD-3-Clause >>> */ >>> >>> +#include <odp_init.h> >>> >>> /** >>> * @file >>> @@ -42,6 +43,8 @@ int odp_schedule_init_local(void); >>> int odp_timer_init_global(void); >>> int odp_timer_disarm_all(void); >>> >>> +int odp_init_platform(odp_global_platform_init_t *platform_params); >>> + >>> #ifdef __cplusplus >>> } >>> #endif >>> diff --git a/platform/linux-generic/odp_init.c b/platform/linux- >>> generic/odp_init.c >>> index 5b7e192..f595def 100644 >>> --- a/platform/linux-generic/odp_init.c >>> +++ b/platform/linux-generic/odp_init.c >>> @@ -8,13 +8,18 @@ >>> #include <odp_internal.h> >>> #include <odp_debug.h> >>> >>> - >>> -int odp_init_global(void) >>> +int odp_init_global(odp_global_init_t *params ODP_UNUSED, >>> + odp_global_platform_init_t *platform_params ODP_UNUSED) >>> { >>> odp_thread_init_global(); >>> >>> odp_system_info_init(); >>> >>> + if (odp_init_platform(platform_params)) { >> >> >> platform_params is used here -> remove ODP_UNUSED >> >> ...OR actually you can remove odp_init_platform() since it's doing nothing. For the same reason, >> there are no calls to e.g. odp_barrier_init(), odp_coremask_init(), odp_spinlock_init() either - those do not need global initialization (in linux-generic implementation). Also if some linux-generic platform init data would be added to odp_global_platform_init_t, it would be consumed by individual init functions e.g. odp_schedule_init_global(platform_params), etc. > > (I thought, I sent this reply to list, But it went only to petri (: - ) > > Petri, I agree what you pointed out. > > However, By separating/removing odp_init_platform () from > odp_global_init() call will lead to segfault for other platform > specific resource init. > > For example, I tried removing odp_platfrom_init() and call separately > from application layer, got this > > (gdb) bt > #0 0x0000000000443e3a in memzone_reserve_aligned_thread_unsafe () > #1 0x0000000000444250 in rte_memzone_reserve_aligned () > #2 0x000000000044ceff in rte_ring_create () > #3 0x000000000042986e in rte_mempool_xmem_create () > #4 0x0000000000429c8b in rte_mempool_create () > #5 0x0000000000403963 in odp_buffer_pool_create > (name=name@entry=0x46bba2 "odp_sched_pool", base_addr=<optimized out>, > size=size@entry=262144, > buf_size=buf_size@entry=4, buf_align=buf_align@entry=64, > buf_type=buf_type@entry=1) at odp_buffer_pool.c:113 > #6 0x0000000000406931 in odp_schedule_init_global () at > ../linux-generic/odp_schedule.c:103 > #7 0x000000000040445b in odp_init_global (params=params@entry=0x0, > platform_params=platform_params@entry=0x0) at > ../linux-generic/odp_init.c:38 > #8 0x0000000000402940 in main (argc=7, argv=0x7fffffffe338) at odp_l2fwd.c:341 > > > My concern : > > - odp_global_init (arg1, agr2), agr2 : is platform centric, Most > likely to receive data from cmdline. cmdline argument processing > happens after odp resource init, So in that case arg2 is redundant. > IIUC, intent of this patch to provide method by which application can > send command from application layer to > odp-platform-centric-generric-layer. I believe this patch NOT > addressing that requirement. Conti.. Root cause of my concern is not entirely specific to this patch. Its more related to odp api initialization calling sequence in application. From early days, we used to follow below step in application - odp_global_init() -> which does almost everything for platform initialization. odp_shm_reserve() -> used for argument processing then odp_create_thread etc... Now we found a need to introduce platform specific init hints, coming from application. Therefore order of odp_xxx_init api calling sequence should change too. Something like. Application "X" main () { - First do argument processing. For that, call odp_shm_init() in application and remove it from odp_global_init OR application has to allocate memory(bad idea ... I guess). - Then call odp_global/platform_init(argX, argY); where arg -X : global param, Y : platform centric stuff. ... Rest stuff as it is. } Any idea? thoughts? Does above make sense. Thanks. >> >> This is the platform specific init ... there's not need for odp_init_platform(). >> >> -Petri >> >> >>> + ODP_ERR("ODP platform init failed.\n"); >>> + return -1; >>> + } >>> + >>> if (odp_shm_init_global()) { >>> ODP_ERR("ODP shm init failed.\n"); >>> return -1; >>> diff --git a/platform/linux-generic/odp_platform.c b/platform/linux- >>> generic/odp_platform.c >>> new file mode 100644 >>> index 0000000..a9efa7f >>> --- /dev/null >>> +++ b/platform/linux-generic/odp_platform.c >>> @@ -0,0 +1,14 @@ >>> +/* Copyright (c) 2014, Linaro Limited >>> + * All rights reserved. >>> + * >>> + * SPDX-License-Identifier: BSD-3-Clause >>> + */ >>> + >>> +#include <odp_init.h> >>> +#include <odp_internal.h> >>> +#include <odp_debug.h> >>> + >>> +int odp_init_platform(odp_global_platform_init_t *platform_params >>> ODP_UNUSED) >>> +{ >>> + return 0; >>> +} >>> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
> Conti.. > > Root cause of my concern is not entirely specific to this patch. Its > more related to odp api initialization calling sequence in > application. From early days, we used to follow below step in > application - > > odp_global_init() -> which does almost everything for platform > initialization. > odp_shm_reserve() -> used for argument processing then > odp_create_thread etc... > > Now we found a need to introduce platform specific init hints, coming > from application. Therefore order of odp_xxx_init api calling sequence > should change too. > > > Something like. > > Application "X" > > main () > { > > - First do argument processing. For that, call odp_shm_init() in > application and remove it from odp_global_init OR application has to > allocate memory(bad idea ... I guess). argX and argY can be allocated from stack or heap (malloc). Shared memory is not needed for those to be able to call odp_global_init(). ODP will take copy of the argument, argX/Y can be destroyed after the call. After ODP init, you can allocate shm and copy args there if you need to share those within your app. -Petri > > - Then call odp_global/platform_init(argX, argY); where arg -X : > global param, Y : platform centric stuff. > > ... Rest stuff as it is. > } > > > Any idea? thoughts? Does above make sense. Thanks. > >
On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com> wrote: >> Conti.. >> >> Root cause of my concern is not entirely specific to this patch. Its >> more related to odp api initialization calling sequence in >> application. From early days, we used to follow below step in >> application - >> >> odp_global_init() -> which does almost everything for platform >> initialization. >> odp_shm_reserve() -> used for argument processing then >> odp_create_thread etc... >> >> Now we found a need to introduce platform specific init hints, coming >> from application. Therefore order of odp_xxx_init api calling sequence >> should change too. >> >> >> Something like. >> >> Application "X" >> >> main () >> { >> >> - First do argument processing. For that, call odp_shm_init() in >> application and remove it from odp_global_init OR application has to >> allocate memory(bad idea ... I guess). > > argX and argY can be allocated from stack or heap (malloc). Shared memory is not needed for those to be able to call odp_global_init(). ODP will take copy of the argument, argX/Y can be destroyed after the call. Not to *call odp_global_init* , It is to pass command line processed input to odp_globa_init().. Are you suggesting calling of parse_args() before and after odp_global_init()? > > After ODP init, you can allocate shm and copy args there if you need to share those within your app. Same argument processed twice ? isn't it. > > -Petri > >> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >> global param, Y : platform centric stuff. >> >> ... Rest stuff as it is. >> } >> >> >> Any idea? thoughts? Does above make sense. Thanks. >> >>
On 3 September 2014 06:18, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Tuesday, September 02, 2014 9:06 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH v5] Add-global_init-paramiters > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > > > v2: > > Factor out odp_platform_init so that odp_global_init is only defined > > for linux_generic and resued. > > > > v3: > > Use -M to simplify the diff > > Fix missing doxygen comments > > > > v4: > > Improve the documentation. > > > > v5: > > rename typedef struct names > > > > This touches all platforms to get crypto working with generic init, DPDK > > should > > be using generic crypto but obviously KS2 will replace this. > > A platform may replace global init entirely but this patch starts out > > assuming > > that the sub module inits can be called from a central global_init as a > > starting > > point. > > > > example/generator/odp_generator.c | 2 +- > > example/l2fwd/odp_l2fwd.c | 2 +- > > example/odp_example/odp_example.c | 2 +- > > example/packet/odp_pktio.c | 2 +- > > example/timer/odp_timer_test.c | 2 +- > > platform/linux-dpdk/Makefile.am | 4 +- > > platform/linux-dpdk/{odp_init.c => odp_platform.c} | 66 > +---------------- > > -- > > platform/linux-generic/Makefile.am | 1 + > > platform/linux-generic/include/api/odp.h | 1 + > > platform/linux-generic/include/api/odp_init.h | 34 +++++++--- > > platform/linux-generic/include/odp_internal.h | 3 + > > platform/linux-generic/odp_init.c | 9 ++- > > platform/linux-generic/odp_platform.c | 14 ++++ > > platform/linux-keystone2/Makefile.am | 4 +- > > .../linux-keystone2/{odp_init.c => odp_platform.c} | 74 > ++--------------- > > ----- > > test/api_test/odp_common.c | 2 +- > > 16 files changed, 73 insertions(+), 149 deletions(-) > > rename platform/linux-dpdk/{odp_init.c => odp_platform.c} (50%) > > create mode 100644 platform/linux-generic/odp_platform.c > > rename platform/linux-keystone2/{odp_init.c => odp_platform.c} (72%) > > > > diff --git a/example/generator/odp_generator.c > > b/example/generator/odp_generator.c > > index 9fa9b37..ba7aa68 100644 > > --- a/example/generator/odp_generator.c > > +++ b/example/generator/odp_generator.c > > @@ -525,7 +525,7 @@ int main(int argc, char *argv[]) > > int core_count; > > > > /* Init ODP before calling anything else */ > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > ODP_ERR("Error: ODP global init failed.\n"); > > exit(EXIT_FAILURE); > > } > > diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c > > index d74449a..f8d6729 100644 > > --- a/example/l2fwd/odp_l2fwd.c > > +++ b/example/l2fwd/odp_l2fwd.c > > @@ -323,7 +323,7 @@ int main(int argc, char *argv[]) > > odp_pktio_t pktio; > > > > /* Init ODP before calling anything else */ > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > ODP_ERR("Error: ODP global init failed.\n"); > > exit(EXIT_FAILURE); > > } > > diff --git a/example/odp_example/odp_example.c > > b/example/odp_example/odp_example.c > > index f0bdf29..3d38ac7 100644 > > --- a/example/odp_example/odp_example.c > > +++ b/example/odp_example/odp_example.c > > @@ -951,7 +951,7 @@ int main(int argc, char *argv[]) > > > > memset(thread_tbl, 0, sizeof(thread_tbl)); > > > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > printf("ODP global init failed.\n"); > > return -1; > > } > > diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c > > index f247bd0..f3543a0 100644 > > --- a/example/packet/odp_pktio.c > > +++ b/example/packet/odp_pktio.c > > @@ -311,7 +311,7 @@ int main(int argc, char *argv[]) > > int core_count; > > > > /* Init ODP before calling anything else */ > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > ODP_ERR("Error: ODP global init failed.\n"); > > exit(EXIT_FAILURE); > > } > > diff --git a/example/timer/odp_timer_test.c > > b/example/timer/odp_timer_test.c > > index bf1d7df..abf90aa 100644 > > --- a/example/timer/odp_timer_test.c > > +++ b/example/timer/odp_timer_test.c > > @@ -260,7 +260,7 @@ int main(int argc, char *argv[]) > > > > memset(thread_tbl, 0, sizeof(thread_tbl)); > > > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > printf("ODP global init failed.\n"); > > return -1; > > } > > diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux- > > dpdk/Makefile.am > > index ff49b7d..48fc9d7 100644 > > --- a/platform/linux-dpdk/Makefile.am > > +++ b/platform/linux-dpdk/Makefile.am > > @@ -67,13 +67,15 @@ __LIB__libodp_la_SOURCES = \ > > odp_buffer.c \ > > odp_buffer_pool.c \ > > ../linux-generic/odp_coremask.c \ > > - odp_init.c \ > > + ../linux-generic/odp_init.c \ > > odp_linux.c \ > > odp_packet.c \ > > odp_packet_dpdk.c \ > > ../linux-generic/odp_packet_flags.c \ > > odp_packet_io.c \ > > ../linux-generic/odp_packet_socket.c \ > > + ../linux-generic/odp_crypto.c \ > > + odp_platform.c \ > > odp_queue.c \ > > ../linux-generic/odp_ring.c \ > > ../linux-generic/odp_rwlock.c \ > > diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux- > > dpdk/odp_platform.c > > similarity index 50% > > rename from platform/linux-dpdk/odp_init.c > > rename to platform/linux-dpdk/odp_platform.c > > index ecc2066..3162c05 100644 > > --- a/platform/linux-dpdk/odp_init.c > > +++ b/platform/linux-dpdk/odp_platform.c > > @@ -1,4 +1,4 @@ > > -/* Copyright (c) 2013, Linaro Limited > > +/* Copyright (c) 2014, Linaro Limited > > * All rights reserved. > > * > > * SPDX-License-Identifier: BSD-3-Clause > > @@ -9,7 +9,7 @@ > > #include <odp_debug.h> > > #include <odp_packet_dpdk.h> > > > > -int odp_init_dpdk(void) > > +int odp_init_platform(odp_global_platform_init_t *platform_params > > ODP_UNUSED) > > { > > int test_argc = 5; > > char *test_argv[6]; > > @@ -49,65 +49,3 @@ int odp_init_dpdk(void) > > > > return 0; > > } > > - > > -int odp_init_global(void) > > -{ > > - odp_thread_init_global(); > > - > > - odp_system_info_init(); > > - > > - if (odp_init_dpdk()) { > > - ODP_ERR("ODP dpdk init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_shm_init_global()) { > > - ODP_ERR("ODP shm init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_buffer_pool_init_global()) { > > - ODP_ERR("ODP buffer pool init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_queue_init_global()) { > > - ODP_ERR("ODP queue init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_schedule_init_global()) { > > - ODP_ERR("ODP schedule init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_pktio_init_global()) { > > - ODP_ERR("ODP packet io init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_timer_init_global()) { > > - ODP_ERR("ODP timer init failed.\n"); > > - return -1; > > - } > > - > > - return 0; > > -} > > - > > - > > -int odp_init_local(int thr_id) > > -{ > > - odp_thread_init_local(thr_id); > > - > > - if (odp_pktio_init_local()) { > > - ODP_ERR("ODP packet io local init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_schedule_init_local()) { > > - ODP_ERR("ODP schedule local init failed.\n"); > > - return -1; > > - } > > - > > - return 0; > > -} > > diff --git a/platform/linux-generic/Makefile.am b/platform/linux- > > generic/Makefile.am > > index f4dfdc1..c4aae9d 100644 > > --- a/platform/linux-generic/Makefile.am > > +++ b/platform/linux-generic/Makefile.am > > @@ -62,6 +62,7 @@ __LIB__libodp_la_SOURCES = \ > > odp_packet_flags.c \ > > odp_packet_io.c \ > > odp_packet_socket.c \ > > + odp_platform.c \ > > odp_queue.c \ > > odp_ring.c \ > > odp_rwlock.c \ > > diff --git a/platform/linux-generic/include/api/odp.h b/platform/linux- > > generic/include/api/odp.h > > index 0ee3faf..99e2ae0 100644 > > --- a/platform/linux-generic/include/api/odp.h > > +++ b/platform/linux-generic/include/api/odp.h > > @@ -48,6 +48,7 @@ extern "C" { > > #include <odp_packet.h> > > #include <odp_packet_flags.h> > > #include <odp_packet_io.h> > > +#include <odp_crypto.h> > > > > #ifdef __cplusplus > > } > > diff --git a/platform/linux-generic/include/api/odp_init.h > > b/platform/linux-generic/include/api/odp_init.h > > index 490324a..8cdce46 100644 > > --- a/platform/linux-generic/include/api/odp_init.h > > +++ b/platform/linux-generic/include/api/odp_init.h > > @@ -22,24 +22,42 @@ extern "C" { > > > > #include <odp_std_types.h> > > > > - > > - > > +/** ODP initialisation data. > > + * Data that is required to initialize the ODP API with the > > + * application specific data such as specifying a logging callback, the > > log > > + * level etc. > > + */ > > +typedef struct odp_global_init_t { > > +} odp_global_init_t; > > + > > +/** ODP platform initialization data. > > + * @note ODP API does nothing with this data. It is the underlying > > + * implementation requires it and any data passed here is not portable. > > + * It is required that the application takes care of identifying and > > + * passing any required platform specific data. > > + */ > > +typedef struct odp_global_platform_init_t { > > +} odp_global_platform_init_t; > > > > /** > > - * Perform global ODP initalisation. > > - * > > - * This function must be called once before calling > > - * any other ODP API functions. > > + * Perform global ODP initialisation. > > * > > + * This function must be called once before calling any other ODP API > > + * functions. > > + * @param[in] params Those parameters that are interpreted by the ODP > API > > + * @param[in] platform_params Those parameters that are passed without > > + * interpretation by the ODP API to the implementation. > > * @return 0 if successful > > */ > > -int odp_init_global(void); > > +int odp_init_global(odp_global_init_t *params, > > + odp_global_platform_init_t *platform_params); > > > > > > /** > > - * Perform thread local ODP initalisation. > > + * Perform thread local ODP initialisation. > > * > > * All threads must call this function before calling > > + * > > > Extra line feed. > Thank you > > > > * any other ODP API functions. > > * @param thr_id Thread id > > * @return 0 if successful > > diff --git a/platform/linux-generic/include/odp_internal.h > > b/platform/linux-generic/include/odp_internal.h > > index aa79493..6c71fc9 100644 > > --- a/platform/linux-generic/include/odp_internal.h > > +++ b/platform/linux-generic/include/odp_internal.h > > @@ -4,6 +4,7 @@ > > * SPDX-License-Identifier: BSD-3-Clause > > */ > > > > +#include <odp_init.h> > > > > /** > > * @file > > @@ -42,6 +43,8 @@ int odp_schedule_init_local(void); > > int odp_timer_init_global(void); > > int odp_timer_disarm_all(void); > > > > +int odp_init_platform(odp_global_platform_init_t *platform_params); > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/platform/linux-generic/odp_init.c b/platform/linux- > > generic/odp_init.c > > index 5b7e192..f595def 100644 > > --- a/platform/linux-generic/odp_init.c > > +++ b/platform/linux-generic/odp_init.c > > @@ -8,13 +8,18 @@ > > #include <odp_internal.h> > > #include <odp_debug.h> > > > > - > > -int odp_init_global(void) > > +int odp_init_global(odp_global_init_t *params ODP_UNUSED, > > + odp_global_platform_init_t *platform_params ODP_UNUSED) > > { > > odp_thread_init_global(); > > > > odp_system_info_init(); > > > > + if (odp_init_platform(platform_params)) { > > > platform_params is used here -> remove ODP_UNUSED > Why is nothing warning about this unneeded warning - need to check into that removed thank you > > ...OR actually you can remove odp_init_platform() since it's doing > nothing. For the same reason, > DPDK re uses this file and then it does call odp_init_platform() that has a definition in that case. :w:w > there are no calls to e.g. odp_barrier_init(), odp_coremask_init(), > odp_spinlock_init() either - those do not need global initialization (in > linux-generic implementation). Also if some linux-generic platform init > data would be added to odp_global_platform_init_t, it would be consumed by > individual init functions e.g. odp_schedule_init_global(platform_params), > etc. > I agree that if we want to change how global_init is structured we should do that, making per platform copies, but the focus for this patch is the arguments working in the API I think. I have a v6 ready to send that addresses these points. > > This is the platform specific init ... there's not need for > odp_init_platform(). > > -Petri > > > > + ODP_ERR("ODP platform init failed.\n"); > > + return -1; > > + } > > + > > if (odp_shm_init_global()) { > > ODP_ERR("ODP shm init failed.\n"); > > return -1; > > diff --git a/platform/linux-generic/odp_platform.c b/platform/linux- > > generic/odp_platform.c > > new file mode 100644 > > index 0000000..a9efa7f > > --- /dev/null > > +++ b/platform/linux-generic/odp_platform.c > > @@ -0,0 +1,14 @@ > > +/* Copyright (c) 2014, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > +#include <odp_init.h> > > +#include <odp_internal.h> > > +#include <odp_debug.h> > > + > > +int odp_init_platform(odp_global_platform_init_t *platform_params > > ODP_UNUSED) > > +{ > > + return 0; > > +} > > http://lists.linaro.org/mailman/listinfo/lng-odp >
I am confused about the init sequence, I imagine this On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> wrote: > On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com> wrote: > >> Conti.. > >> > >> Root cause of my concern is not entirely specific to this patch. Its > >> more related to odp api initialization calling sequence in > >> application. From early days, we used to follow below step in > >> application - > >> > >> odp_global_init() -> which does almost everything for platform > >> initialization. > >> odp_shm_reserve() -> used for argument processing then > >> odp_create_thread etc... > >> > >> Now we found a need to introduce platform specific init hints, coming > >> from application. Therefore order of odp_xxx_init api calling sequence > >> should change too. > >> > >> > >> Something like. > >> > >> Application "X" > >> > >> main () > >> { > >> > >> - First do argument processing. For that, call odp_shm_init() in > >> application and remove it from odp_global_init OR application has to > >> allocate memory(bad idea ... I guess). > > > > argX and argY can be allocated from stack or heap (malloc). Shared > memory is not needed for those to be able to call odp_global_init(). ODP > will take copy of the argument, argX/Y can be destroyed after the call. > > Not to *call odp_global_init* , It is to pass command line processed > input to odp_globa_init().. Are you suggesting calling of parse_args() > before and after odp_global_init()? > > > > > > After ODP init, you can allocate shm and copy args there if you need to > share those within your app. > > Same argument processed twice ? isn't it. > > > > > > > -Petri > > > >> > >> - Then call odp_global/platform_init(argX, argY); where arg -X : > >> global param, Y : platform centric stuff. > >> > >> ... Rest stuff as it is. > >> } > >> > >> > >> Any idea? thoughts? Does above make sense. Thanks. > >> > >> > My view was that it went like this : main starts(args passed in) int foo; odp_init_t odp_data odp_platform_init_t platform_data /*Process the arguments passed in as normal.*/ loop { Set variables that main will use itself i.e. if --use-foo is passed set foo=true locally. Prepare odp_data to pass things into the implementation that ODP specifies, <- the internals of odp_data may be an argv string or a tidy struct we define populated by an ODP helper Prepare platform_data packaging any params required into the platform specific struct. <- Have no idea what these are, platform needs to figure this out } call global init(odp_data, platform_data) with the above ........ /* Act on any remaining parameters such as foo. */ if (foo)
Interesting point about ODP helper routines being used to help construct odp_global_init() parameters. Are we saying that what makes a helper a helper (from an application standpoint) is that it is a routine that can be called before odp_global_init() since it is a stand-alone function that operates independently from the rest of the ODP framework? That might be a good distinction and I'll make a note of it in the API Design Guidelines doc. Bill On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > I am confused about the init sequence, I imagine this > > > > > On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> > wrote: > >> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >> <petri.savolainen@nsn.com> wrote: >> >> Conti.. >> >> >> >> Root cause of my concern is not entirely specific to this patch. Its >> >> more related to odp api initialization calling sequence in >> >> application. From early days, we used to follow below step in >> >> application - >> >> >> >> odp_global_init() -> which does almost everything for platform >> >> initialization. >> >> odp_shm_reserve() -> used for argument processing then >> >> odp_create_thread etc... >> >> >> >> Now we found a need to introduce platform specific init hints, coming >> >> from application. Therefore order of odp_xxx_init api calling sequence >> >> should change too. >> >> >> >> >> >> Something like. >> >> >> >> Application "X" >> >> >> >> main () >> >> { >> >> >> >> - First do argument processing. For that, call odp_shm_init() in >> >> application and remove it from odp_global_init OR application has to >> >> allocate memory(bad idea ... I guess). >> > >> > argX and argY can be allocated from stack or heap (malloc). Shared >> memory is not needed for those to be able to call odp_global_init(). ODP >> will take copy of the argument, argX/Y can be destroyed after the call. >> >> Not to *call odp_global_init* , It is to pass command line processed >> input to odp_globa_init().. Are you suggesting calling of parse_args() >> before and after odp_global_init()? >> >> >> > >> > After ODP init, you can allocate shm and copy args there if you need to >> share those within your app. >> >> Same argument processed twice ? isn't it. >> >> >> >> > >> > -Petri >> > >> >> >> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >> >> global param, Y : platform centric stuff. >> >> >> >> ... Rest stuff as it is. >> >> } >> >> >> >> >> >> Any idea? thoughts? Does above make sense. Thanks. >> >> >> >> >> > > My view was that it went like this : > > > main starts(args passed in) > int foo; > odp_init_t odp_data > odp_platform_init_t platform_data > > /*Process the arguments passed in as normal.*/ > loop { > Set variables that main will use itself i.e. if --use-foo is passed set > foo=true locally. > Prepare odp_data to pass things into the implementation that ODP > specifies, <- the internals of odp_data may be an argv string or a tidy > struct we define populated by an ODP helper > Prepare platform_data packaging any params required into the platform > specific struct. <- Have no idea what these are, platform needs to figure > this out > } > > call global init(odp_data, platform_data) with the above > > ........ > > /* Act on any remaining parameters such as foo. */ > if (foo) > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
I think that is true by definition following Taras's logic from a thread a few weeks ago. The rational was that that helpers are users of the API to add higher functionality for an application to use, they don't help the implementation. However that breaks IPC I think, I have not read the latest patch, but that wants to use odph_ring.h - unless IPC is a helper, but I thought it was part of ODP. On 4 September 2014 14:59, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Interesting point about ODP helper routines being used to help construct > odp_global_init() parameters. Are we saying that what makes a helper a > helper (from an application standpoint) is that it is a routine that can be > called before odp_global_init() since it is a stand-alone function that > operates independently from the rest of the ODP framework? > > That might be a good distinction and I'll make a note of it in the API > Design Guidelines doc. > > Bill > > > On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> I am confused about the init sequence, I imagine this >> >> >> >> >> On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> >> wrote: >> >>> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >>> <petri.savolainen@nsn.com> wrote: >>> >> Conti.. >>> >> >>> >> Root cause of my concern is not entirely specific to this patch. Its >>> >> more related to odp api initialization calling sequence in >>> >> application. From early days, we used to follow below step in >>> >> application - >>> >> >>> >> odp_global_init() -> which does almost everything for platform >>> >> initialization. >>> >> odp_shm_reserve() -> used for argument processing then >>> >> odp_create_thread etc... >>> >> >>> >> Now we found a need to introduce platform specific init hints, coming >>> >> from application. Therefore order of odp_xxx_init api calling sequence >>> >> should change too. >>> >> >>> >> >>> >> Something like. >>> >> >>> >> Application "X" >>> >> >>> >> main () >>> >> { >>> >> >>> >> - First do argument processing. For that, call odp_shm_init() in >>> >> application and remove it from odp_global_init OR application has to >>> >> allocate memory(bad idea ... I guess). >>> > >>> > argX and argY can be allocated from stack or heap (malloc). Shared >>> memory is not needed for those to be able to call odp_global_init(). ODP >>> will take copy of the argument, argX/Y can be destroyed after the call. >>> >>> Not to *call odp_global_init* , It is to pass command line processed >>> input to odp_globa_init().. Are you suggesting calling of parse_args() >>> before and after odp_global_init()? >>> >>> >>> > >>> > After ODP init, you can allocate shm and copy args there if you need >>> to share those within your app. >>> >>> Same argument processed twice ? isn't it. >>> >>> >>> >>> > >>> > -Petri >>> > >>> >> >>> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >>> >> global param, Y : platform centric stuff. >>> >> >>> >> ... Rest stuff as it is. >>> >> } >>> >> >>> >> >>> >> Any idea? thoughts? Does above make sense. Thanks. >>> >> >>> >> >>> >> >> My view was that it went like this : >> >> >> main starts(args passed in) >> int foo; >> odp_init_t odp_data >> odp_platform_init_t platform_data >> >> /*Process the arguments passed in as normal.*/ >> loop { >> Set variables that main will use itself i.e. if --use-foo is passed >> set foo=true locally. >> Prepare odp_data to pass things into the implementation that ODP >> specifies, <- the internals of odp_data may be an argv string or a tidy >> struct we define populated by an ODP helper >> Prepare platform_data packaging any params required into the platform >> specific struct. <- Have no idea what these are, platform needs to figure >> this out >> } >> >> call global init(odp_data, platform_data) with the above >> >> ........ >> >> /* Act on any remaining parameters such as foo. */ >> if (foo) >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >
By that definition, ODP implementations would be free to use helpers. It's not that helpers cannot be used after odp_global_init() its that they are the only routines that MAY be used before odp_global_init(). Yes, IPC is intended to be part of the ODP API so an application cannot call IPC routines until after it has activated ODP via odp_global_init(). Bill On Thu, Sep 4, 2014 at 2:19 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > I think that is true by definition following Taras's logic from a thread a > few weeks ago. The rational was that that helpers are users of the API to > add higher functionality for an application to use, they don't help the > implementation. > > However that breaks IPC I think, I have not read the latest patch, but > that wants to use odph_ring.h - unless IPC is a helper, but I thought it > was part of ODP. > > > On 4 September 2014 14:59, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> Interesting point about ODP helper routines being used to help construct >> odp_global_init() parameters. Are we saying that what makes a helper a >> helper (from an application standpoint) is that it is a routine that can be >> called before odp_global_init() since it is a stand-alone function that >> operates independently from the rest of the ODP framework? >> >> That might be a good distinction and I'll make a note of it in the API >> Design Guidelines doc. >> >> Bill >> >> >> On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> I am confused about the init sequence, I imagine this >>> >>> >>> >>> >>> On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> >>> wrote: >>> >>>> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >>>> <petri.savolainen@nsn.com> wrote: >>>> >> Conti.. >>>> >> >>>> >> Root cause of my concern is not entirely specific to this patch. Its >>>> >> more related to odp api initialization calling sequence in >>>> >> application. From early days, we used to follow below step in >>>> >> application - >>>> >> >>>> >> odp_global_init() -> which does almost everything for platform >>>> >> initialization. >>>> >> odp_shm_reserve() -> used for argument processing then >>>> >> odp_create_thread etc... >>>> >> >>>> >> Now we found a need to introduce platform specific init hints, coming >>>> >> from application. Therefore order of odp_xxx_init api calling >>>> sequence >>>> >> should change too. >>>> >> >>>> >> >>>> >> Something like. >>>> >> >>>> >> Application "X" >>>> >> >>>> >> main () >>>> >> { >>>> >> >>>> >> - First do argument processing. For that, call odp_shm_init() in >>>> >> application and remove it from odp_global_init OR application has to >>>> >> allocate memory(bad idea ... I guess). >>>> > >>>> > argX and argY can be allocated from stack or heap (malloc). Shared >>>> memory is not needed for those to be able to call odp_global_init(). ODP >>>> will take copy of the argument, argX/Y can be destroyed after the call. >>>> >>>> Not to *call odp_global_init* , It is to pass command line processed >>>> input to odp_globa_init().. Are you suggesting calling of parse_args() >>>> before and after odp_global_init()? >>>> >>>> >>>> > >>>> > After ODP init, you can allocate shm and copy args there if you need >>>> to share those within your app. >>>> >>>> Same argument processed twice ? isn't it. >>>> >>>> >>>> >>>> > >>>> > -Petri >>>> > >>>> >> >>>> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >>>> >> global param, Y : platform centric stuff. >>>> >> >>>> >> ... Rest stuff as it is. >>>> >> } >>>> >> >>>> >> >>>> >> Any idea? thoughts? Does above make sense. Thanks. >>>> >> >>>> >> >>>> >>> >>> My view was that it went like this : >>> >>> >>> main starts(args passed in) >>> int foo; >>> odp_init_t odp_data >>> odp_platform_init_t platform_data >>> >>> /*Process the arguments passed in as normal.*/ >>> loop { >>> Set variables that main will use itself i.e. if --use-foo is passed >>> set foo=true locally. >>> Prepare odp_data to pass things into the implementation that ODP >>> specifies, <- the internals of odp_data may be an argv string or a tidy >>> struct we define populated by an ODP helper >>> Prepare platform_data packaging any params required into the platform >>> specific struct. <- Have no idea what these are, platform needs to figure >>> this out >>> } >>> >>> call global init(odp_data, platform_data) with the above >>> >>> ........ >>> >>> /* Act on any remaining parameters such as foo. */ >>> if (foo) >>> >>> >>> -- >>> *Mike Holmes* >>> Linaro Technical Manager / Lead >>> LNG - ODP >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >> > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP >
Ok, that is a nice testable assumption for the validation tests, helpers should work perfectly before global_init. But the optional part of helpers gets lost if IPC uses it, then the ring helper is mandatory, that differs from me believing that an application could opt not use helpers at all and still use all the API. On 4 September 2014 15:23, Bill Fischofer <bill.fischofer@linaro.org> wrote: > By that definition, ODP implementations would be free to use helpers. > It's not that helpers cannot be used after odp_global_init() its that they > are the only routines that MAY be used before odp_global_init(). > > Yes, IPC is intended to be part of the ODP API so an application cannot > call IPC routines until after it has activated ODP via odp_global_init(). > > Bill > > > On Thu, Sep 4, 2014 at 2:19 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> I think that is true by definition following Taras's logic from a thread >> a few weeks ago. The rational was that that helpers are users of the API to >> add higher functionality for an application to use, they don't help the >> implementation. >> >> However that breaks IPC I think, I have not read the latest patch, but >> that wants to use odph_ring.h - unless IPC is a helper, but I thought it >> was part of ODP. >> >> >> On 4 September 2014 14:59, Bill Fischofer <bill.fischofer@linaro.org> >> wrote: >> >>> Interesting point about ODP helper routines being used to help construct >>> odp_global_init() parameters. Are we saying that what makes a helper a >>> helper (from an application standpoint) is that it is a routine that can be >>> called before odp_global_init() since it is a stand-alone function that >>> operates independently from the rest of the ODP framework? >>> >>> That might be a good distinction and I'll make a note of it in the API >>> Design Guidelines doc. >>> >>> Bill >>> >>> >>> On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> >>> wrote: >>> >>>> I am confused about the init sequence, I imagine this >>>> >>>> >>>> >>>> >>>> On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> >>>> wrote: >>>> >>>>> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >>>>> <petri.savolainen@nsn.com> wrote: >>>>> >> Conti.. >>>>> >> >>>>> >> Root cause of my concern is not entirely specific to this patch. Its >>>>> >> more related to odp api initialization calling sequence in >>>>> >> application. From early days, we used to follow below step in >>>>> >> application - >>>>> >> >>>>> >> odp_global_init() -> which does almost everything for platform >>>>> >> initialization. >>>>> >> odp_shm_reserve() -> used for argument processing then >>>>> >> odp_create_thread etc... >>>>> >> >>>>> >> Now we found a need to introduce platform specific init hints, >>>>> coming >>>>> >> from application. Therefore order of odp_xxx_init api calling >>>>> sequence >>>>> >> should change too. >>>>> >> >>>>> >> >>>>> >> Something like. >>>>> >> >>>>> >> Application "X" >>>>> >> >>>>> >> main () >>>>> >> { >>>>> >> >>>>> >> - First do argument processing. For that, call odp_shm_init() in >>>>> >> application and remove it from odp_global_init OR application has to >>>>> >> allocate memory(bad idea ... I guess). >>>>> > >>>>> > argX and argY can be allocated from stack or heap (malloc). Shared >>>>> memory is not needed for those to be able to call odp_global_init(). ODP >>>>> will take copy of the argument, argX/Y can be destroyed after the call. >>>>> >>>>> Not to *call odp_global_init* , It is to pass command line processed >>>>> input to odp_globa_init().. Are you suggesting calling of parse_args() >>>>> before and after odp_global_init()? >>>>> >>>>> >>>>> > >>>>> > After ODP init, you can allocate shm and copy args there if you need >>>>> to share those within your app. >>>>> >>>>> Same argument processed twice ? isn't it. >>>>> >>>>> >>>>> >>>>> > >>>>> > -Petri >>>>> > >>>>> >> >>>>> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >>>>> >> global param, Y : platform centric stuff. >>>>> >> >>>>> >> ... Rest stuff as it is. >>>>> >> } >>>>> >> >>>>> >> >>>>> >> Any idea? thoughts? Does above make sense. Thanks. >>>>> >> >>>>> >> >>>>> >>>> >>>> My view was that it went like this : >>>> >>>> >>>> main starts(args passed in) >>>> int foo; >>>> odp_init_t odp_data >>>> odp_platform_init_t platform_data >>>> >>>> /*Process the arguments passed in as normal.*/ >>>> loop { >>>> Set variables that main will use itself i.e. if --use-foo is passed >>>> set foo=true locally. >>>> Prepare odp_data to pass things into the implementation that ODP >>>> specifies, <- the internals of odp_data may be an argv string or a tidy >>>> struct we define populated by an ODP helper >>>> Prepare platform_data packaging any params required into the platform >>>> specific struct. <- Have no idea what these are, platform needs to figure >>>> this out >>>> } >>>> >>>> call global init(odp_data, platform_data) with the above >>>> >>>> ........ >>>> >>>> /* Act on any remaining parameters such as foo. */ >>>> if (foo) >>>> >>>> >>>> -- >>>> *Mike Holmes* >>>> Linaro Technical Manager / Lead >>>> LNG - ODP >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>>> >>> >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> > >
Implementations are opaque to the application so whether or not an implementation makes use of a helper function should be transparent to the application, no? On Thu, Sep 4, 2014 at 2:28 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > Ok, that is a nice testable assumption for the validation tests, helpers > should work perfectly before global_init. > > But the optional part of helpers gets lost if IPC uses it, then the ring > helper is mandatory, that differs from me believing that an application > could opt not use helpers at all and still use all the API. > > > On 4 September 2014 15:23, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> By that definition, ODP implementations would be free to use helpers. >> It's not that helpers cannot be used after odp_global_init() its that they >> are the only routines that MAY be used before odp_global_init(). >> >> Yes, IPC is intended to be part of the ODP API so an application cannot >> call IPC routines until after it has activated ODP via odp_global_init(). >> >> Bill >> >> >> On Thu, Sep 4, 2014 at 2:19 PM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> I think that is true by definition following Taras's logic from a thread >>> a few weeks ago. The rational was that that helpers are users of the API to >>> add higher functionality for an application to use, they don't help the >>> implementation. >>> >>> However that breaks IPC I think, I have not read the latest patch, but >>> that wants to use odph_ring.h - unless IPC is a helper, but I thought it >>> was part of ODP. >>> >>> >>> On 4 September 2014 14:59, Bill Fischofer <bill.fischofer@linaro.org> >>> wrote: >>> >>>> Interesting point about ODP helper routines being used to help >>>> construct odp_global_init() parameters. Are we saying that what makes a >>>> helper a helper (from an application standpoint) is that it is a routine >>>> that can be called before odp_global_init() since it is a stand-alone >>>> function that operates independently from the rest of the ODP framework? >>>> >>>> That might be a good distinction and I'll make a note of it in the API >>>> Design Guidelines doc. >>>> >>>> Bill >>>> >>>> >>>> On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> >>>> wrote: >>>> >>>>> I am confused about the init sequence, I imagine this >>>>> >>>>> >>>>> >>>>> >>>>> On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> >>>>> wrote: >>>>> >>>>>> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >>>>>> <petri.savolainen@nsn.com> wrote: >>>>>> >> Conti.. >>>>>> >> >>>>>> >> Root cause of my concern is not entirely specific to this patch. >>>>>> Its >>>>>> >> more related to odp api initialization calling sequence in >>>>>> >> application. From early days, we used to follow below step in >>>>>> >> application - >>>>>> >> >>>>>> >> odp_global_init() -> which does almost everything for platform >>>>>> >> initialization. >>>>>> >> odp_shm_reserve() -> used for argument processing then >>>>>> >> odp_create_thread etc... >>>>>> >> >>>>>> >> Now we found a need to introduce platform specific init hints, >>>>>> coming >>>>>> >> from application. Therefore order of odp_xxx_init api calling >>>>>> sequence >>>>>> >> should change too. >>>>>> >> >>>>>> >> >>>>>> >> Something like. >>>>>> >> >>>>>> >> Application "X" >>>>>> >> >>>>>> >> main () >>>>>> >> { >>>>>> >> >>>>>> >> - First do argument processing. For that, call odp_shm_init() in >>>>>> >> application and remove it from odp_global_init OR application has >>>>>> to >>>>>> >> allocate memory(bad idea ... I guess). >>>>>> > >>>>>> > argX and argY can be allocated from stack or heap (malloc). Shared >>>>>> memory is not needed for those to be able to call odp_global_init(). ODP >>>>>> will take copy of the argument, argX/Y can be destroyed after the call. >>>>>> >>>>>> Not to *call odp_global_init* , It is to pass command line processed >>>>>> input to odp_globa_init().. Are you suggesting calling of parse_args() >>>>>> before and after odp_global_init()? >>>>>> >>>>>> >>>>>> > >>>>>> > After ODP init, you can allocate shm and copy args there if you >>>>>> need to share those within your app. >>>>>> >>>>>> Same argument processed twice ? isn't it. >>>>>> >>>>>> >>>>>> >>>>>> > >>>>>> > -Petri >>>>>> > >>>>>> >> >>>>>> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >>>>>> >> global param, Y : platform centric stuff. >>>>>> >> >>>>>> >> ... Rest stuff as it is. >>>>>> >> } >>>>>> >> >>>>>> >> >>>>>> >> Any idea? thoughts? Does above make sense. Thanks. >>>>>> >> >>>>>> >> >>>>>> >>>>> >>>>> My view was that it went like this : >>>>> >>>>> >>>>> main starts(args passed in) >>>>> int foo; >>>>> odp_init_t odp_data >>>>> odp_platform_init_t platform_data >>>>> >>>>> /*Process the arguments passed in as normal.*/ >>>>> loop { >>>>> Set variables that main will use itself i.e. if --use-foo is passed >>>>> set foo=true locally. >>>>> Prepare odp_data to pass things into the implementation that ODP >>>>> specifies, <- the internals of odp_data may be an argv string or a tidy >>>>> struct we define populated by an ODP helper >>>>> Prepare platform_data packaging any params required into the >>>>> platform specific struct. <- Have no idea what these are, platform needs >>>>> to figure this out >>>>> } >>>>> >>>>> call global init(odp_data, platform_data) with the above >>>>> >>>>> ........ >>>>> >>>>> /* Act on any remaining parameters such as foo. */ >>>>> if (foo) >>>>> >>>>> >>>>> -- >>>>> *Mike Holmes* >>>>> Linaro Technical Manager / Lead >>>>> LNG - ODP >>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>> >>>>> >>>> >>> >>> >>> -- >>> *Mike Holmes* >>> Linaro Technical Manager / Lead >>> LNG - ODP >>> >> >> > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP >
You could be right. I had just imagined that helpers gets much bigger and would be something that may be omitted. It would provide meat to ODP like say a TCP/IP stack, DPI - regexp tools - something substantial but not of use to everyone, and something that does not change per platform, but is tightly coupled to using ODP APIs. On 4 September 2014 15:32, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Implementations are opaque to the application so whether or not an > implementation makes use of a helper function should be transparent to the > application, no? > > > > > On Thu, Sep 4, 2014 at 2:28 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> Ok, that is a nice testable assumption for the validation tests, helpers >> should work perfectly before global_init. >> >> But the optional part of helpers gets lost if IPC uses it, then the ring >> helper is mandatory, that differs from me believing that an application >> could opt not use helpers at all and still use all the API. >> >> >> On 4 September 2014 15:23, Bill Fischofer <bill.fischofer@linaro.org> >> wrote: >> >>> By that definition, ODP implementations would be free to use helpers. >>> It's not that helpers cannot be used after odp_global_init() its that they >>> are the only routines that MAY be used before odp_global_init(). >>> >>> Yes, IPC is intended to be part of the ODP API so an application cannot >>> call IPC routines until after it has activated ODP via odp_global_init(). >>> >>> Bill >>> >>> >>> On Thu, Sep 4, 2014 at 2:19 PM, Mike Holmes <mike.holmes@linaro.org> >>> wrote: >>> >>>> I think that is true by definition following Taras's logic from a >>>> thread a few weeks ago. The rational was that that helpers are users of the >>>> API to add higher functionality for an application to use, they don't help >>>> the implementation. >>>> >>>> However that breaks IPC I think, I have not read the latest patch, but >>>> that wants to use odph_ring.h - unless IPC is a helper, but I thought it >>>> was part of ODP. >>>> >>>> >>>> On 4 September 2014 14:59, Bill Fischofer <bill.fischofer@linaro.org> >>>> wrote: >>>> >>>>> Interesting point about ODP helper routines being used to help >>>>> construct odp_global_init() parameters. Are we saying that what makes a >>>>> helper a helper (from an application standpoint) is that it is a routine >>>>> that can be called before odp_global_init() since it is a stand-alone >>>>> function that operates independently from the rest of the ODP framework? >>>>> >>>>> That might be a good distinction and I'll make a note of it in the API >>>>> Design Guidelines doc. >>>>> >>>>> Bill >>>>> >>>>> >>>>> On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> >>>>> wrote: >>>>> >>>>>> I am confused about the init sequence, I imagine this >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> >>>>>> wrote: >>>>>> >>>>>>> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >>>>>>> <petri.savolainen@nsn.com> wrote: >>>>>>> >> Conti.. >>>>>>> >> >>>>>>> >> Root cause of my concern is not entirely specific to this patch. >>>>>>> Its >>>>>>> >> more related to odp api initialization calling sequence in >>>>>>> >> application. From early days, we used to follow below step in >>>>>>> >> application - >>>>>>> >> >>>>>>> >> odp_global_init() -> which does almost everything for platform >>>>>>> >> initialization. >>>>>>> >> odp_shm_reserve() -> used for argument processing then >>>>>>> >> odp_create_thread etc... >>>>>>> >> >>>>>>> >> Now we found a need to introduce platform specific init hints, >>>>>>> coming >>>>>>> >> from application. Therefore order of odp_xxx_init api calling >>>>>>> sequence >>>>>>> >> should change too. >>>>>>> >> >>>>>>> >> >>>>>>> >> Something like. >>>>>>> >> >>>>>>> >> Application "X" >>>>>>> >> >>>>>>> >> main () >>>>>>> >> { >>>>>>> >> >>>>>>> >> - First do argument processing. For that, call odp_shm_init() in >>>>>>> >> application and remove it from odp_global_init OR application has >>>>>>> to >>>>>>> >> allocate memory(bad idea ... I guess). >>>>>>> > >>>>>>> > argX and argY can be allocated from stack or heap (malloc). Shared >>>>>>> memory is not needed for those to be able to call odp_global_init(). ODP >>>>>>> will take copy of the argument, argX/Y can be destroyed after the call. >>>>>>> >>>>>>> Not to *call odp_global_init* , It is to pass command line processed >>>>>>> input to odp_globa_init().. Are you suggesting calling of >>>>>>> parse_args() >>>>>>> before and after odp_global_init()? >>>>>>> >>>>>>> >>>>>>> > >>>>>>> > After ODP init, you can allocate shm and copy args there if you >>>>>>> need to share those within your app. >>>>>>> >>>>>>> Same argument processed twice ? isn't it. >>>>>>> >>>>>>> >>>>>>> >>>>>>> > >>>>>>> > -Petri >>>>>>> > >>>>>>> >> >>>>>>> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >>>>>>> >> global param, Y : platform centric stuff. >>>>>>> >> >>>>>>> >> ... Rest stuff as it is. >>>>>>> >> } >>>>>>> >> >>>>>>> >> >>>>>>> >> Any idea? thoughts? Does above make sense. Thanks. >>>>>>> >> >>>>>>> >> >>>>>>> >>>>>> >>>>>> My view was that it went like this : >>>>>> >>>>>> >>>>>> main starts(args passed in) >>>>>> int foo; >>>>>> odp_init_t odp_data >>>>>> odp_platform_init_t platform_data >>>>>> >>>>>> /*Process the arguments passed in as normal.*/ >>>>>> loop { >>>>>> Set variables that main will use itself i.e. if --use-foo is passed >>>>>> set foo=true locally. >>>>>> Prepare odp_data to pass things into the implementation that ODP >>>>>> specifies, <- the internals of odp_data may be an argv string or a tidy >>>>>> struct we define populated by an ODP helper >>>>>> Prepare platform_data packaging any params required into the >>>>>> platform specific struct. <- Have no idea what these are, platform needs >>>>>> to figure this out >>>>>> } >>>>>> >>>>>> call global init(odp_data, platform_data) with the above >>>>>> >>>>>> ........ >>>>>> >>>>>> /* Act on any remaining parameters such as foo. */ >>>>>> if (foo) >>>>>> >>>>>> >>>>>> -- >>>>>> *Mike Holmes* >>>>>> Linaro Technical Manager / Lead >>>>>> LNG - ODP >>>>>> >>>>>> _______________________________________________ >>>>>> lng-odp mailing list >>>>>> lng-odp@lists.linaro.org >>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>> >>>>>> >>>>> >>>> >>>> >>>> -- >>>> *Mike Holmes* >>>> Linaro Technical Manager / Lead >>>> LNG - ODP >>>> >>> >>> >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> > >
Something like a TCP/IP stack is a bit grandiose for a humble "helper" designation. Sounds more like an ODP sharable service that could be used by ODP applications. We can envision ODP applications being written as specific applications or as general service routines that can be used in other ODP applications as shared libraries. They'd assume they were being called from an ODP application and had the ODP framework available to them. Beyond the scope of ODP development per se, but certainly a reasonable part of a growing ODP ecosystem. On Thu, Sep 4, 2014 at 2:37 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > You could be right. > I had just imagined that helpers gets much bigger and would be something > that may be omitted. > It would provide meat to ODP like say a TCP/IP stack, DPI - regexp tools - > something substantial but not of use to everyone, and something that does > not change per platform, but is tightly coupled to using ODP APIs. > > > On 4 September 2014 15:32, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> Implementations are opaque to the application so whether or not an >> implementation makes use of a helper function should be transparent to the >> application, no? >> >> >> >> >> On Thu, Sep 4, 2014 at 2:28 PM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> Ok, that is a nice testable assumption for the validation tests, helpers >>> should work perfectly before global_init. >>> >>> But the optional part of helpers gets lost if IPC uses it, then the ring >>> helper is mandatory, that differs from me believing that an application >>> could opt not use helpers at all and still use all the API. >>> >>> >>> On 4 September 2014 15:23, Bill Fischofer <bill.fischofer@linaro.org> >>> wrote: >>> >>>> By that definition, ODP implementations would be free to use helpers. >>>> It's not that helpers cannot be used after odp_global_init() its that they >>>> are the only routines that MAY be used before odp_global_init(). >>>> >>>> Yes, IPC is intended to be part of the ODP API so an application cannot >>>> call IPC routines until after it has activated ODP via odp_global_init(). >>>> >>>> Bill >>>> >>>> >>>> On Thu, Sep 4, 2014 at 2:19 PM, Mike Holmes <mike.holmes@linaro.org> >>>> wrote: >>>> >>>>> I think that is true by definition following Taras's logic from a >>>>> thread a few weeks ago. The rational was that that helpers are users of the >>>>> API to add higher functionality for an application to use, they don't help >>>>> the implementation. >>>>> >>>>> However that breaks IPC I think, I have not read the latest patch, but >>>>> that wants to use odph_ring.h - unless IPC is a helper, but I thought it >>>>> was part of ODP. >>>>> >>>>> >>>>> On 4 September 2014 14:59, Bill Fischofer <bill.fischofer@linaro.org> >>>>> wrote: >>>>> >>>>>> Interesting point about ODP helper routines being used to help >>>>>> construct odp_global_init() parameters. Are we saying that what makes a >>>>>> helper a helper (from an application standpoint) is that it is a routine >>>>>> that can be called before odp_global_init() since it is a stand-alone >>>>>> function that operates independently from the rest of the ODP framework? >>>>>> >>>>>> That might be a good distinction and I'll make a note of it in the >>>>>> API Design Guidelines doc. >>>>>> >>>>>> Bill >>>>>> >>>>>> >>>>>> On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> >>>>>> wrote: >>>>>> >>>>>>> I am confused about the init sequence, I imagine this >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org >>>>>>> > wrote: >>>>>>> >>>>>>>> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >>>>>>>> <petri.savolainen@nsn.com> wrote: >>>>>>>> >> Conti.. >>>>>>>> >> >>>>>>>> >> Root cause of my concern is not entirely specific to this patch. >>>>>>>> Its >>>>>>>> >> more related to odp api initialization calling sequence in >>>>>>>> >> application. From early days, we used to follow below step in >>>>>>>> >> application - >>>>>>>> >> >>>>>>>> >> odp_global_init() -> which does almost everything for platform >>>>>>>> >> initialization. >>>>>>>> >> odp_shm_reserve() -> used for argument processing then >>>>>>>> >> odp_create_thread etc... >>>>>>>> >> >>>>>>>> >> Now we found a need to introduce platform specific init hints, >>>>>>>> coming >>>>>>>> >> from application. Therefore order of odp_xxx_init api calling >>>>>>>> sequence >>>>>>>> >> should change too. >>>>>>>> >> >>>>>>>> >> >>>>>>>> >> Something like. >>>>>>>> >> >>>>>>>> >> Application "X" >>>>>>>> >> >>>>>>>> >> main () >>>>>>>> >> { >>>>>>>> >> >>>>>>>> >> - First do argument processing. For that, call odp_shm_init() in >>>>>>>> >> application and remove it from odp_global_init OR application >>>>>>>> has to >>>>>>>> >> allocate memory(bad idea ... I guess). >>>>>>>> > >>>>>>>> > argX and argY can be allocated from stack or heap (malloc). >>>>>>>> Shared memory is not needed for those to be able to call odp_global_init(). >>>>>>>> ODP will take copy of the argument, argX/Y can be destroyed after the call. >>>>>>>> >>>>>>>> Not to *call odp_global_init* , It is to pass command line processed >>>>>>>> input to odp_globa_init().. Are you suggesting calling of >>>>>>>> parse_args() >>>>>>>> before and after odp_global_init()? >>>>>>>> >>>>>>>> >>>>>>>> > >>>>>>>> > After ODP init, you can allocate shm and copy args there if you >>>>>>>> need to share those within your app. >>>>>>>> >>>>>>>> Same argument processed twice ? isn't it. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> > >>>>>>>> > -Petri >>>>>>>> > >>>>>>>> >> >>>>>>>> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >>>>>>>> >> global param, Y : platform centric stuff. >>>>>>>> >> >>>>>>>> >> ... Rest stuff as it is. >>>>>>>> >> } >>>>>>>> >> >>>>>>>> >> >>>>>>>> >> Any idea? thoughts? Does above make sense. Thanks. >>>>>>>> >> >>>>>>>> >> >>>>>>>> >>>>>>> >>>>>>> My view was that it went like this : >>>>>>> >>>>>>> >>>>>>> main starts(args passed in) >>>>>>> int foo; >>>>>>> odp_init_t odp_data >>>>>>> odp_platform_init_t platform_data >>>>>>> >>>>>>> /*Process the arguments passed in as normal.*/ >>>>>>> loop { >>>>>>> Set variables that main will use itself i.e. if --use-foo is >>>>>>> passed set foo=true locally. >>>>>>> Prepare odp_data to pass things into the implementation that ODP >>>>>>> specifies, <- the internals of odp_data may be an argv string or a tidy >>>>>>> struct we define populated by an ODP helper >>>>>>> Prepare platform_data packaging any params required into the >>>>>>> platform specific struct. <- Have no idea what these are, platform needs >>>>>>> to figure this out >>>>>>> } >>>>>>> >>>>>>> call global init(odp_data, platform_data) with the above >>>>>>> >>>>>>> ........ >>>>>>> >>>>>>> /* Act on any remaining parameters such as foo. */ >>>>>>> if (foo) >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *Mike Holmes* >>>>>>> Linaro Technical Manager / Lead >>>>>>> LNG - ODP >>>>>>> >>>>>>> _______________________________________________ >>>>>>> lng-odp mailing list >>>>>>> lng-odp@lists.linaro.org >>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> *Mike Holmes* >>>>> Linaro Technical Manager / Lead >>>>> LNG - ODP >>>>> >>>> >>>> >>> >>> >>> -- >>> *Mike Holmes* >>> Linaro Technical Manager / Lead >>> LNG - ODP >>> >> >> > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP >
Ok, so the germane question so that I have the same image as you is, are helpers mandatory to the implementation ? I really got the impression they would not be. With that spelled out I think it will be clear to me what "helpers" means On 4 September 2014 15:43, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Something like a TCP/IP stack is a bit grandiose for a humble "helper" > designation. Sounds more like an ODP sharable service that could be used > by ODP applications. > > We can envision ODP applications being written as specific applications or > as general service routines that can be used in other ODP applications as > shared libraries. They'd assume they were being called from an ODP > application and had the ODP framework available to them. Beyond the scope > of ODP development per se, but certainly a reasonable part of a growing ODP > ecosystem. > > > On Thu, Sep 4, 2014 at 2:37 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> You could be right. >> I had just imagined that helpers gets much bigger and would be something >> that may be omitted. >> It would provide meat to ODP like say a TCP/IP stack, DPI - regexp tools >> - something substantial but not of use to everyone, and something that does >> not change per platform, but is tightly coupled to using ODP APIs. >> >> >> On 4 September 2014 15:32, Bill Fischofer <bill.fischofer@linaro.org> >> wrote: >> >>> Implementations are opaque to the application so whether or not an >>> implementation makes use of a helper function should be transparent to the >>> application, no? >>> >>> >>> >>> >>> On Thu, Sep 4, 2014 at 2:28 PM, Mike Holmes <mike.holmes@linaro.org> >>> wrote: >>> >>>> Ok, that is a nice testable assumption for the validation tests, >>>> helpers should work perfectly before global_init. >>>> >>>> But the optional part of helpers gets lost if IPC uses it, then the >>>> ring helper is mandatory, that differs from me believing that an >>>> application could opt not use helpers at all and still use all the API. >>>> >>>> >>>> On 4 September 2014 15:23, Bill Fischofer <bill.fischofer@linaro.org> >>>> wrote: >>>> >>>>> By that definition, ODP implementations would be free to use helpers. >>>>> It's not that helpers cannot be used after odp_global_init() its that they >>>>> are the only routines that MAY be used before odp_global_init(). >>>>> >>>>> Yes, IPC is intended to be part of the ODP API so an application >>>>> cannot call IPC routines until after it has activated ODP via >>>>> odp_global_init(). >>>>> >>>>> Bill >>>>> >>>>> >>>>> On Thu, Sep 4, 2014 at 2:19 PM, Mike Holmes <mike.holmes@linaro.org> >>>>> wrote: >>>>> >>>>>> I think that is true by definition following Taras's logic from a >>>>>> thread a few weeks ago. The rational was that that helpers are users of the >>>>>> API to add higher functionality for an application to use, they don't help >>>>>> the implementation. >>>>>> >>>>>> However that breaks IPC I think, I have not read the latest patch, >>>>>> but that wants to use odph_ring.h - unless IPC is a helper, but I thought >>>>>> it was part of ODP. >>>>>> >>>>>> >>>>>> On 4 September 2014 14:59, Bill Fischofer <bill.fischofer@linaro.org> >>>>>> wrote: >>>>>> >>>>>>> Interesting point about ODP helper routines being used to help >>>>>>> construct odp_global_init() parameters. Are we saying that what makes a >>>>>>> helper a helper (from an application standpoint) is that it is a routine >>>>>>> that can be called before odp_global_init() since it is a stand-alone >>>>>>> function that operates independently from the rest of the ODP framework? >>>>>>> >>>>>>> That might be a good distinction and I'll make a note of it in the >>>>>>> API Design Guidelines doc. >>>>>>> >>>>>>> Bill >>>>>>> >>>>>>> >>>>>>> On Thu, Sep 4, 2014 at 1:43 PM, Mike Holmes <mike.holmes@linaro.org> >>>>>>> wrote: >>>>>>> >>>>>>>> I am confused about the init sequence, I imagine this >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 4 September 2014 04:40, Santosh Shukla < >>>>>>>> santosh.shukla@linaro.org> wrote: >>>>>>>> >>>>>>>>> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >>>>>>>>> <petri.savolainen@nsn.com> wrote: >>>>>>>>> >> Conti.. >>>>>>>>> >> >>>>>>>>> >> Root cause of my concern is not entirely specific to this >>>>>>>>> patch. Its >>>>>>>>> >> more related to odp api initialization calling sequence in >>>>>>>>> >> application. From early days, we used to follow below step in >>>>>>>>> >> application - >>>>>>>>> >> >>>>>>>>> >> odp_global_init() -> which does almost everything for platform >>>>>>>>> >> initialization. >>>>>>>>> >> odp_shm_reserve() -> used for argument processing then >>>>>>>>> >> odp_create_thread etc... >>>>>>>>> >> >>>>>>>>> >> Now we found a need to introduce platform specific init hints, >>>>>>>>> coming >>>>>>>>> >> from application. Therefore order of odp_xxx_init api calling >>>>>>>>> sequence >>>>>>>>> >> should change too. >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> >> Something like. >>>>>>>>> >> >>>>>>>>> >> Application "X" >>>>>>>>> >> >>>>>>>>> >> main () >>>>>>>>> >> { >>>>>>>>> >> >>>>>>>>> >> - First do argument processing. For that, call odp_shm_init() in >>>>>>>>> >> application and remove it from odp_global_init OR application >>>>>>>>> has to >>>>>>>>> >> allocate memory(bad idea ... I guess). >>>>>>>>> > >>>>>>>>> > argX and argY can be allocated from stack or heap (malloc). >>>>>>>>> Shared memory is not needed for those to be able to call odp_global_init(). >>>>>>>>> ODP will take copy of the argument, argX/Y can be destroyed after the call. >>>>>>>>> >>>>>>>>> Not to *call odp_global_init* , It is to pass command line >>>>>>>>> processed >>>>>>>>> input to odp_globa_init().. Are you suggesting calling of >>>>>>>>> parse_args() >>>>>>>>> before and after odp_global_init()? >>>>>>>>> >>>>>>>>> >>>>>>>>> > >>>>>>>>> > After ODP init, you can allocate shm and copy args there if you >>>>>>>>> need to share those within your app. >>>>>>>>> >>>>>>>>> Same argument processed twice ? isn't it. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> > >>>>>>>>> > -Petri >>>>>>>>> > >>>>>>>>> >> >>>>>>>>> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >>>>>>>>> >> global param, Y : platform centric stuff. >>>>>>>>> >> >>>>>>>>> >> ... Rest stuff as it is. >>>>>>>>> >> } >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> >> Any idea? thoughts? Does above make sense. Thanks. >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> >>>>>>>> >>>>>>>> My view was that it went like this : >>>>>>>> >>>>>>>> >>>>>>>> main starts(args passed in) >>>>>>>> int foo; >>>>>>>> odp_init_t odp_data >>>>>>>> odp_platform_init_t platform_data >>>>>>>> >>>>>>>> /*Process the arguments passed in as normal.*/ >>>>>>>> loop { >>>>>>>> Set variables that main will use itself i.e. if --use-foo is >>>>>>>> passed set foo=true locally. >>>>>>>> Prepare odp_data to pass things into the implementation that ODP >>>>>>>> specifies, <- the internals of odp_data may be an argv string or a tidy >>>>>>>> struct we define populated by an ODP helper >>>>>>>> Prepare platform_data packaging any params required into the >>>>>>>> platform specific struct. <- Have no idea what these are, platform needs >>>>>>>> to figure this out >>>>>>>> } >>>>>>>> >>>>>>>> call global init(odp_data, platform_data) with the above >>>>>>>> >>>>>>>> ........ >>>>>>>> >>>>>>>> /* Act on any remaining parameters such as foo. */ >>>>>>>> if (foo) >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> *Mike Holmes* >>>>>>>> Linaro Technical Manager / Lead >>>>>>>> LNG - ODP >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> lng-odp mailing list >>>>>>>> lng-odp@lists.linaro.org >>>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> *Mike Holmes* >>>>>> Linaro Technical Manager / Lead >>>>>> LNG - ODP >>>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> *Mike Holmes* >>>> Linaro Technical Manager / Lead >>>> LNG - ODP >>>> >>> >>> >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> > >
On 5 September 2014 00:13, Mike Holmes <mike.holmes@linaro.org> wrote: > I am confused about the init sequence, I imagine this > > > > > On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> wrote: >> >> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >> <petri.savolainen@nsn.com> wrote: >> >> Conti.. >> >> >> >> Root cause of my concern is not entirely specific to this patch. Its >> >> more related to odp api initialization calling sequence in >> >> application. From early days, we used to follow below step in >> >> application - >> >> >> >> odp_global_init() -> which does almost everything for platform >> >> initialization. >> >> odp_shm_reserve() -> used for argument processing then >> >> odp_create_thread etc... >> >> >> >> Now we found a need to introduce platform specific init hints, coming >> >> from application. Therefore order of odp_xxx_init api calling sequence >> >> should change too. >> >> >> >> >> >> Something like. >> >> >> >> Application "X" >> >> >> >> main () >> >> { >> >> >> >> - First do argument processing. For that, call odp_shm_init() in >> >> application and remove it from odp_global_init OR application has to >> >> allocate memory(bad idea ... I guess). >> > >> > argX and argY can be allocated from stack or heap (malloc). Shared >> > memory is not needed for those to be able to call odp_global_init(). ODP >> > will take copy of the argument, argX/Y can be destroyed after the call. >> >> Not to *call odp_global_init* , It is to pass command line processed >> input to odp_globa_init().. Are you suggesting calling of parse_args() >> before and after odp_global_init()? >> >> >> > >> > After ODP init, you can allocate shm and copy args there if you need to >> > share those within your app. >> >> Same argument processed twice ? isn't it. >> >> >> >> > >> > -Petri >> > >> >> >> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >> >> global param, Y : platform centric stuff. >> >> >> >> ... Rest stuff as it is. >> >> } >> >> >> >> >> >> Any idea? thoughts? Does above make sense. Thanks. >> >> >> >> > > > My view was that it went like this : > > > main starts(args passed in) > int foo; > odp_init_t odp_data > odp_platform_init_t platform_data > > /*Process the arguments passed in as normal.*/ > loop { > Set variables that main will use itself i.e. if --use-foo is passed set > foo=true locally. > Prepare odp_data to pass things into the implementation that ODP > specifies, <- the internals of odp_data may be an argv string or a tidy > struct we define populated by an ODP helper > Prepare platform_data packaging any params required into the platform > specific struct. <- Have no idea what these are, platform needs to figure > this out > } > Not sure I understood your proposal/pseudo flow completely. = Are you suggesting processing argument twice one for platform_init second for odp thread? > call global init(odp_data, platform_data) with the above > > ........ > > /* Act on any remaining parameters such as foo. */ > if (foo) > > > -- > Mike Holmes > Linaro Technical Manager / Lead > LNG - ODP
On 4 September 2014 17:30, Santosh Shukla <santosh.shukla@linaro.org> wrote: > On 5 September 2014 00:13, Mike Holmes <mike.holmes@linaro.org> wrote: > > I am confused about the init sequence, I imagine this > > > > > > > > > > On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> > wrote: > >> > >> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) > >> <petri.savolainen@nsn.com> wrote: > >> >> Conti.. > >> >> > >> >> Root cause of my concern is not entirely specific to this patch. Its > >> >> more related to odp api initialization calling sequence in > >> >> application. From early days, we used to follow below step in > >> >> application - > >> >> > >> >> odp_global_init() -> which does almost everything for platform > >> >> initialization. > >> >> odp_shm_reserve() -> used for argument processing then > >> >> odp_create_thread etc... > >> >> > >> >> Now we found a need to introduce platform specific init hints, coming > >> >> from application. Therefore order of odp_xxx_init api calling > sequence > >> >> should change too. > >> >> > >> >> > >> >> Something like. > >> >> > >> >> Application "X" > >> >> > >> >> main () > >> >> { > >> >> > >> >> - First do argument processing. For that, call odp_shm_init() in > >> >> application and remove it from odp_global_init OR application has to > >> >> allocate memory(bad idea ... I guess). > >> > > >> > argX and argY can be allocated from stack or heap (malloc). Shared > >> > memory is not needed for those to be able to call odp_global_init(). > ODP > >> > will take copy of the argument, argX/Y can be destroyed after the > call. > >> > >> Not to *call odp_global_init* , It is to pass command line processed > >> input to odp_globa_init().. Are you suggesting calling of parse_args() > >> before and after odp_global_init()? > >> > >> > >> > > >> > After ODP init, you can allocate shm and copy args there if you need > to > >> > share those within your app. > >> > >> Same argument processed twice ? isn't it. > >> > >> > >> > >> > > >> > -Petri > >> > > >> >> > >> >> - Then call odp_global/platform_init(argX, argY); where arg -X : > >> >> global param, Y : platform centric stuff. > >> >> > >> >> ... Rest stuff as it is. > >> >> } > >> >> > >> >> > >> >> Any idea? thoughts? Does above make sense. Thanks. > >> >> > >> >> > > > > > > My view was that it went like this : > > > > > > main starts(args passed in) > > int foo; > > odp_init_t odp_data > > odp_platform_init_t platform_data > > > > /*Process the arguments passed in as normal.*/ > > loop { > > Set variables that main will use itself i.e. if --use-foo is passed > set > > foo=true locally. > > Prepare odp_data to pass things into the implementation that ODP > > specifies, <- the internals of odp_data may be an argv string or a tidy > > struct we define populated by an ODP helper > > Prepare platform_data packaging any params required into the platform > > specific struct. <- Have no idea what these are, platform needs to > figure > > this out > > } > > > > Not sure I understood your proposal/pseudo flow completely. > > = Are you suggesting processing argument twice one for platform_init > second for odp thread? > > no, all arguments into main are processed once up front, they are either 1. recorded with local variables as appropriate 2. stuffed into odp_init - maybe with the helper 3. stuffed into odp_platform_init however that platform specifies it should be done. > call global init(odp_data, platform_data) with the above > > > > ........ > > > > /* Act on any remaining parameters such as foo. */ > > if (foo) > > > > > > -- > > Mike Holmes > > Linaro Technical Manager / Lead > > LNG - ODP >
On 5 September 2014 03:08, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > On 4 September 2014 17:30, Santosh Shukla <santosh.shukla@linaro.org> wrote: >> >> On 5 September 2014 00:13, Mike Holmes <mike.holmes@linaro.org> wrote: >> > I am confused about the init sequence, I imagine this >> > >> > >> > >> > >> > On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> >> > wrote: >> >> >> >> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >> >> <petri.savolainen@nsn.com> wrote: >> >> >> Conti.. >> >> >> >> >> >> Root cause of my concern is not entirely specific to this patch. Its >> >> >> more related to odp api initialization calling sequence in >> >> >> application. From early days, we used to follow below step in >> >> >> application - >> >> >> >> >> >> odp_global_init() -> which does almost everything for platform >> >> >> initialization. >> >> >> odp_shm_reserve() -> used for argument processing then >> >> >> odp_create_thread etc... >> >> >> >> >> >> Now we found a need to introduce platform specific init hints, >> >> >> coming >> >> >> from application. Therefore order of odp_xxx_init api calling >> >> >> sequence >> >> >> should change too. >> >> >> >> >> >> >> >> >> Something like. >> >> >> >> >> >> Application "X" >> >> >> >> >> >> main () >> >> >> { >> >> >> >> >> >> - First do argument processing. For that, call odp_shm_init() in >> >> >> application and remove it from odp_global_init OR application has to >> >> >> allocate memory(bad idea ... I guess). >> >> > >> >> > argX and argY can be allocated from stack or heap (malloc). Shared >> >> > memory is not needed for those to be able to call odp_global_init(). >> >> > ODP >> >> > will take copy of the argument, argX/Y can be destroyed after the >> >> > call. >> >> >> >> Not to *call odp_global_init* , It is to pass command line processed >> >> input to odp_globa_init().. Are you suggesting calling of parse_args() >> >> before and after odp_global_init()? >> >> >> >> >> >> > >> >> > After ODP init, you can allocate shm and copy args there if you need >> >> > to >> >> > share those within your app. >> >> >> >> Same argument processed twice ? isn't it. >> >> >> >> >> >> >> >> > >> >> > -Petri >> >> > >> >> >> >> >> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >> >> >> global param, Y : platform centric stuff. >> >> >> >> >> >> ... Rest stuff as it is. >> >> >> } >> >> >> >> >> >> >> >> >> Any idea? thoughts? Does above make sense. Thanks. >> >> >> >> >> >> >> > >> > >> > My view was that it went like this : >> > >> > >> > main starts(args passed in) >> > int foo; >> > odp_init_t odp_data >> > odp_platform_init_t platform_data >> > >> > /*Process the arguments passed in as normal.*/ >> > loop { >> > Set variables that main will use itself i.e. if --use-foo is passed >> > set >> > foo=true locally. >> > Prepare odp_data to pass things into the implementation that ODP >> > specifies, <- the internals of odp_data may be an argv string or a tidy >> > struct we define populated by an ODP helper >> > Prepare platform_data packaging any params required into the platform >> > specific struct. <- Have no idea what these are, platform needs to >> > figure >> > this out >> > } >> > >> >> Not sure I understood your proposal/pseudo flow completely. >> >> = Are you suggesting processing argument twice one for platform_init >> second for odp thread? >> > no, all arguments into main are processed once up front, they are either > 1. recorded with local variables as appropriate > 2. stuffed into odp_init - maybe with the helper > 3. stuffed into odp_platform_init however that platform specifies it should > be done. > > Which mean ignore old way of argument processing using odp shmem? right. >> > call global init(odp_data, platform_data) with the above >> > >> > ........ >> > >> > /* Act on any remaining parameters such as foo. */ >> > if (foo) >> > >> > >> > -- >> > Mike Holmes >> > Linaro Technical Manager / Lead >> > LNG - ODP > > > > > -- > Mike Holmes > Linaro Technical Manager / Lead > LNG - ODP
Yes, but that may be my ignorance :) I don't know why the old way is needed now. On 4 September 2014 17:44, Santosh Shukla <santosh.shukla@linaro.org> wrote: > On 5 September 2014 03:08, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > > > > > On 4 September 2014 17:30, Santosh Shukla <santosh.shukla@linaro.org> > wrote: > >> > >> On 5 September 2014 00:13, Mike Holmes <mike.holmes@linaro.org> wrote: > >> > I am confused about the init sequence, I imagine this > >> > > >> > > >> > > >> > > >> > On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> > >> > wrote: > >> >> > >> >> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) > >> >> <petri.savolainen@nsn.com> wrote: > >> >> >> Conti.. > >> >> >> > >> >> >> Root cause of my concern is not entirely specific to this patch. > Its > >> >> >> more related to odp api initialization calling sequence in > >> >> >> application. From early days, we used to follow below step in > >> >> >> application - > >> >> >> > >> >> >> odp_global_init() -> which does almost everything for platform > >> >> >> initialization. > >> >> >> odp_shm_reserve() -> used for argument processing then > >> >> >> odp_create_thread etc... > >> >> >> > >> >> >> Now we found a need to introduce platform specific init hints, > >> >> >> coming > >> >> >> from application. Therefore order of odp_xxx_init api calling > >> >> >> sequence > >> >> >> should change too. > >> >> >> > >> >> >> > >> >> >> Something like. > >> >> >> > >> >> >> Application "X" > >> >> >> > >> >> >> main () > >> >> >> { > >> >> >> > >> >> >> - First do argument processing. For that, call odp_shm_init() in > >> >> >> application and remove it from odp_global_init OR application has > to > >> >> >> allocate memory(bad idea ... I guess). > >> >> > > >> >> > argX and argY can be allocated from stack or heap (malloc). Shared > >> >> > memory is not needed for those to be able to call > odp_global_init(). > >> >> > ODP > >> >> > will take copy of the argument, argX/Y can be destroyed after the > >> >> > call. > >> >> > >> >> Not to *call odp_global_init* , It is to pass command line processed > >> >> input to odp_globa_init().. Are you suggesting calling of > parse_args() > >> >> before and after odp_global_init()? > >> >> > >> >> > >> >> > > >> >> > After ODP init, you can allocate shm and copy args there if you > need > >> >> > to > >> >> > share those within your app. > >> >> > >> >> Same argument processed twice ? isn't it. > >> >> > >> >> > >> >> > >> >> > > >> >> > -Petri > >> >> > > >> >> >> > >> >> >> - Then call odp_global/platform_init(argX, argY); where arg -X : > >> >> >> global param, Y : platform centric stuff. > >> >> >> > >> >> >> ... Rest stuff as it is. > >> >> >> } > >> >> >> > >> >> >> > >> >> >> Any idea? thoughts? Does above make sense. Thanks. > >> >> >> > >> >> >> > >> > > >> > > >> > My view was that it went like this : > >> > > >> > > >> > main starts(args passed in) > >> > int foo; > >> > odp_init_t odp_data > >> > odp_platform_init_t platform_data > >> > > >> > /*Process the arguments passed in as normal.*/ > >> > loop { > >> > Set variables that main will use itself i.e. if --use-foo is passed > >> > set > >> > foo=true locally. > >> > Prepare odp_data to pass things into the implementation that ODP > >> > specifies, <- the internals of odp_data may be an argv string or a > tidy > >> > struct we define populated by an ODP helper > >> > Prepare platform_data packaging any params required into the > platform > >> > specific struct. <- Have no idea what these are, platform needs to > >> > figure > >> > this out > >> > } > >> > > >> > >> Not sure I understood your proposal/pseudo flow completely. > >> > >> = Are you suggesting processing argument twice one for platform_init > >> second for odp thread? > >> > > no, all arguments into main are processed once up front, they are either > > 1. recorded with local variables as appropriate > > 2. stuffed into odp_init - maybe with the helper > > 3. stuffed into odp_platform_init however that platform specifies it > should > > be done. > > > > > > Which mean ignore old way of argument processing using odp shmem? right. > > >> > call global init(odp_data, platform_data) with the above > >> > > >> > ........ > >> > > >> > /* Act on any remaining parameters such as foo. */ > >> > if (foo) > >> > > >> > > >> > -- > >> > Mike Holmes > >> > Linaro Technical Manager / Lead > >> > LNG - ODP > > > > > > > > > > -- > > Mike Holmes > > Linaro Technical Manager / Lead > > LNG - ODP >
On 5 September 2014 03:16, Mike Holmes <mike.holmes@linaro.org> wrote: > Yes, but that may be my ignorance :) I don't know why the old way is needed > now. Thats may be because I was confused and getting impression that expectation to do two level of argument processing although sounds stupid so thats why wanted to be loud and clear. I could send param specific dpdk-l2fwd patch to list now. Thanks for clarification. > > > On 4 September 2014 17:44, Santosh Shukla <santosh.shukla@linaro.org> wrote: >> >> On 5 September 2014 03:08, Mike Holmes <mike.holmes@linaro.org> wrote: >> > >> > >> > >> > On 4 September 2014 17:30, Santosh Shukla <santosh.shukla@linaro.org> >> > wrote: >> >> >> >> On 5 September 2014 00:13, Mike Holmes <mike.holmes@linaro.org> wrote: >> >> > I am confused about the init sequence, I imagine this >> >> > >> >> > >> >> > >> >> > >> >> > On 4 September 2014 04:40, Santosh Shukla <santosh.shukla@linaro.org> >> >> > wrote: >> >> >> >> >> >> On 4 September 2014 13:40, Savolainen, Petri (NSN - FI/Espoo) >> >> >> <petri.savolainen@nsn.com> wrote: >> >> >> >> Conti.. >> >> >> >> >> >> >> >> Root cause of my concern is not entirely specific to this patch. >> >> >> >> Its >> >> >> >> more related to odp api initialization calling sequence in >> >> >> >> application. From early days, we used to follow below step in >> >> >> >> application - >> >> >> >> >> >> >> >> odp_global_init() -> which does almost everything for platform >> >> >> >> initialization. >> >> >> >> odp_shm_reserve() -> used for argument processing then >> >> >> >> odp_create_thread etc... >> >> >> >> >> >> >> >> Now we found a need to introduce platform specific init hints, >> >> >> >> coming >> >> >> >> from application. Therefore order of odp_xxx_init api calling >> >> >> >> sequence >> >> >> >> should change too. >> >> >> >> >> >> >> >> >> >> >> >> Something like. >> >> >> >> >> >> >> >> Application "X" >> >> >> >> >> >> >> >> main () >> >> >> >> { >> >> >> >> >> >> >> >> - First do argument processing. For that, call odp_shm_init() in >> >> >> >> application and remove it from odp_global_init OR application has >> >> >> >> to >> >> >> >> allocate memory(bad idea ... I guess). >> >> >> > >> >> >> > argX and argY can be allocated from stack or heap (malloc). Shared >> >> >> > memory is not needed for those to be able to call >> >> >> > odp_global_init(). >> >> >> > ODP >> >> >> > will take copy of the argument, argX/Y can be destroyed after the >> >> >> > call. >> >> >> >> >> >> Not to *call odp_global_init* , It is to pass command line processed >> >> >> input to odp_globa_init().. Are you suggesting calling of >> >> >> parse_args() >> >> >> before and after odp_global_init()? >> >> >> >> >> >> >> >> >> > >> >> >> > After ODP init, you can allocate shm and copy args there if you >> >> >> > need >> >> >> > to >> >> >> > share those within your app. >> >> >> >> >> >> Same argument processed twice ? isn't it. >> >> >> >> >> >> >> >> >> >> >> >> > >> >> >> > -Petri >> >> >> > >> >> >> >> >> >> >> >> - Then call odp_global/platform_init(argX, argY); where arg -X : >> >> >> >> global param, Y : platform centric stuff. >> >> >> >> >> >> >> >> ... Rest stuff as it is. >> >> >> >> } >> >> >> >> >> >> >> >> >> >> >> >> Any idea? thoughts? Does above make sense. Thanks. >> >> >> >> >> >> >> >> >> >> > >> >> > >> >> > My view was that it went like this : >> >> > >> >> > >> >> > main starts(args passed in) >> >> > int foo; >> >> > odp_init_t odp_data >> >> > odp_platform_init_t platform_data >> >> > >> >> > /*Process the arguments passed in as normal.*/ >> >> > loop { >> >> > Set variables that main will use itself i.e. if --use-foo is passed >> >> > set >> >> > foo=true locally. >> >> > Prepare odp_data to pass things into the implementation that ODP >> >> > specifies, <- the internals of odp_data may be an argv string or a >> >> > tidy >> >> > struct we define populated by an ODP helper >> >> > Prepare platform_data packaging any params required into the >> >> > platform >> >> > specific struct. <- Have no idea what these are, platform needs to >> >> > figure >> >> > this out >> >> > } >> >> > >> >> >> >> Not sure I understood your proposal/pseudo flow completely. >> >> >> >> = Are you suggesting processing argument twice one for platform_init >> >> second for odp thread? >> >> >> > no, all arguments into main are processed once up front, they are either >> > 1. recorded with local variables as appropriate >> > 2. stuffed into odp_init - maybe with the helper >> > 3. stuffed into odp_platform_init however that platform specifies it >> > should >> > be done. >> > >> > >> >> Which mean ignore old way of argument processing using odp shmem? right. >> >> >> > call global init(odp_data, platform_data) with the above >> >> > >> >> > ........ >> >> > >> >> > /* Act on any remaining parameters such as foo. */ >> >> > if (foo) >> >> > >> >> > >> >> > -- >> >> > Mike Holmes >> >> > Linaro Technical Manager / Lead >> >> > LNG - ODP >> > >> > >> > >> > >> > -- >> > Mike Holmes >> > Linaro Technical Manager / Lead >> > LNG - ODP > > > > > -- > Mike Holmes > Linaro Technical Manager / Lead > LNG - ODP
On 09/04/2014 09:59 PM, Bill Fischofer wrote: > Interesting point about ODP helper routines being used to help construct > odp_global_init() parameters. Are we saying that what makes a helper a > helper (from an application standpoint) is that it is a routine that can > be called before odp_global_init() since it is a stand-alone function > that operates independently from the rest of the ODP framework? > > That might be a good distinction and I'll make a note of it in the API > Design Guidelines doc. We actually have two groups of helpers: - one that is ODP-independent and can be used before ODP init. - another one that uses ODP API and must be used after ODP init.
On 09/04/2014 10:19 PM, Mike Holmes wrote: > I think that is true by definition following Taras's logic from a thread > a few weeks ago. The rational was that that helpers are users of the API > to add higher functionality for an application to use, they don't help > the implementation. > > However that breaks IPC I think, I have not read the latest patch, but > that wants to use odph_ring.h - unless IPC is a helper, but I thought > it was part of ODP. I was thinking of helpers as a next layer on top of ODP which provides some additional functionality, but as Bill noted some helpers may not use ODP API and can be called even before ODP init. Maybe it worth to call such helpers in a special way to distinguish them. There is no restriction for helpers to be called from both sides: implementation and application. Like we have IP header structure used by linux-generic and applications.
On 5 September 2014 03:56, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 09/04/2014 10:19 PM, Mike Holmes wrote: > >> I think that is true by definition following Taras's logic from a thread >> a few weeks ago. The rational was that that helpers are users of the API >> to add higher functionality for an application to use, they don't help >> the implementation. >> >> However that breaks IPC I think, I have not read the latest patch, but >> that wants to use odph_ring.h - unless IPC is a helper, but I thought >> it was part of ODP. >> > > I was thinking of helpers as a next layer on top of ODP which provides > some additional functionality, but as Bill noted some helpers may not > use ODP API and can be called even before ODP init. Maybe it worth to > call such helpers in a special way to distinguish them. > I agree that pre and post gloabl_init ones should be easy to distinguish. > > There is no restriction for helpers to be called from both sides: > implementation and application. Like we have IP header structure used > by linux-generic and applications. >
diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c index 9fa9b37..ba7aa68 100644 --- a/example/generator/odp_generator.c +++ b/example/generator/odp_generator.c @@ -525,7 +525,7 @@ int main(int argc, char *argv[]) int core_count; /* Init ODP before calling anything else */ - if (odp_init_global()) { + if (odp_init_global(NULL, NULL)) { ODP_ERR("Error: ODP global init failed.\n"); exit(EXIT_FAILURE); } diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c index d74449a..f8d6729 100644 --- a/example/l2fwd/odp_l2fwd.c +++ b/example/l2fwd/odp_l2fwd.c @@ -323,7 +323,7 @@ int main(int argc, char *argv[]) odp_pktio_t pktio; /* Init ODP before calling anything else */ - if (odp_init_global()) { + if (odp_init_global(NULL, NULL)) { ODP_ERR("Error: ODP global init failed.\n"); exit(EXIT_FAILURE); } diff --git a/example/odp_example/odp_example.c b/example/odp_example/odp_example.c index f0bdf29..3d38ac7 100644 --- a/example/odp_example/odp_example.c +++ b/example/odp_example/odp_example.c @@ -951,7 +951,7 @@ int main(int argc, char *argv[]) memset(thread_tbl, 0, sizeof(thread_tbl)); - if (odp_init_global()) { + if (odp_init_global(NULL, NULL)) { printf("ODP global init failed.\n"); return -1; } diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c index f247bd0..f3543a0 100644 --- a/example/packet/odp_pktio.c +++ b/example/packet/odp_pktio.c @@ -311,7 +311,7 @@ int main(int argc, char *argv[]) int core_count; /* Init ODP before calling anything else */ - if (odp_init_global()) { + if (odp_init_global(NULL, NULL)) { ODP_ERR("Error: ODP global init failed.\n"); exit(EXIT_FAILURE); } diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c index bf1d7df..abf90aa 100644 --- a/example/timer/odp_timer_test.c +++ b/example/timer/odp_timer_test.c @@ -260,7 +260,7 @@ int main(int argc, char *argv[]) memset(thread_tbl, 0, sizeof(thread_tbl)); - if (odp_init_global()) { + if (odp_init_global(NULL, NULL)) { printf("ODP global init failed.\n"); return -1; } diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux-dpdk/Makefile.am index ff49b7d..48fc9d7 100644 --- a/platform/linux-dpdk/Makefile.am +++ b/platform/linux-dpdk/Makefile.am @@ -67,13 +67,15 @@ __LIB__libodp_la_SOURCES = \ odp_buffer.c \ odp_buffer_pool.c \ ../linux-generic/odp_coremask.c \ - odp_init.c \ + ../linux-generic/odp_init.c \ odp_linux.c \ odp_packet.c \ odp_packet_dpdk.c \ ../linux-generic/odp_packet_flags.c \ odp_packet_io.c \ ../linux-generic/odp_packet_socket.c \ + ../linux-generic/odp_crypto.c \ + odp_platform.c \ odp_queue.c \ ../linux-generic/odp_ring.c \ ../linux-generic/odp_rwlock.c \ diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux-dpdk/odp_platform.c similarity index 50% rename from platform/linux-dpdk/odp_init.c rename to platform/linux-dpdk/odp_platform.c index ecc2066..3162c05 100644 --- a/platform/linux-dpdk/odp_init.c +++ b/platform/linux-dpdk/odp_platform.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2013, Linaro Limited +/* Copyright (c) 2014, Linaro Limited * All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause @@ -9,7 +9,7 @@ #include <odp_debug.h> #include <odp_packet_dpdk.h> -int odp_init_dpdk(void) +int odp_init_platform(odp_global_platform_init_t *platform_params ODP_UNUSED) { int test_argc = 5; char *test_argv[6]; @@ -49,65 +49,3 @@ int odp_init_dpdk(void) return 0; } - -int odp_init_global(void) -{ - odp_thread_init_global(); - - odp_system_info_init(); - - if (odp_init_dpdk()) { - ODP_ERR("ODP dpdk init failed.\n"); - return -1; - } - - if (odp_shm_init_global()) { - ODP_ERR("ODP shm init failed.\n"); - return -1; - } - - if (odp_buffer_pool_init_global()) { - ODP_ERR("ODP buffer pool init failed.\n"); - return -1; - } - - if (odp_queue_init_global()) { - ODP_ERR("ODP queue init failed.\n"); - return -1; - } - - if (odp_schedule_init_global()) { - ODP_ERR("ODP schedule init failed.\n"); - return -1; - } - - if (odp_pktio_init_global()) { - ODP_ERR("ODP packet io init failed.\n"); - return -1; - } - - if (odp_timer_init_global()) { - ODP_ERR("ODP timer init failed.\n"); - return -1; - } - - return 0; -} - - -int odp_init_local(int thr_id) -{ - odp_thread_init_local(thr_id); - - if (odp_pktio_init_local()) { - ODP_ERR("ODP packet io local init failed.\n"); - return -1; - } - - if (odp_schedule_init_local()) { - ODP_ERR("ODP schedule local init failed.\n"); - return -1; - } - - return 0; -} diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index f4dfdc1..c4aae9d 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -62,6 +62,7 @@ __LIB__libodp_la_SOURCES = \ odp_packet_flags.c \ odp_packet_io.c \ odp_packet_socket.c \ + odp_platform.c \ odp_queue.c \ odp_ring.c \ odp_rwlock.c \ diff --git a/platform/linux-generic/include/api/odp.h b/platform/linux-generic/include/api/odp.h index 0ee3faf..99e2ae0 100644 --- a/platform/linux-generic/include/api/odp.h +++ b/platform/linux-generic/include/api/odp.h @@ -48,6 +48,7 @@ extern "C" { #include <odp_packet.h> #include <odp_packet_flags.h> #include <odp_packet_io.h> +#include <odp_crypto.h> #ifdef __cplusplus } diff --git a/platform/linux-generic/include/api/odp_init.h b/platform/linux-generic/include/api/odp_init.h index 490324a..8cdce46 100644 --- a/platform/linux-generic/include/api/odp_init.h +++ b/platform/linux-generic/include/api/odp_init.h @@ -22,24 +22,42 @@ extern "C" { #include <odp_std_types.h> - - +/** ODP initialisation data. + * Data that is required to initialize the ODP API with the + * application specific data such as specifying a logging callback, the log + * level etc. + */ +typedef struct odp_global_init_t { +} odp_global_init_t; + +/** ODP platform initialization data. + * @note ODP API does nothing with this data. It is the underlying + * implementation requires it and any data passed here is not portable. + * It is required that the application takes care of identifying and + * passing any required platform specific data. + */ +typedef struct odp_global_platform_init_t { +} odp_global_platform_init_t; /** - * Perform global ODP initalisation. - * - * This function must be called once before calling - * any other ODP API functions. + * Perform global ODP initialisation. * + * This function must be called once before calling any other ODP API + * functions. + * @param[in] params Those parameters that are interpreted by the ODP API + * @param[in] platform_params Those parameters that are passed without + * interpretation by the ODP API to the implementation. * @return 0 if successful */ -int odp_init_global(void); +int odp_init_global(odp_global_init_t *params, + odp_global_platform_init_t *platform_params); /** - * Perform thread local ODP initalisation. + * Perform thread local ODP initialisation. * * All threads must call this function before calling + * * any other ODP API functions. * @param thr_id Thread id * @return 0 if successful diff --git a/platform/linux-generic/include/odp_internal.h b/platform/linux-generic/include/odp_internal.h index aa79493..6c71fc9 100644 --- a/platform/linux-generic/include/odp_internal.h +++ b/platform/linux-generic/include/odp_internal.h @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-3-Clause */ +#include <odp_init.h> /** * @file @@ -42,6 +43,8 @@ int odp_schedule_init_local(void); int odp_timer_init_global(void); int odp_timer_disarm_all(void); +int odp_init_platform(odp_global_platform_init_t *platform_params); + #ifdef __cplusplus } #endif diff --git a/platform/linux-generic/odp_init.c b/platform/linux-generic/odp_init.c index 5b7e192..f595def 100644 --- a/platform/linux-generic/odp_init.c +++ b/platform/linux-generic/odp_init.c @@ -8,13 +8,18 @@ #include <odp_internal.h> #include <odp_debug.h> - -int odp_init_global(void) +int odp_init_global(odp_global_init_t *params ODP_UNUSED, + odp_global_platform_init_t *platform_params ODP_UNUSED) { odp_thread_init_global(); odp_system_info_init(); + if (odp_init_platform(platform_params)) { + ODP_ERR("ODP platform init failed.\n"); + return -1; + } + if (odp_shm_init_global()) { ODP_ERR("ODP shm init failed.\n"); return -1; diff --git a/platform/linux-generic/odp_platform.c b/platform/linux-generic/odp_platform.c new file mode 100644 index 0000000..a9efa7f --- /dev/null +++ b/platform/linux-generic/odp_platform.c @@ -0,0 +1,14 @@ +/* Copyright (c) 2014, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include <odp_init.h> +#include <odp_internal.h> +#include <odp_debug.h> + +int odp_init_platform(odp_global_platform_init_t *platform_params ODP_UNUSED) +{ + return 0; +} diff --git a/platform/linux-keystone2/Makefile.am b/platform/linux-keystone2/Makefile.am index 8e5fbb3..e64b2d8 100644 --- a/platform/linux-keystone2/Makefile.am +++ b/platform/linux-keystone2/Makefile.am @@ -70,13 +70,15 @@ __LIB__libodp_la_SOURCES = \ odp_buffer.c \ odp_buffer_pool.c \ ../linux-generic/odp_coremask.c \ - odp_init.c \ + ../linux-generic/odp_init.c \ ../linux-generic/odp_linux.c \ odp_packet.c \ ../linux-generic/odp_packet_flags.c \ odp_packet_io.c \ ../linux-generic/odp_packet_socket.c \ + odp_platform.c \ odp_queue.c \ + ../linux-generic/odp_crypto.c \ ../linux-generic/odp_ring.c \ ../linux-generic/odp_rwlock.c \ ../linux-generic/odp_schedule.c \ diff --git a/platform/linux-keystone2/odp_init.c b/platform/linux-keystone2/odp_platform.c similarity index 72% rename from platform/linux-keystone2/odp_init.c rename to platform/linux-keystone2/odp_platform.c index f832551..6386610 100644 --- a/platform/linux-keystone2/odp_init.c +++ b/platform/linux-keystone2/odp_platform.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2013, Linaro Limited +/* Copyright (c) 2014, Linaro Limited * All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause @@ -15,17 +15,19 @@ #include <odp_packet_internal.h> /* - * Make region_configs[] global, because hw_config is saved in - * ti_em_rh_init_global() and it references region_configs[]. - */ + * * Make region_configs[] global, because hw_config is saved in + * * ti_em_rh_init_global() and it references region_configs[]. + * */ static ti_em_osal_hw_region_config_t region_configs[TI_ODP_REGION_NUM]; -static int ti_init_hw_config(void) +int odp_init_platform(odp_global_platform_init_t *platform_params ODP_UNUSED) { ti_em_rh_hw_config_t hw_config; ti_em_osal_hw_region_config_t *reg_config; memset(&hw_config, 0, sizeof(ti_em_rh_hw_config_t)); + ti_em_osal_core_init_global(); + /* Set ODP initialization parameters */ hw_config.private_free_queue_idx = MY_EM_PRIVATE_FREE_QUEUE_IDX; hw_config.hw_queue_base_idx = MY_EM_SCHED_QUEUE_IDX; @@ -89,65 +91,3 @@ static int ti_init_hw_config(void) return 0; } - -int odp_init_global(void) -{ - odp_thread_init_global(); - - odp_system_info_init(); - - ti_em_osal_core_init_global(); - ti_init_hw_config(); - - if (odp_shm_init_global()) { - ODP_ERR("ODP shm init failed.\n"); - return -1; - } - - if (odp_buffer_pool_init_global()) { - ODP_ERR("ODP buffer pool init failed.\n"); - return -1; - } - - if (odp_queue_init_global()) { - ODP_ERR("ODP queue init failed.\n"); - return -1; - } - - if (odp_schedule_init_global()) { - ODP_ERR("ODP schedule init failed.\n"); - return -1; - } - - if (odp_pktio_init_global()) { - ODP_ERR("ODP packet io init failed.\n"); - return -1; - } - - if (odp_timer_init_global()) { - ODP_ERR("ODP timer init failed.\n"); - return -1; - } - - return 0; -} - - -int odp_init_local(int thr_id) -{ - odp_thread_init_local(thr_id); - - ti_em_rh_init_local(); - - if (odp_pktio_init_local()) { - ODP_ERR("ODP packet io local init failed.\n"); - return -1; - } - - if (odp_schedule_init_local()) { - ODP_ERR("ODP schedule local init failed.\n"); - return -1; - } - - return 0; -} diff --git a/test/api_test/odp_common.c b/test/api_test/odp_common.c index 89ebd2d..fc583c0 100644 --- a/test/api_test/odp_common.c +++ b/test/api_test/odp_common.c @@ -56,7 +56,7 @@ int odp_test_global_init(void) { memset(thread_tbl, 0, sizeof(thread_tbl)); - if (odp_init_global()) { + if (odp_init_global(NULL, NULL)) { ODP_ERR("ODP global init failed.\n"); return -1; }
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- v2: Factor out odp_platform_init so that odp_global_init is only defined for linux_generic and resued. v3: Use -M to simplify the diff Fix missing doxygen comments v4: Improve the documentation. v5: rename typedef struct names This touches all platforms to get crypto working with generic init, DPDK should be using generic crypto but obviously KS2 will replace this. A platform may replace global init entirely but this patch starts out assuming that the sub module inits can be called from a central global_init as a starting point. example/generator/odp_generator.c | 2 +- example/l2fwd/odp_l2fwd.c | 2 +- example/odp_example/odp_example.c | 2 +- example/packet/odp_pktio.c | 2 +- example/timer/odp_timer_test.c | 2 +- platform/linux-dpdk/Makefile.am | 4 +- platform/linux-dpdk/{odp_init.c => odp_platform.c} | 66 +------------------ platform/linux-generic/Makefile.am | 1 + platform/linux-generic/include/api/odp.h | 1 + platform/linux-generic/include/api/odp_init.h | 34 +++++++--- platform/linux-generic/include/odp_internal.h | 3 + platform/linux-generic/odp_init.c | 9 ++- platform/linux-generic/odp_platform.c | 14 ++++ platform/linux-keystone2/Makefile.am | 4 +- .../linux-keystone2/{odp_init.c => odp_platform.c} | 74 ++-------------------- test/api_test/odp_common.c | 2 +- 16 files changed, 73 insertions(+), 149 deletions(-) rename platform/linux-dpdk/{odp_init.c => odp_platform.c} (50%) create mode 100644 platform/linux-generic/odp_platform.c rename platform/linux-keystone2/{odp_init.c => odp_platform.c} (72%)