[dpdk-stable] [PATCH] app: fix plugin load on static builds

[dpdk-stable] [PATCH] app: fix plugin load on static builds

[Olivier Matz]

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.

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?

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

Re: [dpdk-stable] [PATCH] app: fix plugin load on static builds

[Bruce Richardson]

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

Re: [dpdk-stable] [PATCH] app: fix plugin load on static builds

[Olivier Matz]

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.

Re: [dpdk-stable] [PATCH] app: fix plugin load on static builds

[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>

Re: [dpdk-stable] [PATCH] app: fix plugin load on static builds

[Thomas Monjalon]

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.

Re: [dpdk-stable] [PATCH] app: fix plugin load on static builds

[Olivier Matz]

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.

Re: [dpdk-stable] [PATCH] app: fix plugin load on static builds

[Bruce Richardson]

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

[dpdk-stable] [PATCH v2] app: fix plugin load on static builds

[Olivier Matz]

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

[dpdk-stable] [PATCH v3] build: fix plugin load on static builds

[Olivier Matz]

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

Re: [dpdk-stable] [PATCH v3] build: fix plugin load on static builds

[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>

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] build: fix plugin load on static builds

[Thomas Monjalon]

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