[dpdk-dev] app/bbdev: dynamic lib support

Message ID 20180404140602.9344-8-kamilx.chalupnik@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Pablo de Lara Guarch
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Kamil Chalupnik April 4, 2018, 2:06 p.m. UTC
  From: "Chalupnik, KamilX" <kamilx.chalupnik@intel.com>

Support for linking with dynamic library added in
Baseband Device test application

Signed-off-by: KamilX Chalupnik <kamilx.chalupnik@intel.com>
---
 app/test-bbdev/Makefile        | 4 ++++
 doc/guides/tools/testbbdev.rst | 7 +++++++
 2 files changed, 11 insertions(+)
  

Comments

Mokhtar, Amr April 17, 2018, 5:01 p.m. UTC | #1
> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Wednesday 4 April 2018 15:06
> To: dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> <kamilx.chalupnik@intel.com>
> Subject: [PATCH] app/bbdev: dynamic lib support
> 
> From: "Chalupnik, KamilX" <kamilx.chalupnik@intel.com>
> 
> Support for linking with dynamic library added in
> Baseband Device test application
> 
> Signed-off-by: KamilX Chalupnik <kamilx.chalupnik@intel.com>
> ---

Acked-by: Amr Mokhtar <amr.mokhtar@intel.com>
  
De Lara Guarch, Pablo April 24, 2018, 2:18 p.m. UTC | #2
Hi Kamil,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of KamilX Chalupnik
> Sent: Wednesday, April 4, 2018 3:06 PM
> To: dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> <kamilx.chalupnik@intel.com>
> Subject: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> 
> From: "Chalupnik, KamilX" <kamilx.chalupnik@intel.com>
> 
> Support for linking with dynamic library added in Baseband Device test
> application
> 
> Signed-off-by: KamilX Chalupnik <kamilx.chalupnik@intel.com>
> ---
>  app/test-bbdev/Makefile        | 4 ++++
>  doc/guides/tools/testbbdev.rst | 7 +++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/app/test-bbdev/Makefile b/app/test-bbdev/Makefile index
> 6da0c8e..26c9a4b 100644
> --- a/app/test-bbdev/Makefile
> +++ b/app/test-bbdev/Makefile
> @@ -22,4 +22,8 @@ SRCS-$(CONFIG_RTE_TEST_BBDEV) +=
> test_bbdev_vector.c
> 
>  LDLIBS += -lm
> 
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_BBDEV_TURBO_SW),y)
> +LDLIBS += -lrte_pmd_bbdev_turbo_sw
> +endif

This is already done in mk/rte.app.mk, so it should not be needed.

> +
>  include $(RTE_SDK)/mk/rte.app.mk
> diff --git a/doc/guides/tools/testbbdev.rst b/doc/guides/tools/testbbdev.rst
> index 2ccc646..8a13cbd 100644
> --- a/doc/guides/tools/testbbdev.rst
> +++ b/doc/guides/tools/testbbdev.rst
> @@ -36,6 +36,13 @@ The user must have all libraries, modules, updates and
> compilers installed  in the system prior to this, as described in the earlier
> chapters in this  Getting Started Guide.
> 
> +Compiling the Application with DPDK built as shared library
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Setting flag in config/common_base file:
> +
> +   ``CONFIG_RTE_BUILD_SHARED_LIB=y``

I don't think this is necessary to document.
This is global for all applications and libraries in DPDK, and none of them document individually this.

In a nutshell, I think this patch is not necessary.

Thanks,
Pablo
  
De Lara Guarch, Pablo April 25, 2018, 4:10 p.m. UTC | #3
Hi Kamil,

> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Wednesday, April 25, 2018 9:43 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> 
> Hi Pablo
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Tuesday, April 24, 2018 4:18 PM
> > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> > <kamilx.chalupnik@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> >
> > Hi Kamil,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of KamilX
> > > Chalupnik
> > > Sent: Wednesday, April 4, 2018 3:06 PM
> > > To: dev@dpdk.org
> > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> > > <kamilx.chalupnik@intel.com>
> > > Subject: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > >
> > > From: "Chalupnik, KamilX" <kamilx.chalupnik@intel.com>
> > >
> > > Support for linking with dynamic library added in Baseband Device
> > > test application
> > >
> > > Signed-off-by: KamilX Chalupnik <kamilx.chalupnik@intel.com>
> > > ---
> > >  app/test-bbdev/Makefile        | 4 ++++
> > >  doc/guides/tools/testbbdev.rst | 7 +++++++
> > >  2 files changed, 11 insertions(+)
> > >
> > > diff --git a/app/test-bbdev/Makefile b/app/test-bbdev/Makefile index
> > > 6da0c8e..26c9a4b 100644
> > > --- a/app/test-bbdev/Makefile
> > > +++ b/app/test-bbdev/Makefile
> > > @@ -22,4 +22,8 @@ SRCS-$(CONFIG_RTE_TEST_BBDEV) +=
> > test_bbdev_vector.c
> > >
> > >  LDLIBS += -lm
> > >
> > > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_BBDEV_TURBO_SW),y)
> > > +LDLIBS += -lrte_pmd_bbdev_turbo_sw
> > > +endif
> >
> > This is already done in mk/rte.app.mk, so it should not be needed.
> >
> 
> We need this here when DPDK is built as dynamic library.
> In rte.app.mk lrte_pmd_bbdev_turbo_sw is added to LDLIBS when
> CONFIG_RTE_BUILD_SHARED_LIB is not set.

In that case, you need to use the EAL option "-d" with  the .so library.

./testbbdev -d build/lib/librte_pmd_bbdev_turbo_sw.so --vdev="turbo_sw"

Btw, now that I have seen the name of the vdev, I think you should rename to "baseband_turbo_sw".
Also, NULL PMD should be renamed to "baseband_null".

Thanks,
Pablo
  
De Lara Guarch, Pablo April 26, 2018, 9:47 a.m. UTC | #4
Hi Kamil,

> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Thursday, April 26, 2018 10:42 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> 
> Hi Pablo,
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Wednesday, April 25, 2018 6:11 PM
> > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> >
> > Hi Kamil,
> >
> > > -----Original Message-----
> > > From: Chalupnik, KamilX
> > > Sent: Wednesday, April 25, 2018 9:43 AM
> > > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> > > dev@dpdk.org
> > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > >
> > > Hi Pablo
> > >
> > > > -----Original Message-----
> > > > From: De Lara Guarch, Pablo
> > > > Sent: Tuesday, April 24, 2018 4:18 PM
> > > > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> > > > <kamilx.chalupnik@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > > >
> > > > Hi Kamil,
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of KamilX
> > > > > Chalupnik
> > > > > Sent: Wednesday, April 4, 2018 3:06 PM
> > > > > To: dev@dpdk.org
> > > > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> > > > > <kamilx.chalupnik@intel.com>
> > > > > Subject: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > > > >
> > > > > From: "Chalupnik, KamilX" <kamilx.chalupnik@intel.com>
> > > > >
> > > > > Support for linking with dynamic library added in Baseband
> > > > > Device test application
> > > > >
> > > > > Signed-off-by: KamilX Chalupnik <kamilx.chalupnik@intel.com>
> > > > > ---
> > > > >  app/test-bbdev/Makefile        | 4 ++++
> > > > >  doc/guides/tools/testbbdev.rst | 7 +++++++
> > > > >  2 files changed, 11 insertions(+)
> > > > >
> > > > > diff --git a/app/test-bbdev/Makefile b/app/test-bbdev/Makefile
> > > > > index 6da0c8e..26c9a4b 100644
> > > > > --- a/app/test-bbdev/Makefile
> > > > > +++ b/app/test-bbdev/Makefile
> > > > > @@ -22,4 +22,8 @@ SRCS-$(CONFIG_RTE_TEST_BBDEV) +=
> > > > test_bbdev_vector.c
> > > > >
> > > > >  LDLIBS += -lm
> > > > >
> > > > > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_BBDEV_TURBO_SW),y)
> > > > > +LDLIBS += -lrte_pmd_bbdev_turbo_sw endif
> > > >
> > > > This is already done in mk/rte.app.mk, so it should not be needed.
> > > >
> > >
> > > We need this here when DPDK is built as dynamic library.
> > > In rte.app.mk lrte_pmd_bbdev_turbo_sw is added to LDLIBS when
> > > CONFIG_RTE_BUILD_SHARED_LIB is not set.
> >
> > In that case, you need to use the EAL option "-d" with  the .so library.
> >
> > ./testbbdev -d build/lib/librte_pmd_bbdev_turbo_sw.so --vdev="turbo_sw"
> 
> I thought it is fine as long as other test applications works in that way, e.g. test-
> crypto-perf, test-pmd

