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

>
>
> > -----Original Message-----
> > From: [email protected] [mailto:lng-odp-
> > [email protected]] On Behalf Of ext Mike Holmes
> > Sent: Tuesday, September 02, 2014 9:06 PM
> > To: [email protected]
> > Subject: [lng-odp] [PATCH v5] Add-global_init-paramiters
> >
> > Signed-off-by: Mike Holmes <[email protected]>
> > ---
> >
> > 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
>



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

Reply via email to