DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Laatz, Kevin" <kevin.laatz@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>, harry.van.haaren@intel.com
Cc: dev@dpdk.org, gaetan.rivet@6wind.com,
	mattias.ronnblom@ericsson.com, bruce.richardson@intel.com,
	radu.nicolau@intel.com, ciara.power@intel.com,
	brian.archbold@intel.com
Subject: Re: [dpdk-dev] [PATCH] telemetry: fix shared build for make
Date: Thu, 8 Nov 2018 11:59:24 +0000	[thread overview]
Message-ID: <8747d99c-85c1-4301-b0a2-998dec934d8b@intel.com> (raw)
In-Reply-To: <6475218.XGFdfmr2Ql@xps>

On 07/11/2018 21:08, Thomas Monjalon wrote:
> 07/11/2018 19:10, Kevin Laatz:
>> Currently, telemetry is not working for shared builds in make.
>>
>> The --as-needed flag is preventing telemetry from being linked as there are
>> no direct API calls from the app to telemetry. This is causing the
>> --telemetry option to not be recognized by EAL.
>> Telemetry registers it's EAL option using the RTE_INIT constructor. Since
>> EAL's option parsing is done before the plugins init, the --telemetry
>> option isn't registered at the time of parsing, and as a result, the
>> --telemetry option is not being recognized.
> I do not understand how linking is related to parsing done before plugin init.
> Do you mean telemetry cannot be linked with dlopen?
>
> It makes me think that we could have avoided all the rte_option
> infrastructure by using plugin loading.
> If it is built statically, we can enable telemetry with a compilation
> option. If it is built as a shared library, the -d option is enough.
In EAL, option parsing happens before plugin init. This ordering is 
required in EAL since
plugins have EAL options that need to be parsed.

Telemetry registers it's EAL option using RTE_INIT constructor, so we 
cannot use telemetry
as a plugin since the option won't exist yet at the time of parsing due 
to the ordering of the
parsing and the init (where the telemetry option will be registered).
The -d option is also not enough as the telemetry option won't exist 
until plugin init is
done, which is too late.

Linking the library to the application solves this.

>
> About the patch itself,
>
>> This patch fixes this issue by explicitly linking telemetry to the
>> application by setting the "--no-as-needed" flag for the library in
>> mk/rte.app.mk.
>>
>> Fixes: 8877ac688b52 ("telemetry: introduce infrastructure")
>>
>> Reported-by: Yanjie Xu <yanjie.xu@intel.com>
>> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> [...]
>> --- a/mk/rte.app.mk
>> +++ b/mk/rte.app.mk
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-as-needed
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --whole-archive
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_telemetry -ljansson
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-whole-archive
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --as-needed
> It is forcing --as-needed even if it was disabled in LDFLAGS.
> I think this form may be better:
> 	--push-state,--as-needed
> 	--pop-state
> but it may not be supported by all linkers.
>
This does seem like a cleaner way to do this, and we could also use it 
for the
--whole-archive flag. However, if it is not supported by all linkers 
then I don't think
we should use it.

Regards,
Kevin

  reply	other threads:[~2018-11-08 11:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07 18:10 Kevin Laatz
2018-11-07 21:08 ` Thomas Monjalon
2018-11-08 11:59   ` Laatz, Kevin [this message]
2018-11-09 22:19 ` Ferruh Yigit
2018-11-12  0:23   ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8747d99c-85c1-4301-b0a2-998dec934d8b@intel.com \
    --to=kevin.laatz@intel.com \
    --cc=brian.archbold@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=ciara.power@intel.com \
    --cc=dev@dpdk.org \
    --cc=gaetan.rivet@6wind.com \
    --cc=harry.van.haaren@intel.com \
    --cc=mattias.ronnblom@ericsson.com \
    --cc=radu.nicolau@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).