DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] ring: build with global includes
@ 2022-11-18 23:22 Tyler Retzlaff
  2022-11-18 23:22 ` Tyler Retzlaff
  2022-11-21 10:31 ` Bruce Richardson
  0 siblings, 2 replies; 10+ messages in thread
From: Tyler Retzlaff @ 2022-11-18 23:22 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, Tyler Retzlaff

ring has no dependencies and should be able to be built standalone but
cannot be since it cannot find rte_config.h. this change directs meson
to include global_inc paths just like is done with other libraries
e.g. telemetry.

Tyler Retzlaff (1):
  ring: build with global includes

 lib/ring/meson.build | 2 ++
 1 file changed, 2 insertions(+)

-- 
1.8.3.1


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

* [PATCH] ring: build with global includes
  2022-11-18 23:22 [PATCH] ring: build with global includes Tyler Retzlaff
@ 2022-11-18 23:22 ` Tyler Retzlaff
  2022-11-21 10:31 ` Bruce Richardson
  1 sibling, 0 replies; 10+ messages in thread
From: Tyler Retzlaff @ 2022-11-18 23:22 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, Tyler Retzlaff

Meson include global_inc so that rte_config.h can be found in the
include path.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/ring/meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c..defd9da 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
+includes = [global_inc]
+
 sources = files('rte_ring.c')
 headers = files('rte_ring.h')
 # most sub-headers are not for direct inclusion
-- 
1.8.3.1


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

* Re: [PATCH] ring: build with global includes
  2022-11-18 23:22 [PATCH] ring: build with global includes Tyler Retzlaff
  2022-11-18 23:22 ` Tyler Retzlaff
@ 2022-11-21 10:31 ` Bruce Richardson
  2022-11-21 19:53   ` Tyler Retzlaff
  1 sibling, 1 reply; 10+ messages in thread
From: Bruce Richardson @ 2022-11-21 10:31 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev

On Fri, Nov 18, 2022 at 03:22:07PM -0800, Tyler Retzlaff wrote:
> ring has no dependencies and should be able to be built standalone but
> cannot be since it cannot find rte_config.h. this change directs meson
> to include global_inc paths just like is done with other libraries
> e.g. telemetry.
> 
> Tyler Retzlaff (1):
>   ring: build with global includes
> 
>  lib/ring/meson.build | 2 ++
>  1 file changed, 2 insertions(+)
>

I am a little confused by this change - how do you mean built-standalone?
The ring library depends upon EAL for memory management, does it not? Also,
no DPDK library can be built on its own without the rest of the top-level
build infrastructure, which will ensure that the global-include folders are
on the include path? 

In terms of other libs, e.g. telemetry, the only reason those need the
global includes added to their include path explicitly is because those are
built ahead of EAL. Anything that depends on EAL - including ring - will
have the global includes available.

Can you explain a little more about the use-case you are looking at here,
and how you are attempting to build ring?

/Bruce 

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

* Re: [PATCH] ring: build with global includes
  2022-11-21 10:31 ` Bruce Richardson
@ 2022-11-21 19:53   ` Tyler Retzlaff
  2022-11-21 21:27     ` Konstantin Ananyev
  0 siblings, 1 reply; 10+ messages in thread
From: Tyler Retzlaff @ 2022-11-21 19:53 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Mon, Nov 21, 2022 at 10:31:29AM +0000, Bruce Richardson wrote:
> On Fri, Nov 18, 2022 at 03:22:07PM -0800, Tyler Retzlaff wrote:
> > ring has no dependencies and should be able to be built standalone but
> > cannot be since it cannot find rte_config.h. this change directs meson
> > to include global_inc paths just like is done with other libraries
> > e.g. telemetry.
> > 
> > Tyler Retzlaff (1):
> >   ring: build with global includes
> > 
> >  lib/ring/meson.build | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> 
> I am a little confused by this change - how do you mean built-standalone?
> The ring library depends upon EAL for memory management, does it not? Also,
> no DPDK library can be built on its own without the rest of the top-level
> build infrastructure, which will ensure that the global-include folders are
> on the include path? 
> 
> In terms of other libs, e.g. telemetry, the only reason those need the
> global includes added to their include path explicitly is because those are
> built ahead of EAL. Anything that depends on EAL - including ring - will
> have the global includes available.

