[dpdk-dev] mk: generate internal library dependencies from DEPDIRS-y automatically

Message ID 62da0f5f31b2ab9db2dcc16acd2f8425a575a26f.1465293714.git.pmatilai@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Panu Matilainen June 7, 2016, 10:01 a.m. UTC
  Up to now dependencies between DPDK internal libraries have been
untracked at shared library level, requiring applications to know
about library internal dependencies and often consequently overlinking.

Since the dependencies are already recorded for build ordering in the
makefiles, we can use that information to generate LDLIBS entries for
internal libraries automatically.

Also revert commit 8180554d82b3 ("vhost: fix linkage of driver with
library") which is made redundant by this change.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 drivers/net/vhost/Makefile | 1 -
 mk/rte.lib.mk              | 7 +++++++
 2 files changed, 7 insertions(+), 1 deletion(-)
  

Comments

Christian Ehrhardt June 7, 2016, 12:36 p.m. UTC | #1
Hi Panu,
thanks for refreshing that - I'm much more happy with ldd output of most
libraries now after build with that.
And it comes much slimmer than my initial approach that touched all lib
makefiles.

Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

FYI - for anyone else also trying this in a backported way to 16.04 it
needs c6417ce6 and aace9d0b as prereq.

This being done is a nice step taken towards saner linking.
But I still struggle to see how to fix the circular dependency between
librte_eal and librte_mempool.
Maybe now is a time to look at this part of the original threads again to
eventually get apps less overlinked?
=> http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
My naive suggestions in generalized form can be found there (no answer yet):
=>
http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries


Kind Regards,
Christian



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jun 7, 2016 at 12:01 PM, Panu Matilainen <pmatilai@redhat.com>
wrote:

> Up to now dependencies between DPDK internal libraries have been
> untracked at shared library level, requiring applications to know
> about library internal dependencies and often consequently overlinking.
>
> Since the dependencies are already recorded for build ordering in the
> makefiles, we can use that information to generate LDLIBS entries for
> internal libraries automatically.
>
> Also revert commit 8180554d82b3 ("vhost: fix linkage of driver with
> library") which is made redundant by this change.
>
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> ---
>  drivers/net/vhost/Makefile | 1 -
>  mk/rte.lib.mk              | 7 +++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile
> index 30b91a0..f49a69b 100644
> --- a/drivers/net/vhost/Makefile
> +++ b/drivers/net/vhost/Makefile
> @@ -38,7 +38,6 @@ LIB = librte_pmd_vhost.a
>
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
> -LDLIBS += -lrte_vhost
>
>  EXPORT_MAP := rte_pmd_vhost_version.map
>
> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> index b420280..1ff403f 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -77,6 +77,13 @@ else
>  _CPU_LDFLAGS := $(CPU_LDFLAGS)
>  endif
>
> +# Translate DEPDIRS-y into LDLIBS
> +# Ignore (sub)directory dependencies which do not provide an actual
> library
> +_IGNORE_DIRS = lib/librte_eal/% lib/librte_net lib/librte_compat
> +_DEPDIRS = $(filter-out $(_IGNORE_DIRS),$(DEPDIRS-y))
> +_LDDIRS = $(subst librte_ether,libethdev,$(_DEPDIRS))
> +LDLIBS += $(subst lib/lib,-l,$(_LDDIRS))
> +
>  O_TO_A = $(AR) crDs $(LIB) $(OBJS-y)
>  O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
>  O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  AR $(@)")
> --
> 2.5.5
>
>
  
Thomas Monjalon June 7, 2016, 1 p.m. UTC | #2
2016-06-07 14:36, Christian Ehrhardt:
> But I still struggle to see how to fix the circular dependency between
> librte_eal and librte_mempool.

Why is there a circular dependency?
Only because of logs using mempool?

> Maybe now is a time to look at this part of the original threads again to
> eventually get apps less overlinked?
> => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
> My naive suggestions in generalized form can be found there (no answer yet):
> =>
> http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries

I would prefer removing the circular dependency.
Maybe we can rewrite the code to not use mempool or move it outside of EAL.
  
Bruce Richardson June 7, 2016, 2:07 p.m. UTC | #3
On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:
> 2016-06-07 14:36, Christian Ehrhardt:
> > But I still struggle to see how to fix the circular dependency between
> > librte_eal and librte_mempool.
> 
> Why is there a circular dependency?
> Only because of logs using mempool?
> 
> > Maybe now is a time to look at this part of the original threads again to
> > eventually get apps less overlinked?
> > => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
> > My naive suggestions in generalized form can be found there (no answer yet):
> > =>
> > http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries
> 
> I would prefer removing the circular dependency.
> Maybe we can rewrite the code to not use mempool or move it outside of EAL.

Or else we can take the attitude that the mempools and the rings are just a core
part of DPDK and move them and the EAL into a dpdk_core library at link time.
Having the code separate in the git tree is good, but I'm not sure having
the resulting object files being in separate .a/.so files is particularly useful.
I can't see someone wanting to use one without the other.

/Bruce
  
Thomas Monjalon June 7, 2016, 2:19 p.m. UTC | #4
2016-06-07 15:07, Bruce Richardson:
> On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:
> > 2016-06-07 14:36, Christian Ehrhardt:
> > > But I still struggle to see how to fix the circular dependency between
> > > librte_eal and librte_mempool.
> > 
> > Why is there a circular dependency?
> > Only because of logs using mempool?
> > 
> > > Maybe now is a time to look at this part of the original threads again to
> > > eventually get apps less overlinked?
> > > => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
> > > My naive suggestions in generalized form can be found there (no answer yet):
> > > =>
> > > http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries
> > 
> > I would prefer removing the circular dependency.
> > Maybe we can rewrite the code to not use mempool or move it outside of EAL.
> 
> Or else we can take the attitude that the mempools and the rings are just a core
> part of DPDK and move them and the EAL into a dpdk_core library at link time.
> Having the code separate in the git tree is good, but I'm not sure having
> the resulting object files being in separate .a/.so files is particularly useful.
> I can't see someone wanting to use one without the other.

EAL could be used as an abstraction layer on top of systems and platforms.
And I think keeping things separated and layered help to maintain a design
easy to understand.
  
Bruce Richardson June 7, 2016, 2:37 p.m. UTC | #5
On Tue, Jun 07, 2016 at 04:19:47PM +0200, Thomas Monjalon wrote:
> 2016-06-07 15:07, Bruce Richardson:
> > On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:
> > > 2016-06-07 14:36, Christian Ehrhardt:
> > > > But I still struggle to see how to fix the circular dependency between
> > > > librte_eal and librte_mempool.
> > > 
> > > Why is there a circular dependency?
> > > Only because of logs using mempool?
> > > 
> > > > Maybe now is a time to look at this part of the original threads again to
> > > > eventually get apps less overlinked?
> > > > => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
> > > > My naive suggestions in generalized form can be found there (no answer yet):
> > > > =>
> > > > http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries
> > > 
> > > I would prefer removing the circular dependency.
> > > Maybe we can rewrite the code to not use mempool or move it outside of EAL.
> > 
> > Or else we can take the attitude that the mempools and the rings are just a core
> > part of DPDK and move them and the EAL into a dpdk_core library at link time.
> > Having the code separate in the git tree is good, but I'm not sure having
> > the resulting object files being in separate .a/.so files is particularly useful.
> > I can't see someone wanting to use one without the other.
> 
> EAL could be used as an abstraction layer on top of systems and platforms.
> And I think keeping things separated and layered help to maintain a design
> easy to understand.

All that applies to the code, not so much to the resulting libraries generated.
I don't see how having a separate eal.so and mempool.so particularly helps in
any way, except to require more flags for linking.

/Bruce
  
Wiles, Keith June 7, 2016, 2:40 p.m. UTC | #6
On 6/7/16, 9:19 AM, "dev on behalf of Thomas Monjalon" <dev-bounces@dpdk.org on behalf of thomas.monjalon@6wind.com> wrote:

>2016-06-07 15:07, Bruce Richardson:

>> On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:

>> > 2016-06-07 14:36, Christian Ehrhardt:

>> > > But I still struggle to see how to fix the circular dependency between

>> > > librte_eal and librte_mempool.

>> > 

>> > Why is there a circular dependency?

>> > Only because of logs using mempool?

>> > 

>> > > Maybe now is a time to look at this part of the original threads again to

>> > > eventually get apps less overlinked?

>> > > => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html

>> > > My naive suggestions in generalized form can be found there (no answer yet):

>> > > =>

>> > > http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries

>> > 

>> > I would prefer removing the circular dependency.

>> > Maybe we can rewrite the code to not use mempool or move it outside of EAL.

>> 

>> Or else we can take the attitude that the mempools and the rings are just a core

>> part of DPDK and move them and the EAL into a dpdk_core library at link time.

>> Having the code separate in the git tree is good, but I'm not sure having

>> the resulting object files being in separate .a/.so files is particularly useful.

>> I can't see someone wanting to use one without the other.

>

>EAL could be used as an abstraction layer on top of systems and platforms.

>And I think keeping things separated and layered help to maintain a design

>easy to understand.


One of the other areas I see as an issue when building DPDK is the headers are only linked into place as the makefile is executed causing a dependency on build order. If you have a circular dependency on building libs or core code we have to link directly. Would this problem be helped if we had a two pass type build system, one to link the headers and then build the libraries?
>
  
Olivier Matz June 8, 2016, 12:34 p.m. UTC | #7
On 06/07/2016 04:40 PM, Wiles, Keith wrote:
> On 6/7/16, 9:19 AM, "dev on behalf of Thomas Monjalon" <dev-bounces@dpdk.org on behalf of thomas.monjalon@6wind.com> wrote:
> 
>> 2016-06-07 15:07, Bruce Richardson:
>>> On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:
>>>> 2016-06-07 14:36, Christian Ehrhardt:
>>>>> But I still struggle to see how to fix the circular dependency between
>>>>> librte_eal and librte_mempool.
>>>>
>>>> Why is there a circular dependency?
>>>> Only because of logs using mempool?
>>>>
>>>>> Maybe now is a time to look at this part of the original threads again to
>>>>> eventually get apps less overlinked?
>>>>> => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
>>>>> My naive suggestions in generalized form can be found there (no answer yet):
>>>>> =>
>>>>> http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries
>>>>
>>>> I would prefer removing the circular dependency.
>>>> Maybe we can rewrite the code to not use mempool or move it outside of EAL.

Indeed, mempools are used in eal for history. Is this feature still
useful now that logs are sent to syslog? Maybe we could deprecate this
API, and remove mempool calls in a future release?


>>> Or else we can take the attitude that the mempools and the rings are just a core
>>> part of DPDK and move them and the EAL into a dpdk_core library at link time.
>>> Having the code separate in the git tree is good, but I'm not sure having
>>> the resulting object files being in separate .a/.so files is particularly useful.
>>> I can't see someone wanting to use one without the other.
>>
>> EAL could be used as an abstraction layer on top of systems and platforms.
>> And I think keeping things separated and layered help to maintain a design
>> easy to understand.

I like the idea to have one lib per directory (for consistency). It
may also simplify the Makefiles.
I'm in favor of keeping mempool and ring separated from eal if we
can remove the circular dep.

Olivier
  
Thomas Monjalon June 8, 2016, 1:38 p.m. UTC | #8
2016-06-08 14:34, Olivier Matz:
> On 06/07/2016 04:40 PM, Wiles, Keith wrote:
> > On 6/7/16, 9:19 AM, "dev on behalf of Thomas Monjalon" <dev-bounces@dpdk.org on behalf of thomas.monjalon@6wind.com> wrote:
> > 
> >> 2016-06-07 15:07, Bruce Richardson:
> >>> On Tue, Jun 07, 2016 at 03:00:45PM +0200, Thomas Monjalon wrote:
> >>>> 2016-06-07 14:36, Christian Ehrhardt:
> >>>>> But I still struggle to see how to fix the circular dependency between
> >>>>> librte_eal and librte_mempool.
> >>>>
> >>>> Why is there a circular dependency?
> >>>> Only because of logs using mempool?
> >>>>
> >>>>> Maybe now is a time to look at this part of the original threads again to
> >>>>> eventually get apps less overlinked?
> >>>>> => http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
> >>>>> My naive suggestions in generalized form can be found there (no answer yet):
> >>>>> =>
> >>>>> http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries
> >>>>
> >>>> I would prefer removing the circular dependency.
> >>>> Maybe we can rewrite the code to not use mempool or move it outside of EAL.
> 
> Indeed, mempools are used in eal for history. Is this feature still
> useful now that logs are sent to syslog? Maybe we could deprecate this
> API, and remove mempool calls in a future release?

+1 to deprecate log history

> >>> Or else we can take the attitude that the mempools and the rings are just a core
> >>> part of DPDK and move them and the EAL into a dpdk_core library at link time.
> >>> Having the code separate in the git tree is good, but I'm not sure having
> >>> the resulting object files being in separate .a/.so files is particularly useful.
> >>> I can't see someone wanting to use one without the other.
> >>
> >> EAL could be used as an abstraction layer on top of systems and platforms.
> >> And I think keeping things separated and layered help to maintain a design
> >> easy to understand.
> 
> I like the idea to have one lib per directory (for consistency). It
> may also simplify the Makefiles.
> I'm in favor of keeping mempool and ring separated from eal if we
> can remove the circular dep.

+1 to keep bijectivity in src/lib.
  
Thomas Monjalon June 9, 2016, 9:40 a.m. UTC | #9
> > Up to now dependencies between DPDK internal libraries have been
> > untracked at shared library level, requiring applications to know
> > about library internal dependencies and often consequently overlinking.
> >
> > Since the dependencies are already recorded for build ordering in the
> > makefiles, we can use that information to generate LDLIBS entries for
> > internal libraries automatically.
> >
> > Also revert commit 8180554d82b3 ("vhost: fix linkage of driver with
> > library") which is made redundant by this change.
> >
> > Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

Applied, thanks
  

Patch

diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile
index 30b91a0..f49a69b 100644
--- a/drivers/net/vhost/Makefile
+++ b/drivers/net/vhost/Makefile
@@ -38,7 +38,6 @@  LIB = librte_pmd_vhost.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
-LDLIBS += -lrte_vhost
 
 EXPORT_MAP := rte_pmd_vhost_version.map
 
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index b420280..1ff403f 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -77,6 +77,13 @@  else
 _CPU_LDFLAGS := $(CPU_LDFLAGS)
 endif
 
+# Translate DEPDIRS-y into LDLIBS
+# Ignore (sub)directory dependencies which do not provide an actual library
+_IGNORE_DIRS = lib/librte_eal/% lib/librte_net lib/librte_compat
+_DEPDIRS = $(filter-out $(_IGNORE_DIRS),$(DEPDIRS-y))
+_LDDIRS = $(subst librte_ether,libethdev,$(_DEPDIRS))
+LDLIBS += $(subst lib/lib,-l,$(_LDDIRS))
+
 O_TO_A = $(AR) crDs $(LIB) $(OBJS-y)
 O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
 O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  AR $(@)")