[PATCH] mk: --no-as-needed by default for linux exec-env

[PATCH] mk: --no-as-needed by default for linux exec-env

[Sergio Gonzalez Monroy]

Ubuntu/Debian toolchain passes --as-needed flag to the linker by default.
Add --no-as-needed flag by default in linuxapp exec-env to ensure correct
linking.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 mk/exec-env/linuxapp/rte.vars.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mk/exec-env/linuxapp/rte.vars.mk b/mk/exec-env/linuxapp/rte.vars.mk
index d4808c2..79ddd5f 100644
--- a/mk/exec-env/linuxapp/rte.vars.mk
+++ b/mk/exec-env/linuxapp/rte.vars.mk
@@ -45,7 +45,7 @@ else
 EXECENV_CFLAGS  = -pthread
 endif
 
-EXECENV_LDFLAGS =
+EXECENV_LDFLAGS = --no-as-needed
 EXECENV_LDLIBS  = -lrt -lm
 EXECENV_ASFLAGS =
 
-- 
1.9.3

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Gonzalez Monroy, Sergio]

Some info about the issue for this patch:
https://wiki.ubuntu.com/ToolChain/CompilerFlags#Flags_passed_to_the_linker
https://wiki.debian.org/ToolChain/DSOLinking#Only_link_with_needed_libraries

Basically, Ubuntu GCC is always passing --as-needed to the linker which causes some
Linking issues for us.
I'm not entirely sure that we should patch this issue or just add to the release notes.
Currently we can work around this by setting EXTRA_LDFLAGS='--no-as-needed' 

Thoughts?

Thanks,
Sergio

> -----Original Message-----
> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Sergio Gonzalez
> Monroy
> Sent: Thursday, October 30, 2014 10:58 AM
> To: dev-VfR2kkLFssw@public.gmane.org
> Subject: [dpdk-dev] [PATCH] mk: --no-as-needed by default for linux exec-
> env
> 
> Ubuntu/Debian toolchain passes --as-needed flag to the linker by default.
> Add --no-as-needed flag by default in linuxapp exec-env to ensure correct
> linking.
> 
> Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  mk/exec-env/linuxapp/rte.vars.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mk/exec-env/linuxapp/rte.vars.mk b/mk/exec-
> env/linuxapp/rte.vars.mk
> index d4808c2..79ddd5f 100644
> --- a/mk/exec-env/linuxapp/rte.vars.mk
> +++ b/mk/exec-env/linuxapp/rte.vars.mk
> @@ -45,7 +45,7 @@ else
>  EXECENV_CFLAGS  = -pthread
>  endif
> 
> -EXECENV_LDFLAGS =
> +EXECENV_LDFLAGS = --no-as-needed
>  EXECENV_LDLIBS  = -lrt -lm
>  EXECENV_ASFLAGS =
> 
> --
> 1.9.3

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Neil Horman]

On Thu, Oct 30, 2014 at 11:05:24AM +0000, Gonzalez Monroy, Sergio wrote:
> Some info about the issue for this patch:
> https://wiki.ubuntu.com/ToolChain/CompilerFlags#Flags_passed_to_the_linker
> https://wiki.debian.org/ToolChain/DSOLinking#Only_link_with_needed_libraries
> 
> Basically, Ubuntu GCC is always passing --as-needed to the linker which causes some
> Linking issues for us.
Can you elaborate here?
Neil

> I'm not entirely sure that we should patch this issue or just add to the release notes.
> Currently we can work around this by setting EXTRA_LDFLAGS='--no-as-needed' 
> 
> Thoughts?
> 
> Thanks,
> Sergio
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Sergio Gonzalez
> > Monroy
> > Sent: Thursday, October 30, 2014 10:58 AM
> > To: dev-VfR2kkLFssw@public.gmane.org
> > Subject: [dpdk-dev] [PATCH] mk: --no-as-needed by default for linux exec-
> > env
> > 
> > Ubuntu/Debian toolchain passes --as-needed flag to the linker by default.
> > Add --no-as-needed flag by default in linuxapp exec-env to ensure correct
> > linking.
> > 
> > Signed-off-by: Sergio Gonzalez Monroy
> > <sergio.gonzalez.monroy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> >  mk/exec-env/linuxapp/rte.vars.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mk/exec-env/linuxapp/rte.vars.mk b/mk/exec-
> > env/linuxapp/rte.vars.mk
> > index d4808c2..79ddd5f 100644
> > --- a/mk/exec-env/linuxapp/rte.vars.mk
> > +++ b/mk/exec-env/linuxapp/rte.vars.mk
> > @@ -45,7 +45,7 @@ else
> >  EXECENV_CFLAGS  = -pthread
> >  endif
> > 
> > -EXECENV_LDFLAGS =
> > +EXECENV_LDFLAGS = --no-as-needed
> >  EXECENV_LDLIBS  = -lrt -lm
> >  EXECENV_ASFLAGS =
> > 
> > --
> > 1.9.3
> 
> 

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Gonzalez Monroy, Sergio]