i'm having trouble seeing where in the meson.build that ring depends on
eal can you point me to where it is?

> 
> Can you explain a little more about the use-case you are looking at here,
> and how you are attempting to build ring?

so i found this by trying to understand other libraries dependencies
through a process of disabling the build of various subsets.

it's possible i didn't look deeply enough but i didn't see an explicit
dependency on eal (in the meson.build files). maybe you can point out
where it is because by just having rte_config.h available it compiles
and links.

e.g. i don't see.

deps += ['eal']

is the dependency on eal the library or just eal headers? because if it
is header only it is equivalent to telemetry i think?

thanks!

ty

> 
> /Bruce 

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

* RE: [PATCH] ring: build with global includes
  2022-11-21 19:53   ` Tyler Retzlaff
@ 2022-11-21 21:27     ` Konstantin Ananyev
  2022-11-21 21:36       ` Thomas Monjalon
  0 siblings, 1 reply; 10+ messages in thread
From: Konstantin Ananyev @ 2022-11-21 21:27 UTC (permalink / raw)
  To: Tyler Retzlaff, Bruce Richardson; +Cc: dev



> -----Original Message-----
> From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Sent: Monday, November 21, 2022 7:53 PM
> To: Bruce Richardson <bruce.richardson@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] ring: build with global includes
> 
> On Mon, Nov 21, 2022 at 10:31:29AM +0000, Bruce Richardson wrote:
> > On Fri, Nov 18, 2022 at 03:22:07PM -0800, Tyler Retzlaff wrote:
> > > ring has no dependencies and should be able to be built standalone but
> > > cannot be since it cannot find rte_config.h. this change directs meson
> > > to include global_inc paths just like is done with other libraries
> > > e.g. telemetry.
> > >
> > > Tyler Retzlaff (1):
> > >   ring: build with global includes
> > >
> > >  lib/ring/meson.build | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> >
> > I am a little confused by this change - how do you mean built-standalone?
> > The ring library depends upon EAL for memory management, does it not? Also,
> > no DPDK library can be built on its own without the rest of the top-level
> > build infrastructure, which will ensure that the global-include folders are
> > on the include path?
> >
> > In terms of other libs, e.g. telemetry, the only reason those need the
> > global includes added to their include path explicitly is because those are
> > built ahead of EAL. Anything that depends on EAL - including ring - will
> > have the global includes available.
> 
> i'm having trouble seeing where in the meson.build that ring depends on
> eal can you point me to where it is?
> 
> >
> > Can you explain a little more about the use-case you are looking at here,
> > and how you are attempting to build ring?
> 
> so i found this by trying to understand other libraries dependencies
> through a process of disabling the build of various subsets.
> 
> it's possible i didn't look deeply enough but i didn't see an explicit
> dependency on eal (in the meson.build files). maybe you can point out
> where it is because by just having rte_config.h available it compiles
> and links.
> 
> e.g. i don't see.
> 
> deps += ['eal']
> 
> is the dependency on eal the library or just eal headers? because if it
> is header only it is equivalent to telemetry i think?

rte_ring.c uses bunch of EAL functions:
rte_zmalloc, rte_memzone_*,  rte_log*, rte_mcfg*, etc.  

> thanks!
> 
> ty
> 
> >
> > /Bruce

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

* Re: [PATCH] ring: build with global includes
  2022-11-21 21:27     ` Konstantin Ananyev
@ 2022-11-21 21:36       ` Thomas Monjalon
  2022-11-21 22:48         ` Tyler Retzlaff
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2022-11-21 21:36 UTC (permalink / raw)
  To: Tyler Retzlaff, Bruce Richardson, Konstantin Ananyev; +Cc: dev

21/11/2022 22:27, Konstantin Ananyev:
> From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > e.g. i don't see.
> > 
> > deps += ['eal']
> > 
> > is the dependency on eal the library or just eal headers? because if it
> > is header only it is equivalent to telemetry i think?
> 
> rte_ring.c uses bunch of EAL functions:
> rte_zmalloc, rte_memzone_*,  rte_log*, rte_mcfg*, etc.  

I think deps += ['eal'] is missing in ring meson file.




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