I think some of them are including the PMDs like what you are doing because these have additional API.
In case of Turbo, it does not have it, and therefore, it is just another driver that can be plugged it in through
"-d" option (notice that most of the PMDs are not doing what you are trying to do, which means that it is not required).

Thanks!
Pablo
  
De Lara Guarch, Pablo April 26, 2018, 11:28 a.m. UTC | #5
> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Thursday, April 26, 2018 10:42 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> 
> Hi Pablo,
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Wednesday, April 25, 2018 6:11 PM
> > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> >
> > Hi Kamil,
> >
> > > -----Original Message-----
> > > From: Chalupnik, KamilX
> > > Sent: Wednesday, April 25, 2018 9:43 AM
> > > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> > > dev@dpdk.org
> > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > >
> > > Hi Pablo
> > >
> > > > -----Original Message-----
> > > > From: De Lara Guarch, Pablo
> > > > Sent: Tuesday, April 24, 2018 4:18 PM
> > > > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> > > > <kamilx.chalupnik@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > > >
> > > > Hi Kamil,
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of KamilX
> > > > > Chalupnik
> > > > > Sent: Wednesday, April 4, 2018 3:06 PM
> > > > > To: dev@dpdk.org
> > > > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; Chalupnik, KamilX
> > > > > <kamilx.chalupnik@intel.com>
> > > > > Subject: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > > > >
> > > > > From: "Chalupnik, KamilX" <kamilx.chalupnik@intel.com>
> > > > >
> > > > > Support for linking with dynamic library added in Baseband
> > > > > Device test application
> > > > >
> > > > > Signed-off-by: KamilX Chalupnik <kamilx.chalupnik@intel.com>
> > > > > ---
> > > > >  app/test-bbdev/Makefile        | 4 ++++
> > > > >  doc/guides/tools/testbbdev.rst | 7 +++++++
> > > > >  2 files changed, 11 insertions(+)
> > > > >
> > > > > diff --git a/app/test-bbdev/Makefile b/app/test-bbdev/Makefile
> > > > > index 6da0c8e..26c9a4b 100644
> > > > > --- a/app/test-bbdev/Makefile
> > > > > +++ b/app/test-bbdev/Makefile
> > > > > @@ -22,4 +22,8 @@ SRCS-$(CONFIG_RTE_TEST_BBDEV) +=
> > > > test_bbdev_vector.c
> > > > >
> > > > >  LDLIBS += -lm
> > > > >
> > > > > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_BBDEV_TURBO_SW),y)
> > > > > +LDLIBS += -lrte_pmd_bbdev_turbo_sw endif
> > > >
> > > > This is already done in mk/rte.app.mk, so it should not be needed.
> > > >
> > >
> > > We need this here when DPDK is built as dynamic library.
> > > In rte.app.mk lrte_pmd_bbdev_turbo_sw is added to LDLIBS when
> > > CONFIG_RTE_BUILD_SHARED_LIB is not set.
> >
> > In that case, you need to use the EAL option "-d" with  the .so library.
> >
> > ./testbbdev -d build/lib/librte_pmd_bbdev_turbo_sw.so --vdev="turbo_sw"
> 
> I thought it is fine as long as other test applications works in that way, e.g. test-
> crypto-perf, test-pmd
> 
> > Btw, now that I have seen the name of the vdev, I think you should
> > rename to "baseband_turbo_sw".
> > Also, NULL PMD should be renamed to "baseband_null".
> 
> Ok, we will deliver such changes in patches for next release. Are you ok with
> that?

I think this change should not take a long time. The sooner we do it, the better,
as there might be more people using this in the future.
I can send the patch for you, so you can focus on other changes, and then you can see if it is fine for you.

