Re: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add option to enable and disable ipc pktio
On 02/18/16 12:06, Savolainen, Petri (Nokia - FI/Espoo) wrote: -Original Message- From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Maxim Uvarov Sent: Thursday, February 18, 2016 10:45 AM To: Elo, Matias (Nokia - FI/Espoo) <matias@nokia.com>; lng- o...@lists.linaro.org Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add option to enable and disable ipc pktio Hello Matias, thanks for comments will send v4. During update I thinge we can not use ODP_ prefix here: +#ifdef ODP_PKTIO_IPC _pktio_ops, +#endif as well as in: +#ifdef ODP_PKTIO_DPDK +#endif Because it might be confusing other people thinking that it's related to some odp API. Let's use simple PKTIO_IPC, PKTIO_DPDK, PKTIO_NETMAP and etc. Agree? I think it's better to prefix. Could use _ODP_PKTIO_IPC, ... so that it's clearly internal to the implementation. -Petri I already sent v4. I can change it to _ODP_PKTIO_NAME on apply. Maxim. ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add option to enable and disable ipc pktio
> -Original Message- > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT > Maxim Uvarov > Sent: Thursday, February 18, 2016 10:45 AM > To: Elo, Matias (Nokia - FI/Espoo) <matias@nokia.com>; lng- > o...@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add > option to enable and disable ipc pktio > > Hello Matias, > > thanks for comments will send v4. > > During update I thinge we can not use ODP_ prefix here: > +#ifdef ODP_PKTIO_IPC > _pktio_ops, > +#endif > > as well as in: > > +#ifdef ODP_PKTIO_DPDK > > +#endif > > Because it might be confusing other people thinking that it's related to > some odp API. > > Let's use simple PKTIO_IPC, PKTIO_DPDK, PKTIO_NETMAP and etc. Agree? I think it's better to prefix. Could use _ODP_PKTIO_IPC, ... so that it's clearly internal to the implementation. -Petri ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add option to enable and disable ipc pktio
Hello Matias, thanks for comments will send v4. During update I thinge we can not use ODP_ prefix here: +#ifdef ODP_PKTIO_IPC _pktio_ops, +#endif as well as in: +#ifdef ODP_PKTIO_DPDK +#endif Because it might be confusing other people thinking that it's related to some odp API. Let's use simple PKTIO_IPC, PKTIO_DPDK, PKTIO_NETMAP and etc. Agree? Maxim. On 02/18/16 11:26, Elo, Matias (Nokia - FI/Espoo) wrote: Comments below. -Matias -Original Message- From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Maxim Uvarov Sent: Wednesday, February 17, 2016 9:43 AM To: lng-odp@lists.linaro.org Subject: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add option to enable and disable ipc pktio With more options to enable/disable features it's more easy to isolate and debug problem if any. For now I have suspitious that in CI make check runs in parallel with other make checks which share the same pool memory. I.e. one process can corrupt memory for other process. Moving IPC to option it will be easy to debug that. Signed-off-by: Maxim Uvarov--- configure.ac| 3 ++- platform/linux-generic/m4/configure.m4 | 1 + platform/linux-generic/m4/odp_ipc.m4| 29 + platform/linux-generic/odp_pool.c | 4 platform/linux-generic/pktio/io_ops.c | 2 ++ platform/linux-generic/test/Makefile.am | 7 +-- 6 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 platform/linux-generic/m4/odp_ipc.m4 diff --git a/configure.ac b/configure.ac index 257f8c3..200d565 100644 --- a/configure.ac +++ b/configure.ac @@ -85,6 +85,7 @@ AC_SUBST([platform_with_platform_test], ["platform/${with_platform}/test"]) # Prepare default values for platform specific optional features ## netmap_support=no +pktio_ipc_support=no ## # Run platform specific checks and settings @@ -102,7 +103,7 @@ fi ## AM_CONDITIONAL([netmap_support], [test x$netmap_support = xyes ]) AM_CONDITIONAL([HAVE_PCAP], [test $have_pcap = yes]) - +AM_CONDITIONAL([pktio_ipc_support], [test $pktio_ipc_support = yes]) AC_ARG_WITH([sdk-install-path], AC_HELP_STRING([--with-sdk-install-path=DIR path to external libs and headers], diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux- generic/m4/configure.m4 index 2ac4799..b8c1c52 100644 --- a/platform/linux-generic/m4/configure.m4 +++ b/platform/linux-generic/m4/configure.m4 @@ -21,6 +21,7 @@ m4_include([platform/linux-generic/m4/odp_openssl.m4]) m4_include([platform/linux-generic/m4/odp_netmap.m4]) m4_include([platform/linux-generic/m4/odp_dpdk.m4]) m4_include([platform/linux-generic/m4/odp_pcap.m4]) +m4_include([platform/linux-generic/m4/odp_ipc.m4]) AC_CONFIG_FILES([platform/linux-generic/Makefile platform/linux-generic/test/Makefile diff --git a/platform/linux-generic/m4/odp_ipc.m4 b/platform/linux- generic/m4/odp_ipc.m4 new file mode 100644 index 000..30f869d --- /dev/null +++ b/platform/linux-generic/m4/odp_ipc.m4 @@ -0,0 +1,29 @@ +# # +# Enable IPC pktio support +# # +AC_ARG_ENABLE([pktio_ipc_support], +[ --enable-pktio_ipc-support include ipc IO support], +[if test x$enableval = xyes; then + pktio_ipc_support=yes +fi]) + +# # +# Save and set temporary compilation flags +# # +OLD_CPPFLAGS=$CPPFLAGS +CPPFLAGS="$AM_CPPFLAGS $CPPFLAGS" Is this required as no flags are actually modified? + +# # +# Check for IPC pktio availability Nothing is actually checked here. +# # +if test x$pktio_ipc_support = xyes +then +ODP_CFLAGS="$ODP_CFLAGS -DODP_PKTIO_IPC" +else +pktio_ipc_support=no +fi No need for else statement. Could this be done in AC_ARG_ENABLE? + +# # +# Restore old saved variables +# # +CPPFLAGS=$OLD_CPPFLAGS Same thing as above. diff --git a/platform/linux-generic/odp_pool.c b/platform/linux- generic/odp_pool.c index 78ccc0f..9eaa0c8 100644 --- a/platform/linux-generic/odp_pool.c +++ b/platform/linux-generic/odp_pool.c @@ -421,7 +421,11 @@ odp_pool_t _pool_create(const char *name, odp_pool_t odp_pool_create(const char *name,
Re: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add option to enable and disable ipc pktio
Comments below. -Matias > -Original Message- > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Maxim > Uvarov > Sent: Wednesday, February 17, 2016 9:43 AM > To: lng-odp@lists.linaro.org > Subject: [lng-odp] [API-NEXT PATCHv3 1/2] linux-generic: pktio: add option to > enable and disable ipc pktio > > With more options to enable/disable features it's more easy to isolate > and debug problem if any. For now I have suspitious that in CI make check > runs in parallel with other make checks which share the same pool memory. > I.e. one process can corrupt memory for other process. Moving IPC to option > it will be easy to debug that. > > Signed-off-by: Maxim Uvarov> --- > configure.ac| 3 ++- > platform/linux-generic/m4/configure.m4 | 1 + > platform/linux-generic/m4/odp_ipc.m4| 29 > + > platform/linux-generic/odp_pool.c | 4 > platform/linux-generic/pktio/io_ops.c | 2 ++ > platform/linux-generic/test/Makefile.am | 7 +-- > 6 files changed, 43 insertions(+), 3 deletions(-) > create mode 100644 platform/linux-generic/m4/odp_ipc.m4 > > diff --git a/configure.ac b/configure.ac > index 257f8c3..200d565 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -85,6 +85,7 @@ AC_SUBST([platform_with_platform_test], > ["platform/${with_platform}/test"]) > # Prepare default values for platform specific optional features > > ## > > netmap_support=no > +pktio_ipc_support=no > > > ## > > # Run platform specific checks and settings > @@ -102,7 +103,7 @@ fi > > ## > > AM_CONDITIONAL([netmap_support], [test x$netmap_support = xyes ]) > AM_CONDITIONAL([HAVE_PCAP], [test $have_pcap = yes]) > - > +AM_CONDITIONAL([pktio_ipc_support], [test $pktio_ipc_support = yes]) > > AC_ARG_WITH([sdk-install-path], > AC_HELP_STRING([--with-sdk-install-path=DIR path to external libs and > headers], > diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux- > generic/m4/configure.m4 > index 2ac4799..b8c1c52 100644 > --- a/platform/linux-generic/m4/configure.m4 > +++ b/platform/linux-generic/m4/configure.m4 > @@ -21,6 +21,7 @@ m4_include([platform/linux-generic/m4/odp_openssl.m4]) > m4_include([platform/linux-generic/m4/odp_netmap.m4]) > m4_include([platform/linux-generic/m4/odp_dpdk.m4]) > m4_include([platform/linux-generic/m4/odp_pcap.m4]) > +m4_include([platform/linux-generic/m4/odp_ipc.m4]) > > AC_CONFIG_FILES([platform/linux-generic/Makefile >platform/linux-generic/test/Makefile > diff --git a/platform/linux-generic/m4/odp_ipc.m4 b/platform/linux- > generic/m4/odp_ipc.m4 > new file mode 100644 > index 000..30f869d > --- /dev/null > +++ b/platform/linux-generic/m4/odp_ipc.m4 > @@ -0,0 +1,29 @@ > +# > # > +# Enable IPC pktio support > +# > # > +AC_ARG_ENABLE([pktio_ipc_support], > +[ --enable-pktio_ipc-support include ipc IO support], > +[if test x$enableval = xyes; then > + pktio_ipc_support=yes > +fi]) > + > +# > # > +# Save and set temporary compilation flags > +# > # > +OLD_CPPFLAGS=$CPPFLAGS > +CPPFLAGS="$AM_CPPFLAGS $CPPFLAGS" Is this required as no flags are actually modified? > + > +# > # > +# Check for IPC pktio availability Nothing is actually checked here. > +# > # > +if test x$pktio_ipc_support = xyes > +then > +ODP_CFLAGS="$ODP_CFLAGS -DODP_PKTIO_IPC" > +else > +pktio_ipc_support=no > +fi No need for else statement. Could this be done in AC_ARG_ENABLE? > + > +# > # > +# Restore old saved variables > +# > # > +CPPFLAGS=$OLD_CPPFLAGS Same thing as above. > diff --git a/platform/linux-generic/odp_pool.c b/platform/linux- > generic/odp_pool.c > index 78ccc0f..9eaa0c8 100644 > --- a/platform/linux-generic/odp_pool.c > +++ b/platform/linux-generic/odp_pool.c > @@ -421,7 +421,11 @@ odp_pool_t _pool_create(const char *name, > odp_pool_t odp_pool_create(const char *name, > odp_pool_param_t *params) > { > +#ifdef ODP_PKTIO_IPC > return _pool_create(name, params, ODP_SHM_PROC); > +#else > + return _pool_create(name, params, 0); > +#endif > } > > odp_pool_t odp_pool_lookup(const char *name) >