* Re: [PATCH] ring: build with global includes
  2022-11-21 21:36       ` Thomas Monjalon
@ 2022-11-21 22:48         ` Tyler Retzlaff
  2022-11-22  8:51           ` David Marchand
  0 siblings, 1 reply; 10+ messages in thread
From: Tyler Retzlaff @ 2022-11-21 22:48 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Bruce Richardson, Konstantin Ananyev, dev

On Mon, Nov 21, 2022 at 10:36:24PM +0100, Thomas Monjalon wrote:
> 21/11/2022 22:27, Konstantin Ananyev:
> > From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > e.g. i don't see.
> > > 
> > > deps += ['eal']
> > > 
> > > is the dependency on eal the library or just eal headers? because if it
> > > is header only it is equivalent to telemetry i think?
> > 
> > rte_ring.c uses bunch of EAL functions:
> > rte_zmalloc, rte_memzone_*,  rte_log*, rte_mcfg*, etc.  
> 
> I think deps += ['eal'] is missing in ring meson file.

i guess that's what i'm kind of getting at... if it was there then the
patch i submitted is not required since depending on eal would drag in
global_inc.

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

* Re: [PATCH] ring: build with global includes
  2022-11-21 22:48         ` Tyler Retzlaff
@ 2022-11-22  8:51           ` David Marchand
  2022-11-22  9:17             ` Bruce Richardson
  0 siblings, 1 reply; 10+ messages in thread
From: David Marchand @ 2022-11-22  8:51 UTC (permalink / raw)
  To: Tyler Retzlaff, Thomas Monjalon; +Cc: Bruce Richardson, Konstantin Ananyev, dev

On Mon, Nov 21, 2022 at 11:49 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> On Mon, Nov 21, 2022 at 10:36:24PM +0100, Thomas Monjalon wrote:
> > 21/11/2022 22:27, Konstantin Ananyev:
> > > From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > > e.g. i don't see.
> > > >
> > > > deps += ['eal']
> > > >
> > > > is the dependency on eal the library or just eal headers? because if it
> > > > is header only it is equivalent to telemetry i think?
> > >
> > > rte_ring.c uses bunch of EAL functions:
> > > rte_zmalloc, rte_memzone_*,  rte_log*, rte_mcfg*, etc.
> >
> > I think deps += ['eal'] is missing in ring meson file.
>
> i guess that's what i'm kind of getting at... if it was there then the
> patch i submitted is not required since depending on eal would drag in
> global_inc.

It is implicitly added, via lib/meson.build:

First eal is parsed before a lot of other components:

