* [PATCH 24.03] build: track mandatory rather than optional libs @ 2023-11-03 16:28 Bruce Richardson 2023-11-03 16:52 ` [PATCH 24.03 v2] " Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 0/3] Improve optional lib support Bruce Richardson 0 siblings, 2 replies; 20+ messages in thread From: Bruce Richardson @ 2023-11-03 16:28 UTC (permalink / raw) To: dev; +Cc: Bruce Richardson DPDK now has more optional libraries than mandatory ones, so invert the list stored in the meson.build file from the optional ones to the "always_enable" ones. As well as being a shorter list: * we can remove the loop building up the "always_enable" list dynamically from the optional list * it better aligns with the drivers/meson.build file which maintains an always_enable list. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- lib/meson.build | 70 +++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/lib/meson.build b/lib/meson.build index 6c143ce5a6..2dd2ee7262 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -75,52 +75,29 @@ if is_ms_compiler ] endif -optional_libs = [ - 'bbdev', - 'bitratestats', - 'bpf', - 'cfgfile', - 'compressdev', - 'cryptodev', - 'dispatcher', - 'distributor', - 'dmadev', - 'efd', - 'eventdev', - 'gpudev', - 'graph', - 'gro', - 'gso', - 'ip_frag', - 'ipsec', - 'jobstats', - 'latencystats', - 'member', - 'metrics', - 'mldev', - 'node', - 'pcapng', - 'pdcp', - 'pdump', - 'pipeline', - 'port', - 'power', - 'rawdev', - 'regexdev', - 'reorder', - 'sched', - 'security', - 'table', - 'vhost', +always_enable = [ + 'log', + 'kvargs', + 'telemetry', + 'eal', + 'ring', + 'rcu', + 'mempool', + 'mbuf', + 'net', + 'meter', + 'ethdev', + 'pci', + 'cmdline', + 'hash', + 'timer', + 'acl', + 'lpm', + 'rib', + 'stack', + 'fib' ] -always_enable = [] -foreach l:libraries - if not optional_libs.contains(l) - always_enable += l - endif -endforeach - enable_deprecated_libs = [] foreach l:run_command(list_dir_globs, get_option('enable_deprecated_libs'), check: true).stdout().split() @@ -136,9 +113,10 @@ enable_libs = run_command(list_dir_globs, get_option('enable_libs'), check: true require_libs = true if enable_libs.length() == 0 require_libs = false - enable_libs += optional_libs + enable_libs = libraries +else + enable_libs += always_enable endif -enable_libs += always_enable default_cflags = machine_args default_cflags += ['-DALLOW_EXPERIMENTAL_API'] -- 2.39.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-03 16:28 [PATCH 24.03] build: track mandatory rather than optional libs Bruce Richardson @ 2023-11-03 16:52 ` Bruce Richardson 2023-11-03 17:31 ` Morten Brørup 2023-12-20 14:21 ` [PATCH v3 0/3] Improve optional lib support Bruce Richardson 1 sibling, 1 reply; 20+ messages in thread From: Bruce Richardson @ 2023-11-03 16:52 UTC (permalink / raw) To: dev; +Cc: Bruce Richardson DPDK now has more optional libraries than mandatory ones, so invert the list stored in the meson.build file from the optional ones to the "always_enable" ones. As well as being a shorter list: * we can remove the loop building up the "always_enable" list dynamically from the optional list * it better aligns with the drivers/meson.build file which maintains an always_enable list. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- V2: List of mandatory libraries is now sorted alphabetically --- lib/meson.build | 70 +++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/lib/meson.build b/lib/meson.build index 6c143ce5a6..8c922d3097 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -75,52 +75,29 @@ if is_ms_compiler ] endif -optional_libs = [ - 'bbdev', - 'bitratestats', - 'bpf', - 'cfgfile', - 'compressdev', - 'cryptodev', - 'dispatcher', - 'distributor', - 'dmadev', - 'efd', - 'eventdev', - 'gpudev', - 'graph', - 'gro', - 'gso', - 'ip_frag', - 'ipsec', - 'jobstats', - 'latencystats', - 'member', - 'metrics', - 'mldev', - 'node', - 'pcapng', - 'pdcp', - 'pdump', - 'pipeline', - 'port', - 'power', - 'rawdev', - 'regexdev', - 'reorder', - 'sched', - 'security', - 'table', - 'vhost', +always_enable = [ + 'acl', + 'cmdline', + 'eal', + 'ethdev', + 'fib', + 'hash', + 'kvargs', + 'log', + 'lpm', + 'mbuf', + 'mempool', + 'meter', + 'net', + 'pci', + 'rcu', + 'rib', + 'ring', + 'stack', + 'telemetry', + 'timer', ] -always_enable = [] -foreach l:libraries - if not optional_libs.contains(l) - always_enable += l - endif -endforeach - enable_deprecated_libs = [] foreach l:run_command(list_dir_globs, get_option('enable_deprecated_libs'), check: true).stdout().split() @@ -136,9 +113,10 @@ enable_libs = run_command(list_dir_globs, get_option('enable_libs'), check: true require_libs = true if enable_libs.length() == 0 require_libs = false - enable_libs += optional_libs + enable_libs = libraries +else + enable_libs += always_enable endif -enable_libs += always_enable default_cflags = machine_args default_cflags += ['-DALLOW_EXPERIMENTAL_API'] -- 2.39.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* RE: [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-03 16:52 ` [PATCH 24.03 v2] " Bruce Richardson @ 2023-11-03 17:31 ` Morten Brørup 2023-11-03 18:08 ` Bruce Richardson 0 siblings, 1 reply; 20+ messages in thread From: Morten Brørup @ 2023-11-03 17:31 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Friday, 3 November 2023 17.52 > > DPDK now has more optional libraries than mandatory ones, so invert the > list stored in the meson.build file from the optional ones to the > "always_enable" ones. As well as being a shorter list: > > * we can remove the loop building up the "always_enable" list > dynamically from the optional list > * it better aligns with the drivers/meson.build file which maintains an > always_enable list. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Excellent! It really shows how bloated DPDK CORE still is. I would like to see these go optional: acl - OPTIONAL cmdline - OPTIONAL (if not used by EAL) eal - CORE ethdev - CORE, or OPTIONAL for secondary process fib - OPTIONAL hash - OPTIONAL (if not used by CORE libs) kvargs - OPTIONAL (if not used by EAL) log - CORE lpm - OPTIONAL mbuf - CORE mempool - CORE meter - OPTIONAL net - CORE pci - CORE, or OPTIONAL for secondary process rcu - OPTIONAL (if not used by CORE libs) rib - OPTIONAL ring - CORE stack - OPTIONAL (if not used by CORE libs) telemetry - OPTIONAL timer - OPTIONAL (if not used by CORE libs) Anyway, Acked-by: Morten Brørup <mb@smartsharesystems.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-03 17:31 ` Morten Brørup @ 2023-11-03 18:08 ` Bruce Richardson 2023-11-03 20:19 ` Morten Brørup 0 siblings, 1 reply; 20+ messages in thread From: Bruce Richardson @ 2023-11-03 18:08 UTC (permalink / raw) To: Morten Brørup; +Cc: dev On Fri, Nov 03, 2023 at 06:31:30PM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > Sent: Friday, 3 November 2023 17.52 > > > > DPDK now has more optional libraries than mandatory ones, so invert the > > list stored in the meson.build file from the optional ones to the > > "always_enable" ones. As well as being a shorter list: > > > > * we can remove the loop building up the "always_enable" list > > dynamically from the optional list > > * it better aligns with the drivers/meson.build file which maintains an > > always_enable list. > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > Excellent! > > It really shows how bloated DPDK CORE still is. I would like to see these go optional: > For some I agree, but we need to decide what optional really means. :-) For my mind, there are 3 (maybe 4) key components that need to be built for me to consider a build to be a valid DPDK one: * EAL obviously, * testpmd, because everyone seems to use it * l3fwd, becaues it's the most commonly referenced example and used for benchmarking, and build testing in test-meson-builds. (There are others, but they are pretty likely to build if l3fwd does!) * dpdk-test - I feel this should always be buildable, but for me it's the optional 4th component. Now, the obviously one to relax here is l3fwd, since it is just an example, but I wonder if that may cause some heartache. Anyway some notes: > acl - OPTIONAL Used by l3fwd > cmdline - OPTIONAL (if not used by EAL) I'd consider this core - used by testpmd and the auto-tests. I'd push this down the list to consider making optional. > eal - CORE > ethdev - CORE, or OPTIONAL for secondary process Is there really much use of DPDK without ethdev? Let's just keep core for now. > fib - OPTIONAL Used by l3fwd > hash - OPTIONAL (if not used by CORE libs) Needed by l3fwd. Also listed as a dependency by a number of drivers, e.g. i40e, mlx5, nfp, tap. I think they use it for holding filters and things. This I'd be wary about allowing disabling without some work, as it could cause users surprise when some drivers unexpectedly stop building. > kvargs - OPTIONAL (if not used by EAL) Used by EAL > log - CORE > lpm - OPTIONAL Used by l3fwd > mbuf - CORE > mempool - CORE > meter - OPTIONAL Used by ethdev. > net - CORE > pci - CORE, or OPTIONAL for secondary process > rcu - OPTIONAL (if not used by CORE libs) Used by lpm and hash, which are used by l3fwd. > rib - OPTIONAL Used by fib, used by l3fwd. > ring - CORE Actually, this is one I'd look to maybe say optional. :-) I think we should switch our default mempool backend from ring to stack, and make that mandatory. The reason being that run-to-completion apps should be running entirely out of mempool cache and not care about the underlying driver, while pipelined apps pushing buffers across cores would be better with a LIFO-based rather than FIFO-based mempool driver. Therefore, I think overall switching to stack from ring would be an overall win for performance. > stack - OPTIONAL (if not used by CORE libs) For reasons above, I'd make this core. > telemetry - OPTIONAL Used by EAL > timer - OPTIONAL (if not used by CORE libs) Only lib that seems to depend on this is eventdev, so maybe this can be an easy removal. Though I do see it listed as a dependency of ENA driver too. > > Anyway, > > Acked-by: Morten Brørup <mb@smartsharesystems.com> > Overall, if we want to make more libs optional, I would start looking at l3fwd and making it a bit more modular. I previously made its support for eventdev optional, we should do the same for lpm and fib. Beyond that, we need to decide what core really means. /Bruce ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-03 18:08 ` Bruce Richardson @ 2023-11-03 20:19 ` Morten Brørup 2023-11-06 10:28 ` Bruce Richardson 0 siblings, 1 reply; 20+ messages in thread From: Morten Brørup @ 2023-11-03 20:19 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Friday, 3 November 2023 19.09 > > On Fri, Nov 03, 2023 at 06:31:30PM +0100, Morten Brørup wrote: > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: Friday, 3 November 2023 17.52 > > > > > > DPDK now has more optional libraries than mandatory ones, so invert > the > > > list stored in the meson.build file from the optional ones to the > > > "always_enable" ones. As well as being a shorter list: > > > > > > * we can remove the loop building up the "always_enable" list > > > dynamically from the optional list > > > * it better aligns with the drivers/meson.build file which > maintains an > > > always_enable list. > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > > Excellent! > > > > It really shows how bloated DPDK CORE still is. I would like to see > these go optional: > > > > For some I agree, but we need to decide what optional really means. :-) > > For my mind, there are 3 (maybe 4) key components that need to be built > for > me to consider a build to be a valid DPDK one: > * EAL obviously, > * testpmd, because everyone seems to use it > * l3fwd, becaues it's the most commonly referenced example and used for > benchmarking, and build testing in test-meson-builds. (There are > others, > but they are pretty likely to build if l3fwd does!) > * dpdk-test - I feel this should always be buildable, but for me it's > the > optional 4th component. > > Now, the obviously one to relax here is l3fwd, since it is just an > example, > but I wonder if that may cause some heartache. I don't consider any DPDK lib CORE just because the lib is used by testpmd and/or l3fwd. I agree that all libs should be included by default, so you can run testpmd, l3fwd, and other apps and examples. However, many libs are not needed for *all* DPDK applications, so I would like other apps to be able to build DPDK without superfluous libs. E.g. our StraightShaper CSP appliance is deployed at Layer 2, and doesn't use any of DPDK's L3 libs, so why should the DPDK L3 libs be considered CORE and thus included in our application? I suppose other companies are also using DPDK for other purposes than L3 routing, and don't need the DPDK L3 libs. Furthermore, I suppose that some Layer 3 applications use their own RIB/FIB/LPM libraries. Does OVS use DPDK's rib/fib/lpm libraries? > > Anyway some notes: > > > acl - OPTIONAL > Used by l3fwd But not *all* apps. > > > cmdline - OPTIONAL (if not used by EAL) > I'd consider this core - used by testpmd and the auto-tests. I'd push > this > down the list to consider making optional. > > > eal - CORE > > ethdev - CORE, or OPTIONAL for secondary process > Is there really much use of DPDK without ethdev? Let's just keep core > for > now. Here, I am imagining a secondary process attaching to DPDK shared memory, but not doing any device I/O. For a primary process, I agree 100 % that it is not much of a DPDK application without ethdev. > > > fib - OPTIONAL > Used by l3fwd > > > hash - OPTIONAL (if not used by CORE libs) > Needed by l3fwd. Also listed as a dependency by a number of drivers, > e.g. > i40e, mlx5, nfp, tap. I think they use it for holding filters and > things. > This I'd be wary about allowing disabling without some work, as it > could > cause users surprise when some drivers unexpectedly stop building. It is fair enough if some drivers depend on the hash lib; then they should register that dependency in the build system. In principle, if someone builds DPDK without any of those drivers, so this lib is unused, why should it be included? This lib has a small footprint, so I don't feel strongly about it. > > > kvargs - OPTIONAL (if not used by EAL) > Used by EAL > > > log - CORE > > lpm - OPTIONAL > Used by l3fwd > > > mbuf - CORE > > mempool - CORE > > meter - OPTIONAL > Used by ethdev. I wonder how this became a mandatory part of ethdev... It should be optional if not used by the application. This seems to be EAL all over again... bloat is integrated deeply into EAL, and cannot be removed/disabled. Now, this other bloat is deeply integrated into ethdev, and cannot be removed/disabled. > > > net - CORE > > pci - CORE, or OPTIONAL for secondary process > > rcu - OPTIONAL (if not used by CORE libs) > Used by lpm and hash, which are used by l3fwd. > > > rib - OPTIONAL > Used by fib, used by l3fwd. > > > ring - CORE > Actually, this is one I'd look to maybe say optional. :-) The ring is also used for pipelined applications. (Which in itself doesn't make it mandatory; run-to-completion applications might not need it.) > > I think we should switch our default mempool backend from ring to > stack, > and make that mandatory. The reason being that run-to-completion apps > should be running entirely out of mempool cache and not care about the > underlying driver, while pipelined apps pushing buffers across cores > would > be better with a LIFO-based rather than FIFO-based mempool driver. > Therefore, I think overall switching to stack from ring would be an > overall > win for performance. As a mempool backend, you are probably right that a stack would provide better performance for pipelined applications (due to improved cache utilization). But isn't the stack backend using locks, so it would run into problems if used by an unregistered non-EAL thread? Although I haven't looked into the details, I am under the impression that some of the newer ring backends (Relaxed Tail Sync or Head/Tail Sync) don't have this problem. > > > stack - OPTIONAL (if not used by CORE libs) > For reasons above, I'd make this core. We need at least one mempool backend, so either ring or stack must be mandatory. > > > telemetry - OPTIONAL > Used by EAL This is mega-bloat, and I am surprised that it is not optional! Not all apps use telemetry, and previous DPDK releases were able to provide an *environment abstraction layer* without considering telemetry a CORE component of making applications run on some underlying hardware/environment. An excellent example of non-mandatory stuff added to EAL, instead of keeping EAL lean and mean. This also raises the barrier to implementing an EAL for some other hardware/environment. Long term, I am hoping for a barebone DPDK be able to run on low-end CPE hardware. I we keep moving down the path of adding bloat to DPDK EAL and CORE, it will eventually require 32 GB RAM and 4 CPU cores to fire up DPDK. > > > timer - OPTIONAL (if not used by CORE libs) > Only lib that seems to depend on this is eventdev, so maybe this can be > an > easy removal. Though I do see it listed as a dependency of ENA driver > too. Excellent. If dependencies are set up correctly, it can be omitted from CORE. > > > > > Anyway, > > > > Acked-by: Morten Brørup <mb@smartsharesystems.com> > > > > Overall, if we want to make more libs optional, I would start looking > at > l3fwd and making it a bit more modular. I previously made its support > for > eventdev optional, we should do the same for lpm and fib. Beyond that, > we > need to decide what core really means. Yes - defining CORE is the key to setting the goal here. In my mind, CORE is the minimum requirement to running an absolutely minimal DPDK application. A primary DPDK application would probably need to do some packet I/O; but it might be a simple layer two bridge, not using any of the L3 libs. And a secondary DPDK application might attach to a primary DPDK application only to work on its data structures, e.g. to collect statistics, but not do any packet processing, so that application doesn't need any of those libs (not even the ethdev lib). In reality, DPDK applications would probably need to build more libs than just CORE. But some application might need CORE + lib A, and some other application might need CORE + lib B. In essence, I don't want application A to drag around some unused lib B, and application B to drag around some unused lib A. It's an optimization only available a build time. Distros should continue providing all DPDK libs. There's also system testing and system attack surface to consider... all that bloat makes production systems more fragile and vulnerable. -Morten ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-03 20:19 ` Morten Brørup @ 2023-11-06 10:28 ` Bruce Richardson 2023-11-06 11:22 ` Morten Brørup 0 siblings, 1 reply; 20+ messages in thread From: Bruce Richardson @ 2023-11-06 10:28 UTC (permalink / raw) To: Morten Brørup; +Cc: dev On Fri, Nov 03, 2023 at 09:19:53PM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > Sent: Friday, 3 November 2023 19.09 > > > > On Fri, Nov 03, 2023 at 06:31:30PM +0100, Morten Brørup wrote: > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > Sent: Friday, 3 November 2023 17.52 > > > > > > > > DPDK now has more optional libraries than mandatory ones, so invert > > the > > > > list stored in the meson.build file from the optional ones to the > > > > "always_enable" ones. As well as being a shorter list: > > > > > > > > * we can remove the loop building up the "always_enable" list > > > > dynamically from the optional list > > > > * it better aligns with the drivers/meson.build file which > > maintains an > > > > always_enable list. > > > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > > > > Excellent! > > > > > > It really shows how bloated DPDK CORE still is. I would like to see > > these go optional: > > > > > > > For some I agree, but we need to decide what optional really means. :-) > > > > For my mind, there are 3 (maybe 4) key components that need to be built > > for > > me to consider a build to be a valid DPDK one: > > * EAL obviously, > > * testpmd, because everyone seems to use it > > * l3fwd, becaues it's the most commonly referenced example and used for > > benchmarking, and build testing in test-meson-builds. (There are > > others, > > but they are pretty likely to build if l3fwd does!) > > * dpdk-test - I feel this should always be buildable, but for me it's > > the > > optional 4th component. > > > > Now, the obviously one to relax here is l3fwd, since it is just an > > example, > > but I wonder if that may cause some heartache. > > I don't consider any DPDK lib CORE just because the lib is used by testpmd and/or l3fwd. I agree that all libs should be included by default, so you can run testpmd, l3fwd, and other apps and examples. > > However, many libs are not needed for *all* DPDK applications, so I would like other apps to be able to build DPDK without superfluous libs. > > E.g. our StraightShaper CSP appliance is deployed at Layer 2, and doesn't use any of DPDK's L3 libs, so why should the DPDK L3 libs be considered CORE and thus included in our application? I suppose other companies are also using DPDK for other purposes than L3 routing, and don't need the DPDK L3 libs. > > Furthermore, I suppose that some Layer 3 applications use their own RIB/FIB/LPM libraries. Does OVS use DPDK's rib/fib/lpm libraries? > <snip for brevity> > > Overall, if we want to make more libs optional, I would start looking > > at > > l3fwd and making it a bit more modular. I previously made its support > > for > > eventdev optional, we should do the same for lpm and fib. Beyond that, > > we > > need to decide what core really means. > > Yes - defining CORE is the key to setting the goal here. > > In my mind, CORE is the minimum requirement to running an absolutely minimal DPDK application. > > A primary DPDK application would probably need to do some packet I/O; but it might be a simple layer two bridge, not using any of the L3 libs. > > And a secondary DPDK application might attach to a primary DPDK application only to work on its data structures, e.g. to collect statistics, but not do any packet processing, so that application doesn't need any of those libs (not even the ethdev lib). > > In reality, DPDK applications would probably need to build more libs than just CORE. But some application might need CORE + lib A, and some other application might need CORE + lib B. In essence, I don't want application A to drag around some unused lib B, and application B to drag around some unused lib A. > > It's an optimization only available a build time. Distros should continue providing all DPDK libs. > > There's also system testing and system attack surface to consider... all that bloat makes production systems more fragile and vulnerable. > I largely agree, though I do think that trying to split primary-secondary as having different builds could lead to some headaches, so I'd push any work around that further down the road. Some thoughts on next steps: * From looks of my original list above, it appears the low-hanging fruit is largely gone, in terms of being able to turn off libs that have few dependencies, timer being one possible exception * I think it's worth looking into making l3fwd more modular so it can be build only with backend x or y or z in it. However, if agreeable, we can just start marking lpm and rib/fib libs as optional directly and have l3fwd not buildable in those cases. * For libs that depend on other libs for bits of functionality, we are getting into the realm of using ifdefs to start selectively removing bits. This is the not-so-nice bit as: - it makes it a lot harder to do proper build testing, as we now have to test with individual bits on and off. So long as we were just enabling/ disabling whole components, the build-minimal target was good enough to test we had a working build. With some libs partially depending on others - both of which may be disablable independently - our build test matrix just explodes. - #ifdefs are just really, really ugly in the code, and make it far harder to maintain and manage. Therefore, I'm wondering if we can come up with some sort of neater solution here. For example, can we add support to the build system that allows some form of stubbing out of libraries when they are disabled? That would save the putting of ifdefs throughout other parts of DPDK and keep the management of the disabling of the library someway inside the library itself. One way to do this might be to have a "stub" folder inside a library folder, where that contains a minimal header file to be used to provide empty functions in case where the lib itself is disabled. Other, more interesting schemes, might involve the automatic creation - from the version.map file - of dummy functions for dependent libs to link against on build. /Bruce ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-06 10:28 ` Bruce Richardson @ 2023-11-06 11:22 ` Morten Brørup 2023-11-06 11:27 ` Bruce Richardson 0 siblings, 1 reply; 20+ messages in thread From: Morten Brørup @ 2023-11-06 11:22 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Monday, 6 November 2023 11.29 > > On Fri, Nov 03, 2023 at 09:19:53PM +0100, Morten Brørup wrote: > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: Friday, 3 November 2023 19.09 > > > > > > On Fri, Nov 03, 2023 at 06:31:30PM +0100, Morten Brørup wrote: > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > > Sent: Friday, 3 November 2023 17.52 > > > > > > > > > > DPDK now has more optional libraries than mandatory ones, so > invert > > > the > > > > > list stored in the meson.build file from the optional ones to > the > > > > > "always_enable" ones. As well as being a shorter list: > > > > > > > > > > * we can remove the loop building up the "always_enable" list > > > > > dynamically from the optional list > > > > > * it better aligns with the drivers/meson.build file which > > > maintains an > > > > > always_enable list. > > > > > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > > > > > > Excellent! > > > > > > > > It really shows how bloated DPDK CORE still is. I would like to > see > > > these go optional: > > > > > > > > > > For some I agree, but we need to decide what optional really means. > :-) > > > > > > For my mind, there are 3 (maybe 4) key components that need to be > built > > > for > > > me to consider a build to be a valid DPDK one: > > > * EAL obviously, > > > * testpmd, because everyone seems to use it > > > * l3fwd, becaues it's the most commonly referenced example and used > for > > > benchmarking, and build testing in test-meson-builds. (There are > > > others, > > > but they are pretty likely to build if l3fwd does!) > > > * dpdk-test - I feel this should always be buildable, but for me > it's > > > the > > > optional 4th component. > > > > > > Now, the obviously one to relax here is l3fwd, since it is just an > > > example, > > > but I wonder if that may cause some heartache. > > > > I don't consider any DPDK lib CORE just because the lib is used by > testpmd and/or l3fwd. I agree that all libs should be included by > default, so you can run testpmd, l3fwd, and other apps and examples. > > > > However, many libs are not needed for *all* DPDK applications, so I > would like other apps to be able to build DPDK without superfluous > libs. > > > > E.g. our StraightShaper CSP appliance is deployed at Layer 2, and > doesn't use any of DPDK's L3 libs, so why should the DPDK L3 libs be > considered CORE and thus included in our application? I suppose other > companies are also using DPDK for other purposes than L3 routing, and > don't need the DPDK L3 libs. > > > > Furthermore, I suppose that some Layer 3 applications use their own > RIB/FIB/LPM libraries. Does OVS use DPDK's rib/fib/lpm libraries? > > > > <snip for brevity> > > > > Overall, if we want to make more libs optional, I would start > looking > > > at > > > l3fwd and making it a bit more modular. I previously made its > support > > > for > > > eventdev optional, we should do the same for lpm and fib. Beyond > that, > > > we > > > need to decide what core really means. > > > > Yes - defining CORE is the key to setting the goal here. > > > > In my mind, CORE is the minimum requirement to running an absolutely > minimal DPDK application. > > > > A primary DPDK application would probably need to do some packet I/O; > but it might be a simple layer two bridge, not using any of the L3 > libs. > > > > And a secondary DPDK application might attach to a primary DPDK > application only to work on its data structures, e.g. to collect > statistics, but not do any packet processing, so that application > doesn't need any of those libs (not even the ethdev lib). > > > > In reality, DPDK applications would probably need to build more libs > than just CORE. But some application might need CORE + lib A, and some > other application might need CORE + lib B. In essence, I don't want > application A to drag around some unused lib B, and application B to > drag around some unused lib A. > > > > It's an optimization only available a build time. Distros should > continue providing all DPDK libs. > > > > There's also system testing and system attack surface to consider... > all that bloat makes production systems more fragile and vulnerable. > > > > I largely agree, though I do think that trying to split primary- > secondary > as having different builds could lead to some headaches, so I'd push > any > work around that further down the road. You are probably right that running a secondary process built differently than the primary process will cause an avalanche of new challenges, so I strongly agree to pushing it further down the road. I don't even know if there is any demand for such a secondary process. (We considered something like this for our application, but did something else instead.) Starting the secondary process with some additional run-time parameters will have to suffice. > > Some thoughts on next steps: > * From looks of my original list above, it appears the low-hanging > fruit is > largely gone, in terms of being able to turn off libs that have few > dependencies, timer being one possible exception > * I think it's worth looking into making l3fwd more modular so it can > be > build only with backend x or y or z in it. However, if agreeable, we > can > just start marking lpm and rib/fib libs as optional directly and have > l3fwd not buildable in those cases. I agree with that. (It would also affect the variants of l3fwd.) > * For libs that depend on other libs for bits of functionality, we are > getting into the realm of using ifdefs to start selectively removing > bits. This is the not-so-nice bit as: > > - it makes it a lot harder to do proper build testing, as we now have > to > test with individual bits on and off. So long as we were just > enabling/ > disabling whole components, the build-minimal target was good > enough to > test we had a working build. With some libs partially depending on > others - both of which may be disablable independently - our build > test > matrix just explodes. We could start without the matrix, and have the CI build just two or three variants: 1. Everything (like now), 2. CORE only, and 3. CORE + all drivers with their dependencies. > - #ifdefs are just really, really ugly in the code, and make it far > harder to maintain and manage. > > Therefore, I'm wondering if we can come up with some sort of neater > solution here. > > For example, can we add support to the build system that allows some > form > of stubbing out of libraries when they are disabled? That would save > the > putting of ifdefs throughout other parts of DPDK and keep the > management of > the disabling of the library someway inside the library itself. One way > to > do this might be to have a "stub" folder inside a library folder, where > that contains a minimal header file to be used to provide empty > functions > in case where the lib itself is disabled. Other, more interesting > schemes, > might involve the automatic creation - from the version.map file - of > dummy > functions for dependent libs to link against on build. If we stub out a library, we have to somehow ensure that no application/driver/library calls that library, expecting it to work, if the library disabled. Preferably, this should fail at build time. Although stubbing out libraries might not be a generic solution, it would probably work for some libraries, e.g. trace points, where empty functions do no harm. But they have to be empty in the header file, so the calling code (and associated data) will be optimized away by the compiler. I can come up with one example where we already do this: __rte_mbuf_sanity_check() [1]. [1]: https://elixir.bootlin.com/dpdk/v23.11-rc1/source/lib/mbuf/rte_mbuf.h#L340 PS: I'm not scared of #ifdefs. Many years ago I was involved in a project where one code base was used for generating a variety of firmware for both unmanaged and managed switches (using related families of silicon), controlled at build time by #ifdefs. Consolidating the source code like that worked very well there. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-06 11:22 ` Morten Brørup @ 2023-11-06 11:27 ` Bruce Richardson 2023-11-06 11:37 ` Morten Brørup 0 siblings, 1 reply; 20+ messages in thread From: Bruce Richardson @ 2023-11-06 11:27 UTC (permalink / raw) To: Morten Brørup; +Cc: dev On Mon, Nov 06, 2023 at 12:22:57PM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > Sent: Monday, 6 November 2023 11.29 > > > > On Fri, Nov 03, 2023 at 09:19:53PM +0100, Morten Brørup wrote: > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > Sent: Friday, 3 November 2023 19.09 > > > > > > > > On Fri, Nov 03, 2023 at 06:31:30PM +0100, Morten Brørup wrote: > > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > > > Sent: Friday, 3 November 2023 17.52 > > > > > > > > > > > > DPDK now has more optional libraries than mandatory ones, so > > invert > > > > the > > > > > > list stored in the meson.build file from the optional ones to > > the > > > > > > "always_enable" ones. As well as being a shorter list: > > > > > > > > > > > > * we can remove the loop building up the "always_enable" list > > > > > > dynamically from the optional list > > > > > > * it better aligns with the drivers/meson.build file which > > > > maintains an > > > > > > always_enable list. > > > > > > > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > > > > > > > > Excellent! > > > > > > > > > > It really shows how bloated DPDK CORE still is. I would like to > > see > > > > these go optional: > > > > > > > > > > > > > For some I agree, but we need to decide what optional really means. > > :-) > > > > > > > > For my mind, there are 3 (maybe 4) key components that need to be > > built > > > > for > > > > me to consider a build to be a valid DPDK one: > > > > * EAL obviously, > > > > * testpmd, because everyone seems to use it > > > > * l3fwd, becaues it's the most commonly referenced example and used > > for > > > > benchmarking, and build testing in test-meson-builds. (There are > > > > others, > > > > but they are pretty likely to build if l3fwd does!) > > > > * dpdk-test - I feel this should always be buildable, but for me > > it's > > > > the > > > > optional 4th component. > > > > > > > > Now, the obviously one to relax here is l3fwd, since it is just an > > > > example, > > > > but I wonder if that may cause some heartache. > > > > > > I don't consider any DPDK lib CORE just because the lib is used by > > testpmd and/or l3fwd. I agree that all libs should be included by > > default, so you can run testpmd, l3fwd, and other apps and examples. > > > > > > However, many libs are not needed for *all* DPDK applications, so I > > would like other apps to be able to build DPDK without superfluous > > libs. > > > > > > E.g. our StraightShaper CSP appliance is deployed at Layer 2, and > > doesn't use any of DPDK's L3 libs, so why should the DPDK L3 libs be > > considered CORE and thus included in our application? I suppose other > > companies are also using DPDK for other purposes than L3 routing, and > > don't need the DPDK L3 libs. > > > > > > Furthermore, I suppose that some Layer 3 applications use their own > > RIB/FIB/LPM libraries. Does OVS use DPDK's rib/fib/lpm libraries? > > > > > > > <snip for brevity> > > > > > > Overall, if we want to make more libs optional, I would start > > looking > > > > at > > > > l3fwd and making it a bit more modular. I previously made its > > support > > > > for > > > > eventdev optional, we should do the same for lpm and fib. Beyond > > that, > > > > we > > > > need to decide what core really means. > > > > > > Yes - defining CORE is the key to setting the goal here. > > > > > > In my mind, CORE is the minimum requirement to running an absolutely > > minimal DPDK application. > > > > > > A primary DPDK application would probably need to do some packet I/O; > > but it might be a simple layer two bridge, not using any of the L3 > > libs. > > > > > > And a secondary DPDK application might attach to a primary DPDK > > application only to work on its data structures, e.g. to collect > > statistics, but not do any packet processing, so that application > > doesn't need any of those libs (not even the ethdev lib). > > > > > > In reality, DPDK applications would probably need to build more libs > > than just CORE. But some application might need CORE + lib A, and some > > other application might need CORE + lib B. In essence, I don't want > > application A to drag around some unused lib B, and application B to > > drag around some unused lib A. > > > > > > It's an optimization only available a build time. Distros should > > continue providing all DPDK libs. > > > > > > There's also system testing and system attack surface to consider... > > all that bloat makes production systems more fragile and vulnerable. > > > > > > > I largely agree, though I do think that trying to split primary- > > secondary > > as having different builds could lead to some headaches, so I'd push > > any > > work around that further down the road. > > You are probably right that running a secondary process built differently than the primary process will cause an avalanche of new challenges, so I strongly agree to pushing it further down the road. I don't even know if there is any demand for such a secondary process. (We considered something like this for our application, but did something else instead.) Starting the secondary process with some additional run-time parameters will have to suffice. > > > > > Some thoughts on next steps: > > * From looks of my original list above, it appears the low-hanging > > fruit is > > largely gone, in terms of being able to turn off libs that have few > > dependencies, timer being one possible exception > > * I think it's worth looking into making l3fwd more modular so it can > > be > > build only with backend x or y or z in it. However, if agreeable, we > > can > > just start marking lpm and rib/fib libs as optional directly and have > > l3fwd not buildable in those cases. > > I agree with that. (It would also affect the variants of l3fwd.) > > > * For libs that depend on other libs for bits of functionality, we are > > getting into the realm of using ifdefs to start selectively removing > > bits. This is the not-so-nice bit as: > > > > - it makes it a lot harder to do proper build testing, as we now have > > to > > test with individual bits on and off. So long as we were just > > enabling/ > > disabling whole components, the build-minimal target was good > > enough to > > test we had a working build. With some libs partially depending on > > others - both of which may be disablable independently - our build > > test > > matrix just explodes. > > We could start without the matrix, and have the CI build just two or three variants: > 1. Everything (like now), > 2. CORE only, and > 3. CORE + all drivers with their dependencies. > > > - #ifdefs are just really, really ugly in the code, and make it far > > harder to maintain and manage. > > > > Therefore, I'm wondering if we can come up with some sort of neater > > solution here. > > > > For example, can we add support to the build system that allows some > > form > > of stubbing out of libraries when they are disabled? That would save > > the > > putting of ifdefs throughout other parts of DPDK and keep the > > management of > > the disabling of the library someway inside the library itself. One way > > to > > do this might be to have a "stub" folder inside a library folder, where > > that contains a minimal header file to be used to provide empty > > functions > > in case where the lib itself is disabled. Other, more interesting > > schemes, > > might involve the automatic creation - from the version.map file - of > > dummy > > functions for dependent libs to link against on build. > > If we stub out a library, we have to somehow ensure that no application/driver/library calls that library, expecting it to work, if the library disabled. Preferably, this should fail at build time. > My thinking was that any stubs would only be available internally at build time. For example, we could have libname.h and stubs/libname.h, where stubs/libname.h is never installed or exported for application use. We definitely cannot have stubs generally available to apps. /Bruce ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 24.03 v2] build: track mandatory rather than optional libs 2023-11-06 11:27 ` Bruce Richardson @ 2023-11-06 11:37 ` Morten Brørup 0 siblings, 0 replies; 20+ messages in thread From: Morten Brørup @ 2023-11-06 11:37 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Monday, 6 November 2023 12.27 > > On Mon, Nov 06, 2023 at 12:22:57PM +0100, Morten Brørup wrote: > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: Monday, 6 November 2023 11.29 > > > > > > On Fri, Nov 03, 2023 at 09:19:53PM +0100, Morten Brørup wrote: > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > > Sent: Friday, 3 November 2023 19.09 > > > > > > > > > > On Fri, Nov 03, 2023 at 06:31:30PM +0100, Morten Brørup wrote: > > > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > > > > Sent: Friday, 3 November 2023 17.52 > > > > > > > > > > > > > > DPDK now has more optional libraries than mandatory ones, > so > > > invert > > > > > the > > > > > > > list stored in the meson.build file from the optional ones > to > > > the > > > > > > > "always_enable" ones. As well as being a shorter list: > > > > > > > > > > > > > > * we can remove the loop building up the "always_enable" > list > > > > > > > dynamically from the optional list > > > > > > > * it better aligns with the drivers/meson.build file which > > > > > maintains an > > > > > > > always_enable list. > > > > > > > > > > > > > > Signed-off-by: Bruce Richardson > <bruce.richardson@intel.com> > > > > > > > > > > > > Excellent! > > > > > > > > > > > > It really shows how bloated DPDK CORE still is. I would like > to > > > see > > > > > these go optional: > > > > > > > > > > > > > > > > For some I agree, but we need to decide what optional really > means. > > > :-) > > > > > > > > > > For my mind, there are 3 (maybe 4) key components that need to > be > > > built > > > > > for > > > > > me to consider a build to be a valid DPDK one: > > > > > * EAL obviously, > > > > > * testpmd, because everyone seems to use it > > > > > * l3fwd, becaues it's the most commonly referenced example and > used > > > for > > > > > benchmarking, and build testing in test-meson-builds. (There > are > > > > > others, > > > > > but they are pretty likely to build if l3fwd does!) > > > > > * dpdk-test - I feel this should always be buildable, but for > me > > > it's > > > > > the > > > > > optional 4th component. > > > > > > > > > > Now, the obviously one to relax here is l3fwd, since it is just > an > > > > > example, > > > > > but I wonder if that may cause some heartache. > > > > > > > > I don't consider any DPDK lib CORE just because the lib is used > by > > > testpmd and/or l3fwd. I agree that all libs should be included by > > > default, so you can run testpmd, l3fwd, and other apps and > examples. > > > > > > > > However, many libs are not needed for *all* DPDK applications, so > I > > > would like other apps to be able to build DPDK without superfluous > > > libs. > > > > > > > > E.g. our StraightShaper CSP appliance is deployed at Layer 2, and > > > doesn't use any of DPDK's L3 libs, so why should the DPDK L3 libs > be > > > considered CORE and thus included in our application? I suppose > other > > > companies are also using DPDK for other purposes than L3 routing, > and > > > don't need the DPDK L3 libs. > > > > > > > > Furthermore, I suppose that some Layer 3 applications use their > own > > > RIB/FIB/LPM libraries. Does OVS use DPDK's rib/fib/lpm libraries? > > > > > > > > > > <snip for brevity> > > > > > > > > Overall, if we want to make more libs optional, I would start > > > looking > > > > > at > > > > > l3fwd and making it a bit more modular. I previously made its > > > support > > > > > for > > > > > eventdev optional, we should do the same for lpm and fib. > Beyond > > > that, > > > > > we > > > > > need to decide what core really means. > > > > > > > > Yes - defining CORE is the key to setting the goal here. > > > > > > > > In my mind, CORE is the minimum requirement to running an > absolutely > > > minimal DPDK application. > > > > > > > > A primary DPDK application would probably need to do some packet > I/O; > > > but it might be a simple layer two bridge, not using any of the L3 > > > libs. > > > > > > > > And a secondary DPDK application might attach to a primary DPDK > > > application only to work on its data structures, e.g. to collect > > > statistics, but not do any packet processing, so that application > > > doesn't need any of those libs (not even the ethdev lib). > > > > > > > > In reality, DPDK applications would probably need to build more > libs > > > than just CORE. But some application might need CORE + lib A, and > some > > > other application might need CORE + lib B. In essence, I don't want > > > application A to drag around some unused lib B, and application B > to > > > drag around some unused lib A. > > > > > > > > It's an optimization only available a build time. Distros should > > > continue providing all DPDK libs. > > > > > > > > There's also system testing and system attack surface to > consider... > > > all that bloat makes production systems more fragile and > vulnerable. > > > > > > > > > > I largely agree, though I do think that trying to split primary- > > > secondary > > > as having different builds could lead to some headaches, so I'd > push > > > any > > > work around that further down the road. > > > > You are probably right that running a secondary process built > differently than the primary process will cause an avalanche of new > challenges, so I strongly agree to pushing it further down the road. I > don't even know if there is any demand for such a secondary process. > (We considered something like this for our application, but did > something else instead.) Starting the secondary process with some > additional run-time parameters will have to suffice. > > > > > > > > Some thoughts on next steps: > > > * From looks of my original list above, it appears the low-hanging > > > fruit is > > > largely gone, in terms of being able to turn off libs that have > few > > > dependencies, timer being one possible exception > > > * I think it's worth looking into making l3fwd more modular so it > can > > > be > > > build only with backend x or y or z in it. However, if agreeable, > we > > > can > > > just start marking lpm and rib/fib libs as optional directly and > have > > > l3fwd not buildable in those cases. > > > > I agree with that. (It would also affect the variants of l3fwd.) > > > > > * For libs that depend on other libs for bits of functionality, we > are > > > getting into the realm of using ifdefs to start selectively > removing > > > bits. This is the not-so-nice bit as: > > > > > > - it makes it a lot harder to do proper build testing, as we now > have > > > to > > > test with individual bits on and off. So long as we were just > > > enabling/ > > > disabling whole components, the build-minimal target was good > > > enough to > > > test we had a working build. With some libs partially depending > on > > > others - both of which may be disablable independently - our > build > > > test > > > matrix just explodes. > > > > We could start without the matrix, and have the CI build just two or > three variants: > > 1. Everything (like now), > > 2. CORE only, and > > 3. CORE + all drivers with their dependencies. > > > > > - #ifdefs are just really, really ugly in the code, and make it > far > > > harder to maintain and manage. > > > > > > Therefore, I'm wondering if we can come up with some sort of neater > > > solution here. > > > > > > For example, can we add support to the build system that allows > some > > > form > > > of stubbing out of libraries when they are disabled? That would > save > > > the > > > putting of ifdefs throughout other parts of DPDK and keep the > > > management of > > > the disabling of the library someway inside the library itself. One > way > > > to > > > do this might be to have a "stub" folder inside a library folder, > where > > > that contains a minimal header file to be used to provide empty > > > functions > > > in case where the lib itself is disabled. Other, more interesting > > > schemes, > > > might involve the automatic creation - from the version.map file - > of > > > dummy > > > functions for dependent libs to link against on build. > > > > If we stub out a library, we have to somehow ensure that no > application/driver/library calls that library, expecting it to work, if > the library disabled. Preferably, this should fail at build time. > > > > My thinking was that any stubs would only be available internally at > build > time. For example, we could have libname.h and stubs/libname.h, where > stubs/libname.h is never installed or exported for application use. We > definitely cannot have stubs generally available to apps. That would eliminate the risk for applications, yes. The risk of using them (and expecting them to work) would still be there for DPDK libs and drivers. Nonetheless, it might the preferred alternative to #ifdefs is some cases. And in some cases it might be an easier-to-reach first step for making a lib optional. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 0/3] Improve optional lib support 2023-11-03 16:28 [PATCH 24.03] build: track mandatory rather than optional libs Bruce Richardson 2023-11-03 16:52 ` [PATCH 24.03 v2] " Bruce Richardson @ 2023-12-20 14:21 ` Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 1/3] build: track mandatory rather than optional libs Bruce Richardson ` (3 more replies) 1 sibling, 4 replies; 20+ messages in thread From: Bruce Richardson @ 2023-12-20 14:21 UTC (permalink / raw) To: dev; +Cc: mb, Bruce Richardson This patchset builds on the previous v1 and v2, and the discussion which followed about what libraries should be optional. While still a long way to go from the position suggested in the v2 discussion, this set moves us a bit further along the road. The first patch is a direct v3 from previous versions, changing the tracking from a list of optional libs to a list of mandatory ones. The second patch then knocks off another 5 libs off the mandatory list - all are libs which should have few dependencies in other components, but which some common sample apps like l3fwd depend upon. It should be possible to make other libs optional in future, but to do so likely requires code-changes to #ifdef out support in places. To that end, patch 3 is an RFC to add support for "optional libs" to the build system. For now, nothing uses this, but it may be useful if someone does want to take on the task of removing a hard dependency from one component on another and making it optional. Bruce Richardson (3): build: track mandatory rather than optional libs build: remove 5 libs from mandatory list build: RFC - add support for optional dependencies app/test/meson.build | 4 +-- drivers/meson.build | 7 +++++ lib/meson.build | 72 ++++++++++++++++---------------------------- 3 files changed, 35 insertions(+), 48 deletions(-) -- 2.40.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 1/3] build: track mandatory rather than optional libs 2023-12-20 14:21 ` [PATCH v3 0/3] Improve optional lib support Bruce Richardson @ 2023-12-20 14:21 ` Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 2/3] build: remove 5 libs from mandatory list Bruce Richardson ` (2 subsequent siblings) 3 siblings, 0 replies; 20+ messages in thread From: Bruce Richardson @ 2023-12-20 14:21 UTC (permalink / raw) To: dev; +Cc: mb, Bruce Richardson DPDK now has more optional libraries than mandatory ones, so invert the list stored in the meson.build file from the optional ones to the "always_enable" ones. As well as being a shorter list: * we can remove the loop building up the "always_enable" list dynamically from the optional list * it better aligns with the drivers/meson.build file which maintains an always_enable list. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Morten Brørup <mb@smartsharesystems.com> --- lib/meson.build | 70 +++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/lib/meson.build b/lib/meson.build index 6c143ce5a6..8c922d3097 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -75,52 +75,29 @@ if is_ms_compiler ] endif -optional_libs = [ - 'bbdev', - 'bitratestats', - 'bpf', - 'cfgfile', - 'compressdev', - 'cryptodev', - 'dispatcher', - 'distributor', - 'dmadev', - 'efd', - 'eventdev', - 'gpudev', - 'graph', - 'gro', - 'gso', - 'ip_frag', - 'ipsec', - 'jobstats', - 'latencystats', - 'member', - 'metrics', - 'mldev', - 'node', - 'pcapng', - 'pdcp', - 'pdump', - 'pipeline', - 'port', - 'power', - 'rawdev', - 'regexdev', - 'reorder', - 'sched', - 'security', - 'table', - 'vhost', +always_enable = [ + 'acl', + 'cmdline', + 'eal', + 'ethdev', + 'fib', + 'hash', + 'kvargs', + 'log', + 'lpm', + 'mbuf', + 'mempool', + 'meter', + 'net', + 'pci', + 'rcu', + 'rib', + 'ring', + 'stack', + 'telemetry', + 'timer', ] -always_enable = [] -foreach l:libraries - if not optional_libs.contains(l) - always_enable += l - endif -endforeach - enable_deprecated_libs = [] foreach l:run_command(list_dir_globs, get_option('enable_deprecated_libs'), check: true).stdout().split() @@ -136,9 +113,10 @@ enable_libs = run_command(list_dir_globs, get_option('enable_libs'), check: true require_libs = true if enable_libs.length() == 0 require_libs = false - enable_libs += optional_libs + enable_libs = libraries +else + enable_libs += always_enable endif -enable_libs += always_enable default_cflags = machine_args default_cflags += ['-DALLOW_EXPERIMENTAL_API'] -- 2.40.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 2/3] build: remove 5 libs from mandatory list 2023-12-20 14:21 ` [PATCH v3 0/3] Improve optional lib support Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 1/3] build: track mandatory rather than optional libs Bruce Richardson @ 2023-12-20 14:21 ` Bruce Richardson 2023-12-20 15:18 ` Morten Brørup 2023-12-20 14:21 ` [PATCH v3 3/3] build: RFC - add support for optional dependencies Bruce Richardson 2024-02-01 9:23 ` [PATCH v3 0/3] Improve optional lib support David Marchand 3 siblings, 1 reply; 20+ messages in thread From: Bruce Richardson @ 2023-12-20 14:21 UTC (permalink / raw) To: dev; +Cc: mb, Bruce Richardson Remove five more libs from the mandatory build list. Only one needing any special treatment is LPM, which is an optional dependency for some secondary process autotests. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- app/test/meson.build | 4 ++-- lib/meson.build | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/test/meson.build b/app/test/meson.build index dcc93f4a43..6389ae83ee 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -7,7 +7,7 @@ sources += files('commands.c', 'test.c') # optional dependencies: some files may use these - and so we should link them in - # but do not explicitly require them so they are not listed in the per-file lists below -optional_deps = ['crypto_scheduler'] +optional_deps = ['crypto_scheduler', 'lpm'] # some other utility C files, providing functions used by various tests # so we need to include these deps in the dependency list for the files using those fns. @@ -126,7 +126,7 @@ source_file_deps = { 'test_memzone.c': [], 'test_meter.c': ['meter'], 'test_metrics.c': ['metrics'], - 'test_mp_secondary.c': ['hash', 'lpm'], + 'test_mp_secondary.c': ['hash'], 'test_net_ether.c': ['net'], 'test_pcapng.c': ['ethdev', 'net', 'pcapng', 'bus_vdev'], 'test_pdcp.c': ['eventdev', 'pdcp', 'net', 'timer', 'security'], diff --git a/lib/meson.build b/lib/meson.build index 8c922d3097..72e9138d14 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -76,26 +76,21 @@ if is_ms_compiler endif always_enable = [ - 'acl', 'cmdline', 'eal', 'ethdev', - 'fib', 'hash', 'kvargs', 'log', - 'lpm', 'mbuf', 'mempool', 'meter', 'net', 'pci', 'rcu', - 'rib', 'ring', 'stack', 'telemetry', - 'timer', ] enable_deprecated_libs = [] -- 2.40.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* RE: [PATCH v3 2/3] build: remove 5 libs from mandatory list 2023-12-20 14:21 ` [PATCH v3 2/3] build: remove 5 libs from mandatory list Bruce Richardson @ 2023-12-20 15:18 ` Morten Brørup 2023-12-20 16:05 ` Bruce Richardson 0 siblings, 1 reply; 20+ messages in thread From: Morten Brørup @ 2023-12-20 15:18 UTC (permalink / raw) To: Bruce Richardson, dev > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Wednesday, 20 December 2023 15.22 > > Remove five more libs from the mandatory build list. Only one needing > any special treatment is LPM, which is an optional dependency for some > secondary process autotests. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- Good stuff. Acked-by: Morten Brørup <mb@smartsharesystems.com> <rant> As previously discussed, I would like to see 'meter' become optional too, but it's probably too deeply embedded into the ethdev lib. And I somehow missed that the 'telemetry' lib was not optional when it was added, so now we're stuck with all the bloat that comes with it. :-( </rant> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] build: remove 5 libs from mandatory list 2023-12-20 15:18 ` Morten Brørup @ 2023-12-20 16:05 ` Bruce Richardson 0 siblings, 0 replies; 20+ messages in thread From: Bruce Richardson @ 2023-12-20 16:05 UTC (permalink / raw) To: Morten Brørup; +Cc: dev On Wed, Dec 20, 2023 at 04:18:32PM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > Sent: Wednesday, 20 December 2023 15.22 > > > > Remove five more libs from the mandatory build list. Only one needing > > any special treatment is LPM, which is an optional dependency for some > > secondary process autotests. > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > --- > > Good stuff. > > Acked-by: Morten Brørup <mb@smartsharesystems.com> > > <rant> > As previously discussed, I would like to see 'meter' become optional too, but it's probably too deeply embedded into the ethdev lib. > The metering stuff I think is in separate files, so it might not be too difficult to separate. > And I somehow missed that the 'telemetry' lib was not optional when it was added, so now we're stuck with all the bloat that comes with it. :-( I've been thinking about this, and I will hopefully try prototyping some ways to make it optional in future. Will probably need function stubs somewhere to make things clean. As with the meter stuff, it's probably just a matter of having a bit of time to look at it. /Bruce ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 3/3] build: RFC - add support for optional dependencies 2023-12-20 14:21 ` [PATCH v3 0/3] Improve optional lib support Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 1/3] build: track mandatory rather than optional libs Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 2/3] build: remove 5 libs from mandatory list Bruce Richardson @ 2023-12-20 14:21 ` Bruce Richardson 2023-12-20 15:08 ` Morten Brørup 2024-02-01 9:23 ` [PATCH v3 0/3] Improve optional lib support David Marchand 3 siblings, 1 reply; 20+ messages in thread From: Bruce Richardson @ 2023-12-20 14:21 UTC (permalink / raw) To: dev; +Cc: mb, Bruce Richardson In order to remove more libraries from the mandatory list, we need to have support for optionally having a dependency from a driver or library to another driver or lib. This patch adds this support by adding a new optional_deps variable, the contents of which are added to the deps list if those optional dependencies are present in the build. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/meson.build | 7 +++++++ lib/meson.build | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/drivers/meson.build b/drivers/meson.build index 5ba534049a..af2d8da5a8 100644 --- a/drivers/meson.build +++ b/drivers/meson.build @@ -127,6 +127,7 @@ foreach subpath:subdirs includes = [include_directories(drv_path)] # set up internal deps. Drivers can append/override as necessary deps = std_deps + optional_deps = [] # ext_deps: Stores external library dependency got # using dependency() (preferred) or find_library(). # For the find_library() case (but not with dependency()) we also @@ -168,6 +169,12 @@ foreach subpath:subdirs # get dependency objs from strings shared_deps = ext_deps static_deps = ext_deps + foreach d:optional_deps + #if optional dep exists, add it to the deps list + if is_variable('shared_rte_' + d) + deps += d + endif + endforeach foreach d:deps if not build break diff --git a/lib/meson.build b/lib/meson.build index 72e9138d14..733412c276 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -140,6 +140,7 @@ foreach l:libraries # external package/library requirements ext_deps = [] deps = [] + optional_deps = [] # eal is standard dependency once built if dpdk_conf.has('RTE_LIB_EAL') deps += ['eal'] @@ -177,6 +178,12 @@ foreach l:libraries shared_deps = ext_deps static_deps = ext_deps + foreach d:optional_deps + #if optional dep exists, add it to the deps list + if is_variable('shared_rte_' + d) + deps += d + endif + endforeach foreach d:deps if not build break -- 2.40.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* RE: [PATCH v3 3/3] build: RFC - add support for optional dependencies 2023-12-20 14:21 ` [PATCH v3 3/3] build: RFC - add support for optional dependencies Bruce Richardson @ 2023-12-20 15:08 ` Morten Brørup 2023-12-20 15:43 ` Bruce Richardson 0 siblings, 1 reply; 20+ messages in thread From: Morten Brørup @ 2023-12-20 15:08 UTC (permalink / raw) To: Bruce Richardson, dev > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Wednesday, 20 December 2023 15.22 > > In order to remove more libraries from the mandatory list, we need to > have support for optionally having a dependency from a driver or > library > to another driver or lib. This patch adds this support by adding a new > optional_deps variable, the contents of which are added to the deps > list > if those optional dependencies are present in the build. > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/meson.build | 7 +++++++ > lib/meson.build | 7 +++++++ > 2 files changed, 14 insertions(+) > > diff --git a/drivers/meson.build b/drivers/meson.build > index 5ba534049a..af2d8da5a8 100644 > --- a/drivers/meson.build > +++ b/drivers/meson.build > @@ -127,6 +127,7 @@ foreach subpath:subdirs > includes = [include_directories(drv_path)] > # set up internal deps. Drivers can append/override as > necessary > deps = std_deps > + optional_deps = [] (I'm a meson noob, so please bear with my stupid questions.) Is a separate "optional_deps" necessary? If a driver has any of these dependencies, why can it not just add them to the "deps" in the driver's meson.build file? Ohhh... It's the other way around: The driver only depends on the other (optional) lib if that other lib is enabled! Correct? Either way, please add an in-line comment describing optional_deps in the meson.build file. > # ext_deps: Stores external library dependency got > # using dependency() (preferred) or find_library(). > # For the find_library() case (but not with dependency()) we > also > @@ -168,6 +169,12 @@ foreach subpath:subdirs > # get dependency objs from strings > shared_deps = ext_deps > static_deps = ext_deps > + foreach d:optional_deps > + #if optional dep exists, add it to the deps list > + if is_variable('shared_rte_' + d) > + deps += d > + endif > + endforeach > foreach d:deps > if not build > break The same feedback also applies to the lib/meson.build changes. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] build: RFC - add support for optional dependencies 2023-12-20 15:08 ` Morten Brørup @ 2023-12-20 15:43 ` Bruce Richardson 2024-10-04 20:59 ` Stephen Hemminger 0 siblings, 1 reply; 20+ messages in thread From: Bruce Richardson @ 2023-12-20 15:43 UTC (permalink / raw) To: Morten Brørup; +Cc: dev On Wed, Dec 20, 2023 at 04:08:08PM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > Sent: Wednesday, 20 December 2023 15.22 > > > > In order to remove more libraries from the mandatory list, we need to > > have support for optionally having a dependency from a driver or > > library > > to another driver or lib. This patch adds this support by adding a new > > optional_deps variable, the contents of which are added to the deps > > list > > if those optional dependencies are present in the build. > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > --- > > drivers/meson.build | 7 +++++++ > > lib/meson.build | 7 +++++++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/drivers/meson.build b/drivers/meson.build > > index 5ba534049a..af2d8da5a8 100644 > > --- a/drivers/meson.build > > +++ b/drivers/meson.build > > @@ -127,6 +127,7 @@ foreach subpath:subdirs > > includes = [include_directories(drv_path)] > > # set up internal deps. Drivers can append/override as > > necessary > > deps = std_deps > > + optional_deps = [] > > (I'm a meson noob, so please bear with my stupid questions.) > > Is a separate "optional_deps" necessary? If a driver has any of these dependencies, why can it not just add them to the "deps" in the driver's meson.build file? > > Ohhh... It's the other way around: The driver only depends on the other (optional) lib if that other lib is enabled! Correct? > Yes, it's a shortcut to save an app having to manually check for its optional dependency itself in the meson.build file. However, this is only really useful if it's an optional dependency where we just have #ifdefs in the C code for it. For the initial example I was thinking of to try and use it - meter lib in ethdev - something that simple is not enough. So long as there are extra C files or headers that need to be built when a dependency is present, we need to change the meson.build file to explicitly check anyway. Hence it's only an RFC until such time as we find a use-case or two that uses it. /Bruce ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] build: RFC - add support for optional dependencies 2023-12-20 15:43 ` Bruce Richardson @ 2024-10-04 20:59 ` Stephen Hemminger 0 siblings, 0 replies; 20+ messages in thread From: Stephen Hemminger @ 2024-10-04 20:59 UTC (permalink / raw) To: Bruce Richardson; +Cc: Morten Brørup, dev On Wed, 20 Dec 2023 15:43:36 +0000 Bruce Richardson <bruce.richardson@intel.com> wrote: > On Wed, Dec 20, 2023 at 04:08:08PM +0100, Morten Brørup wrote: > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: Wednesday, 20 December 2023 15.22 > > > > > > In order to remove more libraries from the mandatory list, we need to > > > have support for optionally having a dependency from a driver or > > > library > > > to another driver or lib. This patch adds this support by adding a new > > > optional_deps variable, the contents of which are added to the deps > > > list > > > if those optional dependencies are present in the build. > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > --- > > > drivers/meson.build | 7 +++++++ > > > lib/meson.build | 7 +++++++ > > > 2 files changed, 14 insertions(+) > > > > > > diff --git a/drivers/meson.build b/drivers/meson.build > > > index 5ba534049a..af2d8da5a8 100644 > > > --- a/drivers/meson.build > > > +++ b/drivers/meson.build > > > @@ -127,6 +127,7 @@ foreach subpath:subdirs > > > includes = [include_directories(drv_path)] > > > # set up internal deps. Drivers can append/override as > > > necessary > > > deps = std_deps > > > + optional_deps = [] > > > > (I'm a meson noob, so please bear with my stupid questions.) > > > > Is a separate "optional_deps" necessary? If a driver has any of these dependencies, why can it not just add them to the "deps" in the driver's meson.build file? > > > > Ohhh... It's the other way around: The driver only depends on the other (optional) lib if that other lib is enabled! Correct? > > > Yes, it's a shortcut to save an app having to manually check for its > optional dependency itself in the meson.build file. > > However, this is only really useful if it's an optional dependency where we > just have #ifdefs in the C code for it. For the initial example I was > thinking of to try and use it - meter lib in ethdev - something that simple > is not enough. So long as there are extra C files or headers that need to > be built when a dependency is present, we need to change the meson.build > file to explicitly check anyway. Hence it's only an RFC until such time as > we find a use-case or two that uses it. Reasonable solution, but it hasn't been an issue for anything so far over a year. Marking it as "Awaiting upstream" since if a feature needs this, then use this. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 0/3] Improve optional lib support 2023-12-20 14:21 ` [PATCH v3 0/3] Improve optional lib support Bruce Richardson ` (2 preceding siblings ...) 2023-12-20 14:21 ` [PATCH v3 3/3] build: RFC - add support for optional dependencies Bruce Richardson @ 2024-02-01 9:23 ` David Marchand 2024-02-01 9:25 ` Bruce Richardson 3 siblings, 1 reply; 20+ messages in thread From: David Marchand @ 2024-02-01 9:23 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, mb On Wed, Dec 20, 2023 at 3:22 PM Bruce Richardson <bruce.richardson@intel.com> wrote: > > This patchset builds on the previous v1 and v2, and the discussion > which followed about what libraries should be optional. While still > a long way to go from the position suggested in the v2 discussion, > this set moves us a bit further along the road. > > The first patch is a direct v3 from previous versions, changing the > tracking from a list of optional libs to a list of mandatory ones. > The second patch then knocks off another 5 libs off the mandatory > list - all are libs which should have few dependencies in other > components, but which some common sample apps like l3fwd depend upon. > It should be possible to make other libs optional in future, but to > do so likely requires code-changes to #ifdef out support in places. > > To that end, patch 3 is an RFC to add support for "optional libs" to > the build system. For now, nothing uses this, but it may be useful if > someone does want to take on the task of removing a hard dependency > from one component on another and making it optional. > > Bruce Richardson (3): > build: track mandatory rather than optional libs > build: remove 5 libs from mandatory list > build: RFC - add support for optional dependencies I was not sure if I should take the third patch, but the two others make sense to me and I applied them. The third one can be reconsidered when the need arises. Thanks Bruce. -- David Marchand ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 0/3] Improve optional lib support 2024-02-01 9:23 ` [PATCH v3 0/3] Improve optional lib support David Marchand @ 2024-02-01 9:25 ` Bruce Richardson 0 siblings, 0 replies; 20+ messages in thread From: Bruce Richardson @ 2024-02-01 9:25 UTC (permalink / raw) To: David Marchand; +Cc: dev, mb On Thu, Feb 01, 2024 at 10:23:40AM +0100, David Marchand wrote: > On Wed, Dec 20, 2023 at 3:22 PM Bruce Richardson > <bruce.richardson@intel.com> wrote: > > > > This patchset builds on the previous v1 and v2, and the discussion > > which followed about what libraries should be optional. While still > > a long way to go from the position suggested in the v2 discussion, > > this set moves us a bit further along the road. > > > > The first patch is a direct v3 from previous versions, changing the > > tracking from a list of optional libs to a list of mandatory ones. > > The second patch then knocks off another 5 libs off the mandatory > > list - all are libs which should have few dependencies in other > > components, but which some common sample apps like l3fwd depend upon. > > It should be possible to make other libs optional in future, but to > > do so likely requires code-changes to #ifdef out support in places. > > > > To that end, patch 3 is an RFC to add support for "optional libs" to > > the build system. For now, nothing uses this, but it may be useful if > > someone does want to take on the task of removing a hard dependency > > from one component on another and making it optional. > > > > Bruce Richardson (3): > > build: track mandatory rather than optional libs > > build: remove 5 libs from mandatory list > > build: RFC - add support for optional dependencies > > I was not sure if I should take the third patch, but the two others > make sense to me and I applied them. > The third one can be reconsidered when the need arises. > Yep, agreed. Thanks. /Bruce ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2024-10-04 20:59 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-03 16:28 [PATCH 24.03] build: track mandatory rather than optional libs Bruce Richardson 2023-11-03 16:52 ` [PATCH 24.03 v2] " Bruce Richardson 2023-11-03 17:31 ` Morten Brørup 2023-11-03 18:08 ` Bruce Richardson 2023-11-03 20:19 ` Morten Brørup 2023-11-06 10:28 ` Bruce Richardson 2023-11-06 11:22 ` Morten Brørup 2023-11-06 11:27 ` Bruce Richardson 2023-11-06 11:37 ` Morten Brørup 2023-12-20 14:21 ` [PATCH v3 0/3] Improve optional lib support Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 1/3] build: track mandatory rather than optional libs Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 2/3] build: remove 5 libs from mandatory list Bruce Richardson 2023-12-20 15:18 ` Morten Brørup 2023-12-20 16:05 ` Bruce Richardson 2023-12-20 14:21 ` [PATCH v3 3/3] build: RFC - add support for optional dependencies Bruce Richardson 2023-12-20 15:08 ` Morten Brørup 2023-12-20 15:43 ` Bruce Richardson 2024-10-04 20:59 ` Stephen Hemminger 2024-02-01 9:23 ` [PATCH v3 0/3] Improve optional lib support David Marchand 2024-02-01 9:25 ` Bruce Richardson
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).