DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap
@ 2021-03-26  8:22 Gabriel Ganne
  2021-03-26  9:30 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Gabriel Ganne @ 2021-03-26  8:22 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: thierry.herbelot, dev, Gabriel Ganne

libpcap is already found and registered as a dependency by meson, and
the dependency is already correctly used in librte_port. This line is
just unnecessary.

It also has the side effect of messing with the meson link line: dpdk
link will be declared twice: manually and then through pkg-config. If
you configure meson to prefer static linking over dynamic, this will
cause the build to fail on librte_port, since the pcap deps are not yet
seen by the linker.

Signed-off-by: Gabriel Ganne <gabriel.ganne@6wind.com>
---
 config/meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config/meson.build b/config/meson.build
index 66a2edcc47f5..95777cf33169 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -183,7 +183,6 @@ if not pcap_dep.found()
 endif
 if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
 	dpdk_conf.set('RTE_PORT_PCAP', 1)
-	dpdk_extra_ldflags += '-lpcap'
 endif
 
 # for clang 32-bit compiles we need libatomic for 64-bit atomic ops
-- 
2.29.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap
  2021-03-26  8:22 [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap Gabriel Ganne
@ 2021-03-26  9:30 ` Thomas Monjalon
  2021-04-04  0:05 ` Dmitry Kozlyuk
  2021-04-08 22:38 ` Thomas Monjalon
  2 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2021-03-26  9:30 UTC (permalink / raw)
  To: Gabriel Ganne; +Cc: Bruce Richardson, dev, thierry.herbelot, dev

26/03/2021 09:22, Gabriel Ganne:
> libpcap is already found and registered as a dependency by meson, and
> the dependency is already correctly used in librte_port. This line is
> just unnecessary.
> 
> It also has the side effect of messing with the meson link line: dpdk
> link will be declared twice: manually and then through pkg-config. If
> you configure meson to prefer static linking over dynamic, this will
> cause the build to fail on librte_port, since the pcap deps are not yet
> seen by the linker.
> 
> Signed-off-by: Gabriel Ganne <gabriel.ganne@6wind.com>

When sending a new revision of a patch,
please add a changelog below "---"
and use --in-reply-to for threading, thanks.

> ---
>  config/meson.build | 1 -
>  1 file changed, 1 deletion(-)





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap
  2021-03-26  8:22 [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap Gabriel Ganne
  2021-03-26  9:30 ` Thomas Monjalon
@ 2021-04-04  0:05 ` Dmitry Kozlyuk
  2021-04-08 22:38 ` Thomas Monjalon
  2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Kozlyuk @ 2021-04-04  0:05 UTC (permalink / raw)
  To: Gabriel Ganne; +Cc: Bruce Richardson, thierry.herbelot, dev

2021-03-26 09:22 (UTC+0100), Gabriel Ganne:
> libpcap is already found and registered as a dependency by meson, and
> the dependency is already correctly used in librte_port. This line is
> just unnecessary.
> 
> It also has the side effect of messing with the meson link line: dpdk
> link will be declared twice: manually and then through pkg-config. If
> you configure meson to prefer static linking over dynamic, this will
> cause the build to fail on librte_port, since the pcap deps are not yet
> seen by the linker.
> 
> Signed-off-by: Gabriel Ganne <gabriel.ganne@6wind.com>
> ---
>  config/meson.build | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 66a2edcc47f5..95777cf33169 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -183,7 +183,6 @@ if not pcap_dep.found()
>  endif
>  if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
>  	dpdk_conf.set('RTE_PORT_PCAP', 1)
> -	dpdk_extra_ldflags += '-lpcap'
>  endif
>  
>  # for clang 32-bit compiles we need libatomic for 64-bit atomic ops

This patch also simplifies future changes to discover libpcap on Windows.

Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap
  2021-03-26  8:22 [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap Gabriel Ganne
  2021-03-26  9:30 ` Thomas Monjalon
  2021-04-04  0:05 ` Dmitry Kozlyuk
@ 2021-04-08 22:38 ` Thomas Monjalon
  2021-04-09  8:31   ` Gabriel Ganne
  2 siblings, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2021-04-08 22:38 UTC (permalink / raw)
  To: Gabriel Ganne
  Cc: Bruce Richardson, thierry.herbelot, dev, olivier.matz, david.marchand

Thank you, the fix looks good.
I would like to improve the explanation.

If I understand the issue, the title should be
"build: remove redundant libpcap link"

26/03/2021 09:22, Gabriel Ganne:
> libpcap is already found and registered as a dependency by meson, and
> the dependency is already correctly used in librte_port. This line is
> just unnecessary.

To be precise, the pcap PMD and the librte_port both declare their
dependency to libpcap with a line "ext_deps += pcap_dep"
and meson automatically adds this dependency to the pkg-config file
in the private section for static builds.
That's why it is not needed to add the dependency explicitly
in dpdk_extra_ldflags (involved in static build with pkg-config).

> It also has the side effect of messing with the meson link line: dpdk

Which "link line"?

> link will be declared twice: manually and then through pkg-config. If

What is "manually"?

> you configure meson to prefer static linking over dynamic, this will

No need to configure meson for that. Static linking can be tested with
make in an example. Please avoid messing with meson explanation
for application linking, it is already complicate enough :)

> cause the build to fail on librte_port, since the pcap deps are not yet
> seen by the linker.

Sorry it is not clear to me.
I think it would help to see a before/after effect on the link command.
Something like:

before:
	Libs.private: -lpcap
	Requires.private: libpcap
after:
	Libs.private:
	Requires.private: libpcap

[...]
> -	dpdk_extra_ldflags += '-lpcap'




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap
  2021-04-08 22:38 ` Thomas Monjalon
@ 2021-04-09  8:31   ` Gabriel Ganne
  2021-04-09  9:24     ` Thomas Monjalon
  0 siblings, 1 reply; 6+ messages in thread
From: Gabriel Ganne @ 2021-04-09  8:31 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Bruce Richardson, Thierry Herbelot, dev, Olivier Matz, david.marchand

Hi Thomas,

Thanks for the review.

The use case that made me see this is that I configured the dpdk using meson
option "-- default_library=static" in conjunction with buildroot which is
patching
meson to prefer static libraries in this case.
This causes the link line generated from the dependency() directive to
result
in an expanded "/path/to/libpcap.a".
The dpdk_extra_ldflags += '-lpcap' is a direct (manual) addition to the
link line
that cannot be changed in the same way.

In this specific setup, the change is the following:
Before:
  -lpcap /path/to/libpcap.a
After:
  /path/to/libpcap.a

I admit this is quite the corner case (and probably not a great idea).

I will replace that confusing second paragraph with the comment you made
regarding " ext_deps += pcap_dep".

Best regards,

On Fri, Apr 9, 2021 at 12:39 AM Thomas Monjalon <thomas@monjalon.net> wrote:

> Thank you, the fix looks good.
> I would like to improve the explanation.
>
> If I understand the issue, the title should be
> "build: remove redundant libpcap link"
>
> 26/03/2021 09:22, Gabriel Ganne:
> > libpcap is already found and registered as a dependency by meson, and
> > the dependency is already correctly used in librte_port. This line is
> > just unnecessary.
>
> To be precise, the pcap PMD and the librte_port both declare their
> dependency to libpcap with a line "ext_deps += pcap_dep"
> and meson automatically adds this dependency to the pkg-config file
> in the private section for static builds.
> That's why it is not needed to add the dependency explicitly
> in dpdk_extra_ldflags (involved in static build with pkg-config).
>
> > It also has the side effect of messing with the meson link line: dpdk
>
> Which "link line"?
>
> > link will be declared twice: manually and then through pkg-config. If
>
> What is "manually"?
>
> > you configure meson to prefer static linking over dynamic, this will
>
> No need to configure meson for that. Static linking can be tested with
> make in an example. Please avoid messing with meson explanation
> for application linking, it is already complicate enough :)
>
> > cause the build to fail on librte_port, since the pcap deps are not yet
> > seen by the linker.
>
> Sorry it is not clear to me.
> I think it would help to see a before/after effect on the link command.
> Something like:
>
> before:
>         Libs.private: -lpcap
>         Requires.private: libpcap
> after:
>         Libs.private:
>         Requires.private: libpcap
>
> [...]
> > -     dpdk_extra_ldflags += '-lpcap'
>
>
>
>

-- 
Gabriel Ganne

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap
  2021-04-09  8:31   ` Gabriel Ganne
@ 2021-04-09  9:24     ` Thomas Monjalon
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2021-04-09  9:24 UTC (permalink / raw)
  To: Gabriel Ganne
  Cc: Bruce Richardson, Thierry Herbelot, dev, Olivier Matz, david.marchand

09/04/2021 10:31, Gabriel Ganne:
> Hi Thomas,
> 
> Thanks for the review.
> 
> The use case that made me see this is that I configured the dpdk using meson
> option "-- default_library=static" in conjunction with buildroot which is
> patching
> meson to prefer static libraries in this case.
> This causes the link line generated from the dependency() directive to
> result
> in an expanded "/path/to/libpcap.a".
> The dpdk_extra_ldflags += '-lpcap' is a direct (manual) addition to the
> link line
> that cannot be changed in the same way.

No it is only changing the generated pkg-config file.
I think you are messing between what happens in DPDK build,
and what your specific environment/application does.

> In this specific setup, the change is the following:
> Before:
>   -lpcap /path/to/libpcap.a
> After:
>   /path/to/libpcap.a

The real before/after is in the pkg-config file as I showed below.

> I admit this is quite the corner case (and probably not a great idea).
> 
> I will replace that confusing second paragraph with the comment you made
> regarding " ext_deps += pcap_dep".

You missed my other comments.
I will propose a v5 with my explanations.

PS: please avoid top-post.


> On Fri, Apr 9, 2021 at 12:39 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > Thank you, the fix looks good.
> > I would like to improve the explanation.
> >
> > If I understand the issue, the title should be
> > "build: remove redundant libpcap link"
> >
> > 26/03/2021 09:22, Gabriel Ganne:
> > > libpcap is already found and registered as a dependency by meson, and
> > > the dependency is already correctly used in librte_port. This line is
> > > just unnecessary.
> >
> > To be precise, the pcap PMD and the librte_port both declare their
> > dependency to libpcap with a line "ext_deps += pcap_dep"
> > and meson automatically adds this dependency to the pkg-config file
> > in the private section for static builds.
> > That's why it is not needed to add the dependency explicitly
> > in dpdk_extra_ldflags (involved in static build with pkg-config).
> >
> > > It also has the side effect of messing with the meson link line: dpdk
> >
> > Which "link line"?
> >
> > > link will be declared twice: manually and then through pkg-config. If
> >
> > What is "manually"?
> >
> > > you configure meson to prefer static linking over dynamic, this will
> >
> > No need to configure meson for that. Static linking can be tested with
> > make in an example. Please avoid messing with meson explanation
> > for application linking, it is already complicate enough :)
> >
> > > cause the build to fail on librte_port, since the pcap deps are not yet
> > > seen by the linker.
> >
> > Sorry it is not clear to me.
> > I think it would help to see a before/after effect on the link command.
> > Something like:
> >
> > before:
> >         Libs.private: -lpcap
> >         Requires.private: libpcap
> > after:
> >         Libs.private:
> >         Requires.private: libpcap
> >
> > [...]
> > > -     dpdk_extra_ldflags += '-lpcap'






^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-09  9:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26  8:22 [dpdk-dev] [PATCH v3] meson: remove unnecessary explicit link to libpcap Gabriel Ganne
2021-03-26  9:30 ` Thomas Monjalon
2021-04-04  0:05 ` Dmitry Kozlyuk
2021-04-08 22:38 ` Thomas Monjalon
2021-04-09  8:31   ` Gabriel Ganne
2021-04-09  9:24     ` Thomas Monjalon

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).