Thanks,
Pablo

> 
> 
> Best regards,
> Kamil
  
De Lara Guarch, Pablo April 26, 2018, 1:22 p.m. UTC | #6
> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Thursday, April 26, 2018 1:48 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> 
> Hi Pablo
> 

...

> > > Ok, we will deliver such changes in patches for next release. Are
> > > you ok with that?
> >
> > I think this change should not take a long time. The sooner we do it,
> > the better, as there might be more people using this in the future.
> > I can send the patch for you, so you can focus on other changes, and
> > then you can see if it is fine for you.
> >
> > Thanks,
> > Pablo
> 
> Ok. I will add this change to our patchset as well.

In order to make sure that users can still use the previous name, to avoid breaking
Backwards compatibility, you modify DRIVER_NAME with the new name and then use
RTE_PMD_REGISTER_ALIAS(DRIVER_NAME, turbo_sw);
This way, both names can be used, but you should update the docs to use the new name.

Thanks,
Pablo
  
De Lara Guarch, Pablo April 26, 2018, 2:29 p.m. UTC | #7
> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Thursday, April 26, 2018 3:24 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> 
> Hi Pablo
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Thursday, April 26, 2018 3:22 PM
> > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> >
> >
> >
> > > -----Original Message-----
> > > From: Chalupnik, KamilX
> > > Sent: Thursday, April 26, 2018 1:48 PM
> > > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> > > dev@dpdk.org
> > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] app/bbdev: dynamic lib support
> > >
> > > Hi Pablo
> > >
> >
> > ...
> >
> > > > > Ok, we will deliver such changes in patches for next release.
> > > > > Are you ok with that?
> > > >
> > > > I think this change should not take a long time. The sooner we do
> > > > it, the better, as there might be more people using this in the future.
> > > > I can send the patch for you, so you can focus on other changes,
> > > > and then you can see if it is fine for you.
> > > >
> > > > Thanks,
> > > > Pablo
> > >
> > > Ok. I will add this change to our patchset as well.
> >
> > In order to make sure that users can still use the previous name, to
> > avoid breaking Backwards compatibility, you modify DRIVER_NAME with
> > the new name and then use RTE_PMD_REGISTER_ALIAS(DRIVER_NAME,
> > turbo_sw); This way, both names can be used, but you should update the
> > docs to use the new name.
> >
> > Thanks,
> > Pablo
> 
> 
> Thanks for suggestions :)
> I've already prepared a changes. Just one question. Can I prepare separate patch
> based on the 13 patchset? Is that ok? Such solution will be cleaner in my opinion.
> 

Sure, this change should be independent from the other ones. You don't have to add all patches in the same patchset (when they are not logically related).

Thanks,
Pablo

> Best regards,
> Kamil
>
  

Patch

diff --git a/app/test-bbdev/Makefile b/app/test-bbdev/Makefile
index 6da0c8e..26c9a4b 100644
--- a/app/test-bbdev/Makefile
+++ b/app/test-bbdev/Makefile
@@ -22,4 +22,8 @@  SRCS-$(CONFIG_RTE_TEST_BBDEV) += test_bbdev_vector.c
 
 LDLIBS += -lm
 
+ifeq ($(CONFIG_RTE_LIBRTE_PMD_BBDEV_TURBO_SW),y)
+LDLIBS += -lrte_pmd_bbdev_turbo_sw
+endif
+
 include $(RTE_SDK)/mk/rte.app.mk
diff --git a/doc/guides/tools/testbbdev.rst b/doc/guides/tools/testbbdev.rst
index 2ccc646..8a13cbd 100644
--- a/doc/guides/tools/testbbdev.rst
+++ b/doc/guides/tools/testbbdev.rst
@@ -36,6 +36,13 @@  The user must have all libraries, modules, updates and compilers installed
 in the system prior to this, as described in the earlier chapters in this
 Getting Started Guide.
 
+Compiling the Application with DPDK built as shared library
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Setting flag in config/common_base file:
+
+   ``CONFIG_RTE_BUILD_SHARED_LIB=y``
+
 Running the Application
 -----------------------