> > Basically, Ubuntu GCC is always passing --as-needed to the linker
> > which causes some Linking issues for us.
> Can you elaborate here?
> Neil
> 
Sorry, probably I could have given more info about the issue.

Currently if we build DPDK with shared libs on Ubuntu, build process fails when it tries to link test apps with following error:
/tmp/DPDK-1.8-RC4/x86_64-native-linuxapp-gcc/lib/librte_eal.so: undefined reference to `rte_mempool_lookup'
/tmp/DPDK-1.8-RC4/x86_64-native-linuxapp-gcc/lib/librte_eal.so: undefined reference to `rte_mempool_create'

We can work around that error by building with EXTRA_LDFLAGS='--no-as-needed' because as the link about
Debian/Ubuntu toolchain mention, they always pass the '--as-needed' flag to the compiler.

I see a couple of issues with this flag in our current build process (this is from my own testing, I could be missing something here):
- We do not set dependencies for DPDK shared libraries. This is the cause of the error shown above.
   As the libs have no dependencies (librte_eal has no dependency on librte_mempool) and the app itself is not
   referencing the functions, librte_mempool is 'not needed' and the app fails to link when it tries to resolve librte_eal symbols.
- We push PMDs into the binary without being reference with --whole-archive but that does not seem to be the
   case if we previously have --as-needed (maybe there is a way around this? )

I have posted an RFC about improving the current build process, mostly affecting shared lib building.
By adding dependencies we could fix the error but not the second issue with PMDs.

I guess now it makes more sense what I mention below about not being sure about this patch, cause is not tackling the real issue here.

Hope this helps.

Thanks,
Sergio

> > I'm not entirely sure that we should patch this issue or just add to the
> release notes.
> > Currently we can work around this by setting EXTRA_LDFLAGS='--no-as-
> needed'
> >

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Neil Horman]

On Thu, Oct 30, 2014 at 04:20:17PM +0000, Gonzalez Monroy, Sergio wrote:
> > > Basically, Ubuntu GCC is always passing --as-needed to the linker
> > > which causes some Linking issues for us.
> > Can you elaborate here?
> > Neil
> > 
> Sorry, probably I could have given more info about the issue.
> 
> Currently if we build DPDK with shared libs on Ubuntu, build process fails when it tries to link test apps with following error:
> /tmp/DPDK-1.8-RC4/x86_64-native-linuxapp-gcc/lib/librte_eal.so: undefined reference to `rte_mempool_lookup'
> /tmp/DPDK-1.8-RC4/x86_64-native-linuxapp-gcc/lib/librte_eal.so: undefined reference to `rte_mempool_create'
> 
> We can work around that error by building with EXTRA_LDFLAGS='--no-as-needed' because as the link about
> Debian/Ubuntu toolchain mention, they always pass the '--as-needed' flag to the compiler.
> 
> I see a couple of issues with this flag in our current build process (this is from my own testing, I could be missing something here):
> - We do not set dependencies for DPDK shared libraries. This is the cause of the error shown above.
>    As the libs have no dependencies (librte_eal has no dependency on librte_mempool) and the app itself is not
>    referencing the functions, librte_mempool is 'not needed' and the app fails to link when it tries to resolve librte_eal symbols.
> - We push PMDs into the binary without being reference with --whole-archive but that does not seem to be the
>    case if we previously have --as-needed (maybe there is a way around this? )
> 
> I have posted an RFC about improving the current build process, mostly affecting shared lib building.
> By adding dependencies we could fix the error but not the second issue with PMDs.
> 
> I guess now it makes more sense what I mention below about not being sure about this patch, cause is not tackling the real issue here.
> 
> Hope this helps.
> 
> Thanks,
> Sergio
> 
Thank you, it does, though it raises an intersting question.  By flipping that
switch around you definately solve the problem at hand, but you don't really
close the bug completely.  The problem arises because librte_eal doesn't add a
DT_NEEDED entry for librte_mempool despite the fact that it references symbols
in that library.  It does this because we don't explicitly link with
-lrte_mempool when we build librte_eal.  Normally thats ok, because libraries
don't load on their own, and every application that needs to link rte_eal also
needs to link rte_mempool, so the symbols happen to resolve properly.  But that
may not always be the case for all libraries. It gives rise to the argument that
building a single large library may be preferable, as we might otherwise need to
create dependency chains where libraries fully specify their dependencies in
their DT_NEEDED sections

