* Re: [dpdk-dev] [PATCH] app: fix plugin load on static builds
2020-11-26 14:20 [dpdk-dev] [PATCH] app: fix plugin load on static builds Olivier Matz
@ 2020-11-26 15:28 ` Bruce Richardson
2020-11-26 16:32 ` Olivier Matz
2020-12-18 12:52 ` [dpdk-dev] [PATCH v2] " Olivier Matz
2020-12-18 13:14 ` [dpdk-dev] [PATCH v3] build: " Olivier Matz
2 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2020-11-26 15:28 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, Harry van Haaren, Keith Wiles, Luca Boccassi, stable
On Thu, Nov 26, 2020 at 03:20:42PM +0100, Olivier Matz wrote:
> When dpdk is compiled as static libraries, it is not possible
> to load a plugin from an application. We get the following error:
>
> EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
>
> This happens because the dpdk symbols are not exported. Add them to the
> dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> previously present when compiled with Makefiles, it was introduced in
> commit f9a08f650211 ("eal: add support for shared object drivers")
>
> Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> Cc: stable@dpdk.org
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>
> Hi,
>
> Maybe the patch can be improved: I suppose that --export-dynamic
> should only be added in case we are linking in static mode. Any
> help about how to do that is welcome.
get_option('default_library') == 'static'
However, if the flag is harmless in shared linking mode, I don't think we
need bother with this.
>
> There is probably a better place to define the default ldflags for
> all applications (to factorize between app and example).
>
> Also, should this flag be advertised in pkg-config?
>
Perhaps. However, I'm not sure how common it would be for people to static
link their own apps with DPDK and then want to plug in extra drivers after
the fact? Are there any possible negative impacts to making that change?
If we weren't right before the release deadline I'd definitely suggest
adding it to the pkg-config files. At this late stage in release, I'm more
cautious.
> Thanks,
> Olivier
>
>
> app/meson.build | 3 +++
> examples/meson.build | 4 +++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/app/meson.build b/app/meson.build
> index eb74f215a3..92479c7729 100644
> --- a/app/meson.build
> +++ b/app/meson.build
> @@ -25,6 +25,7 @@ apps = [
> lib_execinfo = cc.find_library('execinfo', required: false)
>
> default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> +default_ldflags = ['-Wl,--export-dynamic']
>
> foreach app:apps
> build = true
> @@ -32,6 +33,7 @@ foreach app:apps
> sources = []
> includes = []
> cflags = default_cflags
> + ldflags = default_ldflags
> objs = [] # other object files to link against, used e.g. for
> # instruction-set optimized versions of code
>
> @@ -58,6 +60,7 @@ foreach app:apps
> executable('dpdk-' + name,
> sources,
> c_args: cflags,
> + link_args: ldflags,
> link_whole: link_libs,
> dependencies: dep_objs,
> install_rpath: join_paths(get_option('prefix'),
> diff --git a/examples/meson.build b/examples/meson.build
> index 46ec80919e..def4540e8f 100644
> --- a/examples/meson.build
> +++ b/examples/meson.build
> @@ -63,6 +63,7 @@ default_cflags = machine_args
> if cc.has_argument('-Wno-format-truncation')
> default_cflags += '-Wno-format-truncation'
> endif
> +default_ldflags = ['-Wl,--export-dynamic'] + dpdk_extra_ldflags
>
> foreach example: examples
> name = example.split('/')[-1]
> @@ -70,6 +71,7 @@ foreach example: examples
> sources = []
> allow_experimental_apis = false
> cflags = default_cflags
> + ldflags = default_ldflags
>
> ext_deps = [execinfo]
> includes = [include_directories(example)]
> @@ -91,7 +93,7 @@ foreach example: examples
> executable('dpdk-' + name, sources,
> include_directories: includes,
> link_whole: link_whole_libs,
> - link_args: dpdk_extra_ldflags,
> + link_args: ldflags,
> c_args: cflags,
> dependencies: dep_objs)
> elif not allow_skips
> --
> 2.25.1
>
Patch looks reasonable to me. In terms of other approaches, we have:
1. Add "add_project_link_arguments('-Wl,--export-dynamic', language: 'c')"
to "config/meson.build". This has the advantage of being a lot shorter, but
it would apply to all .so's too, so not sure if there is any impact there.
2. If there is no reason why any particular app would want to not provide
this flag, you can skip defining a new ldflags variable and just add the
flag into the existing link_args line for each app and example. However,
your approach here, though longer, is more in keeping with the style of
what is already there.
/Bruce
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH] app: fix plugin load on static builds
2020-11-26 15:28 ` Bruce Richardson
@ 2020-11-26 16:32 ` Olivier Matz
2020-11-26 16:37 ` Bruce Richardson
0 siblings, 1 reply; 11+ messages in thread
From: Olivier Matz @ 2020-11-26 16:32 UTC (permalink / raw)
To: Bruce Richardson
Cc: dev, Harry van Haaren, Keith Wiles, Luca Boccassi, stable
Hi Bruce,
Thanks for the feedback.
On Thu, Nov 26, 2020 at 03:28:46PM +0000, Bruce Richardson wrote:
> On Thu, Nov 26, 2020 at 03:20:42PM +0100, Olivier Matz wrote:
> > When dpdk is compiled as static libraries, it is not possible
> > to load a plugin from an application. We get the following error:
> >
> > EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
> >
> > This happens because the dpdk symbols are not exported. Add them to the
> > dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> > previously present when compiled with Makefiles, it was introduced in
> > commit f9a08f650211 ("eal: add support for shared object drivers")
> >
> > Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> > Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >
> > Hi,
> >
> > Maybe the patch can be improved: I suppose that --export-dynamic
> > should only be added in case we are linking in static mode. Any
> > help about how to do that is welcome.
>
> get_option('default_library') == 'static'
>
> However, if the flag is harmless in shared linking mode, I don't think we
> need bother with this.
ok
> >
> > There is probably a better place to define the default ldflags for
> > all applications (to factorize between app and example).
> >
> > Also, should this flag be advertised in pkg-config?
> >
> Perhaps. However, I'm not sure how common it would be for people to static
> link their own apps with DPDK and then want to plug in extra drivers after
> the fact? Are there any possible negative impacts to making that change?
I don't know if it is common, but this is something we do :)
> If we weren't right before the release deadline I'd definitely suggest
> adding it to the pkg-config files. At this late stage in release, I'm more
> cautious.
Yes, it is too late for 20.11. Maybe even for this patch without
updating pkg-config.
>
> > Thanks,
> > Olivier
> >
> >
> > app/meson.build | 3 +++
> > examples/meson.build | 4 +++-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/meson.build b/app/meson.build
> > index eb74f215a3..92479c7729 100644
> > --- a/app/meson.build
> > +++ b/app/meson.build
> > @@ -25,6 +25,7 @@ apps = [
> > lib_execinfo = cc.find_library('execinfo', required: false)
> >
> > default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> > +default_ldflags = ['-Wl,--export-dynamic']
> >
> > foreach app:apps
> > build = true
> > @@ -32,6 +33,7 @@ foreach app:apps
> > sources = []
> > includes = []
> > cflags = default_cflags
> > + ldflags = default_ldflags
> > objs = [] # other object files to link against, used e.g. for
> > # instruction-set optimized versions of code
> >
> > @@ -58,6 +60,7 @@ foreach app:apps
> > executable('dpdk-' + name,
> > sources,
> > c_args: cflags,
> > + link_args: ldflags,
> > link_whole: link_libs,
> > dependencies: dep_objs,
> > install_rpath: join_paths(get_option('prefix'),
> > diff --git a/examples/meson.build b/examples/meson.build
> > index 46ec80919e..def4540e8f 100644
> > --- a/examples/meson.build
> > +++ b/examples/meson.build
> > @@ -63,6 +63,7 @@ default_cflags = machine_args
> > if cc.has_argument('-Wno-format-truncation')
> > default_cflags += '-Wno-format-truncation'
> > endif
> > +default_ldflags = ['-Wl,--export-dynamic'] + dpdk_extra_ldflags
> >
> > foreach example: examples
> > name = example.split('/')[-1]
> > @@ -70,6 +71,7 @@ foreach example: examples
> > sources = []
> > allow_experimental_apis = false
> > cflags = default_cflags
> > + ldflags = default_ldflags
> >
> > ext_deps = [execinfo]
> > includes = [include_directories(example)]
> > @@ -91,7 +93,7 @@ foreach example: examples
> > executable('dpdk-' + name, sources,
> > include_directories: includes,
> > link_whole: link_whole_libs,
> > - link_args: dpdk_extra_ldflags,
> > + link_args: ldflags,
> > c_args: cflags,
> > dependencies: dep_objs)
> > elif not allow_skips
> > --
> > 2.25.1
> >
>
> Patch looks reasonable to me. In terms of other approaches, we have:
>
> 1. Add "add_project_link_arguments('-Wl,--export-dynamic', language: 'c')"
> to "config/meson.build". This has the advantage of being a lot shorter, but
> it would apply to all .so's too, so not sure if there is any impact there.
I will check if there is any impact on .so with this approach.
> 2. If there is no reason why any particular app would want to not provide
> this flag, you can skip defining a new ldflags variable and just add the
> flag into the existing link_args line for each app and example. However,
> your approach here, though longer, is more in keeping with the style of
> what is already there.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH] app: fix plugin load on static builds
2020-11-26 16:32 ` Olivier Matz
@ 2020-11-26 16:37 ` Bruce Richardson
2020-11-26 17:07 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
0 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2020-11-26 16:37 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, Harry van Haaren, Keith Wiles, Luca Boccassi, stable
On Thu, Nov 26, 2020 at 05:32:26PM +0100, Olivier Matz wrote:
> Hi Bruce,
>
> Thanks for the feedback.
>
> On Thu, Nov 26, 2020 at 03:28:46PM +0000, Bruce Richardson wrote:
> > On Thu, Nov 26, 2020 at 03:20:42PM +0100, Olivier Matz wrote:
> > > When dpdk is compiled as static libraries, it is not possible
> > > to load a plugin from an application. We get the following error:
> > >
> > > EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
> > >
> > > This happens because the dpdk symbols are not exported. Add them to the
> > > dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> > > previously present when compiled with Makefiles, it was introduced in
> > > commit f9a08f650211 ("eal: add support for shared object drivers")
> > >
> > > Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> > > Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > ---
> > >
> > > Hi,
> > >
> > > Maybe the patch can be improved: I suppose that --export-dynamic
> > > should only be added in case we are linking in static mode. Any
> > > help about how to do that is welcome.
> >
> > get_option('default_library') == 'static'
> >
> > However, if the flag is harmless in shared linking mode, I don't think we
> > need bother with this.
>
> ok
>
> > >
> > > There is probably a better place to define the default ldflags for
> > > all applications (to factorize between app and example).
> > >
> > > Also, should this flag be advertised in pkg-config?
> > >
> > Perhaps. However, I'm not sure how common it would be for people to static
> > link their own apps with DPDK and then want to plug in extra drivers after
> > the fact? Are there any possible negative impacts to making that change?
>
> I don't know if it is common, but this is something we do :)
>
> > If we weren't right before the release deadline I'd definitely suggest
> > adding it to the pkg-config files. At this late stage in release, I'm more
> > cautious.
>
> Yes, it is too late for 20.11. Maybe even for this patch without
> updating pkg-config.
>
Well, since this is something clearly broken that is of use to you, I think
we should strive to get some fix for it in. Based on the lateness of the
hour, I think your patch is pretty close to the least-risky option we could
take here. Therefore, despite my previous comment about not needing to
limit the additional linking flag to static-builds only, I would suggest -
out of an abundance of caution - that we do so, and keep the rest of your
patch as-is for 20.11. Thereafter we can look at other possible approaches
and simplifications in 21.02 and backport them.
So, for your patch with "if get_option('default_library') ..." checks added
appropriately:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH] app: fix plugin load on static builds
2020-11-26 16:37 ` Bruce Richardson
@ 2020-11-26 17:07 ` Thomas Monjalon
2020-11-26 17:13 ` Olivier Matz
2020-11-26 17:15 ` Bruce Richardson
0 siblings, 2 replies; 11+ messages in thread
From: Thomas Monjalon @ 2020-11-26 17:07 UTC (permalink / raw)
To: Olivier Matz, Bruce Richardson
Cc: stable, dev, Harry van Haaren, Keith Wiles, Luca Boccassi
26/11/2020 17:37, Bruce Richardson:
> On Thu, Nov 26, 2020 at 05:32:26PM +0100, Olivier Matz wrote:
> > Hi Bruce,
> >
> > Thanks for the feedback.
> >
> > On Thu, Nov 26, 2020 at 03:28:46PM +0000, Bruce Richardson wrote:
> > > On Thu, Nov 26, 2020 at 03:20:42PM +0100, Olivier Matz wrote:
> > > > When dpdk is compiled as static libraries, it is not possible
> > > > to load a plugin from an application. We get the following error:
> > > >
> > > > EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
> > > >
> > > > This happens because the dpdk symbols are not exported. Add them to the
> > > > dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> > > > previously present when compiled with Makefiles, it was introduced in
> > > > commit f9a08f650211 ("eal: add support for shared object drivers")
> > > >
> > > > Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> > > > Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > ---
> > > >
> > > > Hi,
> > > >
> > > > Maybe the patch can be improved: I suppose that --export-dynamic
> > > > should only be added in case we are linking in static mode. Any
> > > > help about how to do that is welcome.
> > >
> > > get_option('default_library') == 'static'
> > >
> > > However, if the flag is harmless in shared linking mode, I don't think we
> > > need bother with this.
> >
> > ok
> >
> > > >
> > > > There is probably a better place to define the default ldflags for
> > > > all applications (to factorize between app and example).
> > > >
> > > > Also, should this flag be advertised in pkg-config?
> > > >
> > > Perhaps. However, I'm not sure how common it would be for people to static
> > > link their own apps with DPDK and then want to plug in extra drivers after
> > > the fact? Are there any possible negative impacts to making that change?
> >
> > I don't know if it is common, but this is something we do :)
> >
> > > If we weren't right before the release deadline I'd definitely suggest
> > > adding it to the pkg-config files. At this late stage in release, I'm more
> > > cautious.
> >
> > Yes, it is too late for 20.11. Maybe even for this patch without
> > updating pkg-config.
> >
>
> Well, since this is something clearly broken that is of use to you, I think
> we should strive to get some fix for it in. Based on the lateness of the
> hour, I think your patch is pretty close to the least-risky option we could
> take here. Therefore, despite my previous comment about not needing to
> limit the additional linking flag to static-builds only, I would suggest -
> out of an abundance of caution - that we do so, and keep the rest of your
> patch as-is for 20.11. Thereafter we can look at other possible approaches
> and simplifications in 21.02 and backport them.
>
> So, for your patch with "if get_option('default_library') ..." checks added
> appropriately:
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
The most critical issue is for externally built applications
statically linked using external plugins.
This patch fixes only internal apps built with meson.
I am for keeping the issue as-is for 20.11.0,
because it is really too late to change a compilation option.
We have already been bitten by compilers not supporting an option
or being buggy in specific contexts. I prefer not taking this risk
for a fix which helps nobody really.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH] app: fix plugin load on static builds
2020-11-26 17:07 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
@ 2020-11-26 17:13 ` Olivier Matz
2020-11-26 17:15 ` Bruce Richardson
1 sibling, 0 replies; 11+ messages in thread
From: Olivier Matz @ 2020-11-26 17:13 UTC (permalink / raw)
To: Thomas Monjalon
Cc: Bruce Richardson, stable, dev, Harry van Haaren, Keith Wiles,
Luca Boccassi
On Thu, Nov 26, 2020 at 06:07:32PM +0100, Thomas Monjalon wrote:
> 26/11/2020 17:37, Bruce Richardson:
> > On Thu, Nov 26, 2020 at 05:32:26PM +0100, Olivier Matz wrote:
> > > Hi Bruce,
> > >
> > > Thanks for the feedback.
> > >
> > > On Thu, Nov 26, 2020 at 03:28:46PM +0000, Bruce Richardson wrote:
> > > > On Thu, Nov 26, 2020 at 03:20:42PM +0100, Olivier Matz wrote:
> > > > > When dpdk is compiled as static libraries, it is not possible
> > > > > to load a plugin from an application. We get the following error:
> > > > >
> > > > > EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
> > > > >
> > > > > This happens because the dpdk symbols are not exported. Add them to the
> > > > > dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> > > > > previously present when compiled with Makefiles, it was introduced in
> > > > > commit f9a08f650211 ("eal: add support for shared object drivers")
> > > > >
> > > > > Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> > > > > Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > ---
> > > > >
> > > > > Hi,
> > > > >
> > > > > Maybe the patch can be improved: I suppose that --export-dynamic
> > > > > should only be added in case we are linking in static mode. Any
> > > > > help about how to do that is welcome.
> > > >
> > > > get_option('default_library') == 'static'
> > > >
> > > > However, if the flag is harmless in shared linking mode, I don't think we
> > > > need bother with this.
> > >
> > > ok
> > >
> > > > >
> > > > > There is probably a better place to define the default ldflags for
> > > > > all applications (to factorize between app and example).
> > > > >
> > > > > Also, should this flag be advertised in pkg-config?
> > > > >
> > > > Perhaps. However, I'm not sure how common it would be for people to static
> > > > link their own apps with DPDK and then want to plug in extra drivers after
> > > > the fact? Are there any possible negative impacts to making that change?
> > >
> > > I don't know if it is common, but this is something we do :)
> > >
> > > > If we weren't right before the release deadline I'd definitely suggest
> > > > adding it to the pkg-config files. At this late stage in release, I'm more
> > > > cautious.
> > >
> > > Yes, it is too late for 20.11. Maybe even for this patch without
> > > updating pkg-config.
> > >
> >
> > Well, since this is something clearly broken that is of use to you, I think
> > we should strive to get some fix for it in. Based on the lateness of the
> > hour, I think your patch is pretty close to the least-risky option we could
> > take here. Therefore, despite my previous comment about not needing to
> > limit the additional linking flag to static-builds only, I would suggest -
> > out of an abundance of caution - that we do so, and keep the rest of your
> > patch as-is for 20.11. Thereafter we can look at other possible approaches
> > and simplifications in 21.02 and backport them.
> >
> > So, for your patch with "if get_option('default_library') ..." checks added
> > appropriately:
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> The most critical issue is for externally built applications
> statically linked using external plugins.
> This patch fixes only internal apps built with meson.
>
> I am for keeping the issue as-is for 20.11.0,
> because it is really too late to change a compilation option.
> We have already been bitten by compilers not supporting an option
> or being buggy in specific contexts. I prefer not taking this risk
> for a fix which helps nobody really.
Correct. On my side, it is not an issue if this patch does not go
in 20.11.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH] app: fix plugin load on static builds
2020-11-26 17:07 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2020-11-26 17:13 ` Olivier Matz
@ 2020-11-26 17:15 ` Bruce Richardson
1 sibling, 0 replies; 11+ messages in thread
From: Bruce Richardson @ 2020-11-26 17:15 UTC (permalink / raw)
To: Thomas Monjalon
Cc: Olivier Matz, stable, dev, Harry van Haaren, Keith Wiles, Luca Boccassi
On Thu, Nov 26, 2020 at 06:07:32PM +0100, Thomas Monjalon wrote:
> 26/11/2020 17:37, Bruce Richardson:
> > On Thu, Nov 26, 2020 at 05:32:26PM +0100, Olivier Matz wrote:
> > > Hi Bruce,
> > >
> > > Thanks for the feedback.
> > >
> > > On Thu, Nov 26, 2020 at 03:28:46PM +0000, Bruce Richardson wrote:
> > > > On Thu, Nov 26, 2020 at 03:20:42PM +0100, Olivier Matz wrote:
> > > > > When dpdk is compiled as static libraries, it is not possible
> > > > > to load a plugin from an application. We get the following error:
> > > > >
> > > > > EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
> > > > >
> > > > > This happens because the dpdk symbols are not exported. Add them to the
> > > > > dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> > > > > previously present when compiled with Makefiles, it was introduced in
> > > > > commit f9a08f650211 ("eal: add support for shared object drivers")
> > > > >
> > > > > Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> > > > > Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > ---
> > > > >
> > > > > Hi,
> > > > >
> > > > > Maybe the patch can be improved: I suppose that --export-dynamic
> > > > > should only be added in case we are linking in static mode. Any
> > > > > help about how to do that is welcome.
> > > >
> > > > get_option('default_library') == 'static'
> > > >
> > > > However, if the flag is harmless in shared linking mode, I don't think we
> > > > need bother with this.
> > >
> > > ok
> > >
> > > > >
> > > > > There is probably a better place to define the default ldflags for
> > > > > all applications (to factorize between app and example).
> > > > >
> > > > > Also, should this flag be advertised in pkg-config?
> > > > >
> > > > Perhaps. However, I'm not sure how common it would be for people to static
> > > > link their own apps with DPDK and then want to plug in extra drivers after
> > > > the fact? Are there any possible negative impacts to making that change?
> > >
> > > I don't know if it is common, but this is something we do :)
> > >
> > > > If we weren't right before the release deadline I'd definitely suggest
> > > > adding it to the pkg-config files. At this late stage in release, I'm more
> > > > cautious.
> > >
> > > Yes, it is too late for 20.11. Maybe even for this patch without
> > > updating pkg-config.
> > >
> >
> > Well, since this is something clearly broken that is of use to you, I think
> > we should strive to get some fix for it in. Based on the lateness of the
> > hour, I think your patch is pretty close to the least-risky option we could
> > take here. Therefore, despite my previous comment about not needing to
> > limit the additional linking flag to static-builds only, I would suggest -
> > out of an abundance of caution - that we do so, and keep the rest of your
> > patch as-is for 20.11. Thereafter we can look at other possible approaches
> > and simplifications in 21.02 and backport them.
> >
> > So, for your patch with "if get_option('default_library') ..." checks added
> > appropriately:
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> The most critical issue is for externally built applications
> statically linked using external plugins.
> This patch fixes only internal apps built with meson.
>
> I am for keeping the issue as-is for 20.11.0,
> because it is really too late to change a compilation option.
> We have already been bitten by compilers not supporting an option
> or being buggy in specific contexts. I prefer not taking this risk
> for a fix which helps nobody really.
>
I'm ok with that fairly conservative stance.
In general the workaround for this issue is to set the appropriate ldflags
when building. For those building their own app, I would assume adding the
flag can be fairly easily done, and for anyone building their own DPDK who
needs it, it can be set via "-Dc_link_args" meson flag. Anyone using
pre-built DPDK from packagers will already have a shared library build
anyway.
/Bruce
^ permalink raw reply [flat|nested] 11+ messages in thread
* [dpdk-dev] [PATCH v2] app: fix plugin load on static builds
2020-11-26 14:20 [dpdk-dev] [PATCH] app: fix plugin load on static builds Olivier Matz
2020-11-26 15:28 ` Bruce Richardson
@ 2020-12-18 12:52 ` Olivier Matz
2020-12-18 13:14 ` [dpdk-dev] [PATCH v3] build: " Olivier Matz
2 siblings, 0 replies; 11+ messages in thread
From: Olivier Matz @ 2020-12-18 12:52 UTC (permalink / raw)
To: dev
Cc: bruce.richardson, harry.van.haaren, keith.wiles, luca.boccassi,
olivier.matz, stable, thomas
When dpdk is compiled as static libraries, it is not possible
to load a plugin from an application. We get the following error:
EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
This happens because the dpdk symbols are not exported. Add them to the
dynamic symbol table by using '-Wl,--export-dynamic'. This option was
previously present when compiled with Makefiles, it was introduced in
commit f9a08f650211 ("eal: add support for shared object drivers")
Also add it to the pkg-config file.
Fixes: 16ade738fd0d ("app/testpmd: build with meson")
Fixes: 89f0711f9ddf ("examples: build some samples with meson")
Cc: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
app/meson.build | 6 ++++++
examples/meson.build | 7 ++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/app/meson.build b/app/meson.build
index eb74f215a3..fd72d7da68 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -25,6 +25,10 @@ apps = [
lib_execinfo = cc.find_library('execinfo', required: false)
default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
+default_ldflags = []
+if get_option('default_library') == 'static'
+ default_ldflags += ['-Wl,--export-dynamic']
+endif
foreach app:apps
build = true
@@ -32,6 +36,7 @@ foreach app:apps
sources = []
includes = []
cflags = default_cflags
+ ldflags = default_ldflags
objs = [] # other object files to link against, used e.g. for
# instruction-set optimized versions of code
@@ -58,6 +63,7 @@ foreach app:apps
executable('dpdk-' + name,
sources,
c_args: cflags,
+ link_args: ldflags,
link_whole: link_libs,
dependencies: dep_objs,
install_rpath: join_paths(get_option('prefix'),
diff --git a/examples/meson.build b/examples/meson.build
index 46ec80919e..f643ec1bad 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -63,6 +63,10 @@ default_cflags = machine_args
if cc.has_argument('-Wno-format-truncation')
default_cflags += '-Wno-format-truncation'
endif
+default_ldflags = dpdk_extra_ldflags
+if get_option('default_library') == 'static'
+ default_ldflags += ['-Wl,--export-dynamic']
+endif
foreach example: examples
name = example.split('/')[-1]
@@ -70,6 +74,7 @@ foreach example: examples
sources = []
allow_experimental_apis = false
cflags = default_cflags
+ ldflags = default_ldflags
ext_deps = [execinfo]
includes = [include_directories(example)]
@@ -91,7 +96,7 @@ foreach example: examples
executable('dpdk-' + name, sources,
include_directories: includes,
link_whole: link_whole_libs,
- link_args: dpdk_extra_ldflags,
+ link_args: ldflags,
c_args: cflags,
dependencies: dep_objs)
elif not allow_skips
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [dpdk-dev] [PATCH v3] build: fix plugin load on static builds
2020-11-26 14:20 [dpdk-dev] [PATCH] app: fix plugin load on static builds Olivier Matz
2020-11-26 15:28 ` Bruce Richardson
2020-12-18 12:52 ` [dpdk-dev] [PATCH v2] " Olivier Matz
@ 2020-12-18 13:14 ` Olivier Matz
2020-12-21 11:04 ` Bruce Richardson
2 siblings, 1 reply; 11+ messages in thread
From: Olivier Matz @ 2020-12-18 13:14 UTC (permalink / raw)
To: dev
Cc: bruce.richardson, harry.van.haaren, keith.wiles, luca.boccassi,
olivier.matz, stable, thomas
When dpdk is compiled as static libraries, it is not possible
to load a plugin from an application. We get the following error:
EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
This happens because the dpdk symbols are not exported. Add them to the
dynamic symbol table by using '-Wl,--export-dynamic'. This option was
previously present when compiled with Makefiles, it was introduced in
commit f9a08f650211 ("eal: add support for shared object drivers")
Also add it to the pkg-config file.
Fixes: 16ade738fd0d ("app/testpmd: build with meson")
Fixes: 89f0711f9ddf ("examples: build some samples with meson")
Cc: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
v2
* only set examples/applications flags if build is static
v3
* add missing pkg-config part in previous version
app/meson.build | 6 ++++++
buildtools/pkg-config/meson.build | 2 +-
examples/meson.build | 7 ++++++-
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/app/meson.build b/app/meson.build
index eb74f215a3..fd72d7da68 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -25,6 +25,10 @@ apps = [
lib_execinfo = cc.find_library('execinfo', required: false)
default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
+default_ldflags = []
+if get_option('default_library') == 'static'
+ default_ldflags += ['-Wl,--export-dynamic']
+endif
foreach app:apps
build = true
@@ -32,6 +36,7 @@ foreach app:apps
sources = []
includes = []
cflags = default_cflags
+ ldflags = default_ldflags
objs = [] # other object files to link against, used e.g. for
# instruction-set optimized versions of code
@@ -58,6 +63,7 @@ foreach app:apps
executable('dpdk-' + name,
sources,
c_args: cflags,
+ link_args: ldflags,
link_whole: link_libs,
dependencies: dep_objs,
install_rpath: join_paths(get_option('prefix'),
diff --git a/buildtools/pkg-config/meson.build b/buildtools/pkg-config/meson.build
index 5f19304289..168ee08e58 100644
--- a/buildtools/pkg-config/meson.build
+++ b/buildtools/pkg-config/meson.build
@@ -47,7 +47,7 @@ This is required for a number of static inline functions in the public headers.'
# if libbsd is not enabled, then this is blank
libraries_private: ['-Wl,--whole-archive'] +
dpdk_drivers + dpdk_static_libraries +
- ['-Wl,--no-whole-archive']
+ ['-Wl,--no-whole-archive', '-Wl,--export-dynamic']
)
# For static linking with dependencies as shared libraries,
diff --git a/examples/meson.build b/examples/meson.build
index 46ec80919e..f643ec1bad 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -63,6 +63,10 @@ default_cflags = machine_args
if cc.has_argument('-Wno-format-truncation')
default_cflags += '-Wno-format-truncation'
endif
+default_ldflags = dpdk_extra_ldflags
+if get_option('default_library') == 'static'
+ default_ldflags += ['-Wl,--export-dynamic']
+endif
foreach example: examples
name = example.split('/')[-1]
@@ -70,6 +74,7 @@ foreach example: examples
sources = []
allow_experimental_apis = false
cflags = default_cflags
+ ldflags = default_ldflags
ext_deps = [execinfo]
includes = [include_directories(example)]
@@ -91,7 +96,7 @@ foreach example: examples
executable('dpdk-' + name, sources,
include_directories: includes,
link_whole: link_whole_libs,
- link_args: dpdk_extra_ldflags,
+ link_args: ldflags,
c_args: cflags,
dependencies: dep_objs)
elif not allow_skips
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH v3] build: fix plugin load on static builds
2020-12-18 13:14 ` [dpdk-dev] [PATCH v3] build: " Olivier Matz
@ 2020-12-21 11:04 ` Bruce Richardson
2021-01-05 21:46 ` Thomas Monjalon
0 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2020-12-21 11:04 UTC (permalink / raw)
To: Olivier Matz
Cc: dev, harry.van.haaren, keith.wiles, luca.boccassi, stable, thomas
On Fri, Dec 18, 2020 at 02:14:22PM +0100, Olivier Matz wrote:
> When dpdk is compiled as static libraries, it is not possible
> to load a plugin from an application. We get the following error:
>
> EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
>
> This happens because the dpdk symbols are not exported. Add them to the
> dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> previously present when compiled with Makefiles, it was introduced in
> commit f9a08f650211 ("eal: add support for shared object drivers")
>
> Also add it to the pkg-config file.
>
> Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> Cc: stable@dpdk.org
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [dpdk-dev] [PATCH v3] build: fix plugin load on static builds
2020-12-21 11:04 ` Bruce Richardson
@ 2021-01-05 21:46 ` Thomas Monjalon
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2021-01-05 21:46 UTC (permalink / raw)
To: Olivier Matz
Cc: dev, harry.van.haaren, keith.wiles, luca.boccassi, stable,
Bruce Richardson
21/12/2020 12:04, Bruce Richardson:
> On Fri, Dec 18, 2020 at 02:14:22PM +0100, Olivier Matz wrote:
> > When dpdk is compiled as static libraries, it is not possible
> > to load a plugin from an application. We get the following error:
> >
> > EAL: librte_pmd_xxxx.so: undefined symbol: per_lcore__rte_errno
> >
> > This happens because the dpdk symbols are not exported. Add them to the
> > dynamic symbol table by using '-Wl,--export-dynamic'. This option was
> > previously present when compiled with Makefiles, it was introduced in
> > commit f9a08f650211 ("eal: add support for shared object drivers")
> >
> > Also add it to the pkg-config file.
> >
> > Fixes: 16ade738fd0d ("app/testpmd: build with meson")
> > Fixes: 89f0711f9ddf ("examples: build some samples with meson")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
Applied, thanks
^ permalink raw reply [flat|nested] 11+ messages in thread