libraries = [
        'kvargs', # eal depends on kvargs
        'telemetry', # basic info querying
        'eal', # everything depends on eal
        'ring',

Then, there is:

    # eal is standard dependency once built
    if dpdk_conf.has('RTE_LIB_EAL')
        deps += ['eal']
    endif


-- 
David Marchand


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

* Re: [PATCH] ring: build with global includes
  2022-11-22  8:51           ` David Marchand
@ 2022-11-22  9:17             ` Bruce Richardson
  2022-11-22 16:22               ` Tyler Retzlaff
  0 siblings, 1 reply; 10+ messages in thread
From: Bruce Richardson @ 2022-11-22  9:17 UTC (permalink / raw)
  To: David Marchand; +Cc: Tyler Retzlaff, Thomas Monjalon, Konstantin Ananyev, dev

On Tue, Nov 22, 2022 at 09:51:53AM +0100, David Marchand wrote:
> On Mon, Nov 21, 2022 at 11:49 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > On Mon, Nov 21, 2022 at 10:36:24PM +0100, Thomas Monjalon wrote:
> > > 21/11/2022 22:27, Konstantin Ananyev:
> > > > From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > > > e.g. i don't see.
> > > > >
> > > > > deps += ['eal']
> > > > >
> > > > > is the dependency on eal the library or just eal headers? because if it
> > > > > is header only it is equivalent to telemetry i think?
> > > >
> > > > rte_ring.c uses bunch of EAL functions:
> > > > rte_zmalloc, rte_memzone_*,  rte_log*, rte_mcfg*, etc.
> > >
> > > I think deps += ['eal'] is missing in ring meson file.
> >
> > i guess that's what i'm kind of getting at... if it was there then the
> > patch i submitted is not required since depending on eal would drag in
> > global_inc.
> 
> It is implicitly added, via lib/meson.build:
> 
> First eal is parsed before a lot of other components:
> 
> libraries = [
>         'kvargs', # eal depends on kvargs
>         'telemetry', # basic info querying
>         'eal', # everything depends on eal
>         'ring',
> 
> Then, there is:
> 
>     # eal is standard dependency once built
>     if dpdk_conf.has('RTE_LIB_EAL')
>         deps += ['eal']
>     endif
> 
> 
Since every library in DPDK that is built after EAL depends upon EAL, it's
added as a standard dependency. However, if we prefer to have more explicit
dependencies we can remove that and add it to what every libraries need
it.

[Ideally, I'd like to have all library meson.build files call out the
full list of other libs they use, but I found in the past that it caused
the configure time with meson to balloon as it tracked recursive
dependencies, leading to entries appearing multiple times and then having to
be pruned down. This is why in meson.build files dependency lists have been
kept to a minimum up till now]

/Bruce

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

* Re: [PATCH] ring: build with global includes
  2022-11-22  9:17             ` Bruce Richardson
@ 2022-11-22 16:22               ` Tyler Retzlaff
  0 siblings, 0 replies; 10+ messages in thread
From: Tyler Retzlaff @ 2022-11-22 16:22 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: David Marchand, Thomas Monjalon, Konstantin Ananyev, dev

On Tue, Nov 22, 2022 at 09:17:50AM +0000, Bruce Richardson wrote:
> On Tue, Nov 22, 2022 at 09:51:53AM +0100, David Marchand wrote:
> > On Mon, Nov 21, 2022 at 11:49 PM Tyler Retzlaff
> > <roretzla@linux.microsoft.com> wrote:
> > >
> > > On Mon, Nov 21, 2022 at 10:36:24PM +0100, Thomas Monjalon wrote:
> > > > 21/11/2022 22:27, Konstantin Ananyev:
> > > > > From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > > > > e.g. i don't see.
> > > > > >
> > > > > > deps += ['eal']
> > > > > >
> > > > > > is the dependency on eal the library or just eal headers? because if it
> > > > > > is header only it is equivalent to telemetry i think?
> > > > >
> > > > > rte_ring.c uses bunch of EAL functions:
> > > > > rte_zmalloc, rte_memzone_*,  rte_log*, rte_mcfg*, etc.
> > > >
> > > > I think deps += ['eal'] is missing in ring meson file.
> > >
> > > i guess that's what i'm kind of getting at... if it was there then the
> > > patch i submitted is not required since depending on eal would drag in
> > > global_inc.
> > 
> > It is implicitly added, via lib/meson.build:
> > 
> > First eal is parsed before a lot of other components:
> > 
> > libraries = [
> >         'kvargs', # eal depends on kvargs
> >         'telemetry', # basic info querying
> >         'eal', # everything depends on eal
> >         'ring',
> > 
> > Then, there is:
> > 
> >     # eal is standard dependency once built
> >     if dpdk_conf.has('RTE_LIB_EAL')
> >         deps += ['eal']
> >     endif
> > 
> > 

ah! thank you for pointing this out. i just couldn't see it.

> Since every library in DPDK that is built after EAL depends upon EAL, it's
> added as a standard dependency. However, if we prefer to have more explicit
> dependencies we can remove that and add it to what every libraries need
> it.
> 
> [Ideally, I'd like to have all library meson.build files call out the
> full list of other libs they use, but I found in the past that it caused
> the configure time with meson to balloon as it tracked recursive
> dependencies, leading to entries appearing multiple times and then having to
> be pruned down. This is why in meson.build files dependency lists have been
> kept to a minimum up till now]

no, this is fine the way it works. i just obsessed about understanding
how it was working.

i'll withdraw the patch, it isn't needed.

thanks!

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

end of thread, other threads:[~2022-11-22 16:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-18 23:22 [PATCH] ring: build with global includes Tyler Retzlaff
2022-11-18 23:22 ` Tyler Retzlaff
2022-11-21 10:31 ` Bruce Richardson
2022-11-21 19:53   ` Tyler Retzlaff
2022-11-21 21:27     ` Konstantin Ananyev
2022-11-21 21:36       ` Thomas Monjalon
2022-11-21 22:48         ` Tyler Retzlaff
2022-11-22  8:51           ` David Marchand
2022-11-22  9:17             ` Bruce Richardson
2022-11-22 16:22               ` Tyler Retzlaff

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