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

[Bruce Richardson <bruce.richardson@intel.com>]

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