Neil

> > > I'm not entirely sure that we should patch this issue or just add to the
> > release notes.
> > > Currently we can work around this by setting EXTRA_LDFLAGS='--no-as-
> > needed'
> > >
> 

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Gonzalez Monroy, Sergio]

> From: Neil Horman [mailto:nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org]
> Subject: Re: [dpdk-dev] [PATCH] mk: --no-as-needed by default for linux
> exec-env
> 
> Thank you, it does, though it raises an intersting question.  By flipping that
> switch around you definately solve the problem at hand, but you don't really
> close the bug completely.  The problem arises because librte_eal doesn't add
> a DT_NEEDED entry for librte_mempool despite the fact that it references
> symbols in that library.  It does this because we don't explicitly link with -
> lrte_mempool when we build librte_eal.  Normally thats ok, because libraries
> don't load on their own, and every application that needs to link rte_eal also
> needs to link rte_mempool, so the symbols happen to resolve properly.  But
> that may not always be the case for all libraries. It gives rise to the argument
> that building a single large library may be preferable, as we might otherwise
> need to create dependency chains where libraries fully specify their
> dependencies in their DT_NEEDED sections
> 
> Neil
> 
That is a better explanation of what is really going on :) , and it does rise an interesting question.
In that regard, there is an open RFC to discuss how to improve the current build system.
http://dpdk.org/ml/archives/dev/2014-October/007389.html

Thanks,
Sergio

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Sergio Gonzalez
> Monroy
> Sent: Thursday, October 30, 2014 10:58 AM
> To: dev-VfR2kkLFssw@public.gmane.org
> Subject: [dpdk-dev] [PATCH] mk: --no-as-needed by default for linux exec-
> env
> 
> Ubuntu/Debian toolchain passes --as-needed flag to the linker by default.
> Add --no-as-needed flag by default in linuxapp exec-env to ensure correct
> linking.
> 
> Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Acked-by: Pablo de Lara <pablo.de.lara.guarch-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Anyway, it is worth stating that as Neil and Sergio have pointed out, we should probably
change the way we build the libraries, considering all problems that we have encountered recently.

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Thomas Monjalon]

Hi Sergio, Neil,

Do you agree to add this comment (before applying this *needed* patch)?

+# Workaround lack of DT_NEEDED entry

> -EXECENV_LDFLAGS =
> +EXECENV_LDFLAGS = --no-as-needed

-- 
Thomas

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Neil Horman]

On Fri, Nov 28, 2014 at 04:35:04PM +0100, Thomas Monjalon wrote:
> Hi Sergio, Neil,
> 
> Do you agree to add this comment (before applying this *needed* patch)?
> 
> +# Workaround lack of DT_NEEDED entry
> 
Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

We can solve the larger dependency chain issue later if we need to
Neil

> > -EXECENV_LDFLAGS =
> > +EXECENV_LDFLAGS = --no-as-needed
> 
> -- 
> Thomas
> 

Re: [PATCH] mk: --no-as-needed by default for linux exec-env

[Thomas Monjalon]

> > Do you agree to add this comment (before applying this *needed* patch)?
> > 
> > +# Workaround lack of DT_NEEDED entry
> > 
> Acked-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

Applied with above comment and Neil's explanations in commit log.

Thanks
 
> We can solve the larger dependency chain issue later if we need to

Yes, we probably need to rework a lot of things around the build system.
Let's re-discuss it after the 1.8.0 release.

-- 
Thomas