[5/5] build: select optional libraries
Checks
Commit Message
There is currently no way to know which libraries are optional.
Introduce a enable_libs option (close to what we have for drivers) so
that packagers or projects consuming DPDK can more easily select the
optional libraries that matter to them and disable other optional
libraries.
Note: the enabled_libs variable is renamed for sake of consistency.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
buildtools/chkincs/meson.build | 2 +-
lib/meson.build | 34 ++++++++++++++++++----------------
meson.build | 3 ++-
meson_options.txt | 2 ++
4 files changed, 23 insertions(+), 18 deletions(-)
Comments
On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote:
> There is currently no way to know which libraries are optional.
> Introduce a enable_libs option (close to what we have for drivers) so
> that packagers or projects consuming DPDK can more easily select the
> optional libraries that matter to them and disable other optional
> libraries.
>
> Note: the enabled_libs variable is renamed for sake of consistency.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
This is the only patch of this set I would have some concerns about. I'm
just not sure that it makes sense to have this option for libraries
compared to drivers.
Specifically:
* We have over 200 drivers in DPDK (rough count using find), of which 2 are
mandatory, and therefore specifying just 1 or 2 that you want can make
sense.
* On the other hand, we have 53 libraries, of which only 7 or so (after
this patchset) are optional. This means that use of the term
"enable_libs" is misleading - at least to me - in that it's only a very
small proportion of the libs which would be affected by that flag
(compared to 99% of the drivers)
* Also, while the number of mandatory drivers is unlikely to change much
(since there are only 2), it should be fairly safe to do builds using
"--enable-drivers". On the other hand, the list of libraries affected by
"--enable-libs" is likely to change, so short of each user naming each
and every lib they use (and each library those depend on), to the list,
it's quite possible that any --enable-libs use could lead to a broken
build in future if a library changes from mandatory to optional.
Overall, I'm just concerned that this flag is premature, and would prefer
to keep it just to the disable option until we are confident that out
"optional library" list is relatively settled.
/Bruce
10/11/2021 18:34, Bruce Richardson:
> On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote:
> > There is currently no way to know which libraries are optional.
> > Introduce a enable_libs option (close to what we have for drivers) so
> > that packagers or projects consuming DPDK can more easily select the
> > optional libraries that matter to them and disable other optional
> > libraries.
> >
> > Note: the enabled_libs variable is renamed for sake of consistency.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> This is the only patch of this set I would have some concerns about. I'm
> just not sure that it makes sense to have this option for libraries
> compared to drivers.
>
> Specifically:
> * We have over 200 drivers in DPDK (rough count using find), of which 2 are
> mandatory, and therefore specifying just 1 or 2 that you want can make
> sense.
> * On the other hand, we have 53 libraries, of which only 7 or so (after
> this patchset) are optional. This means that use of the term
> "enable_libs" is misleading - at least to me - in that it's only a very
> small proportion of the libs which would be affected by that flag
> (compared to 99% of the drivers)
The options are described like this:
option('disable_libs', type: 'string', value: '', description:
'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
+option('enable_libs', type: 'string', value: '', description:
+ 'Comma-separated list of libraries to explicitly enable.')
I feel we should mention it is enabling optional libraries,
and the default is to enable all.
> * Also, while the number of mandatory drivers is unlikely to change much
> (since there are only 2), it should be fairly safe to do builds using
> "--enable-drivers". On the other hand, the list of libraries affected by
> "--enable-libs" is likely to change, so short of each user naming each
> and every lib they use (and each library those depend on), to the list,
> it's quite possible that any --enable-libs use could lead to a broken
> build in future if a library changes from mandatory to optional.
In order to be safe, the user can list all required libs,
including non-optional ones.
> Overall, I'm just concerned that this flag is premature, and would prefer
> to keep it just to the disable option until we are confident that out
> "optional library" list is relatively settled.
I see it the opposite way:
Someone who does not wish to deliver extra libs could use this option
to list the required libs, so not-required libs will disappear from
the build once they are declared optional in future releases.
On Tue, Nov 16, 2021 at 06:25:28PM +0100, Thomas Monjalon wrote:
> 10/11/2021 18:34, Bruce Richardson:
> > On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote:
> > > There is currently no way to know which libraries are optional.
> > > Introduce a enable_libs option (close to what we have for drivers) so
> > > that packagers or projects consuming DPDK can more easily select the
> > > optional libraries that matter to them and disable other optional
> > > libraries.
> > >
> > > Note: the enabled_libs variable is renamed for sake of consistency.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > This is the only patch of this set I would have some concerns about. I'm
> > just not sure that it makes sense to have this option for libraries
> > compared to drivers.
> >
> > Specifically:
> > * We have over 200 drivers in DPDK (rough count using find), of which 2 are
> > mandatory, and therefore specifying just 1 or 2 that you want can make
> > sense.
> > * On the other hand, we have 53 libraries, of which only 7 or so (after
> > this patchset) are optional. This means that use of the term
> > "enable_libs" is misleading - at least to me - in that it's only a very
> > small proportion of the libs which would be affected by that flag
> > (compared to 99% of the drivers)
>
> The options are described like this:
>
> option('disable_libs', type: 'string', value: '', description:
> 'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
> +option('enable_libs', type: 'string', value: '', description:
> + 'Comma-separated list of libraries to explicitly enable.')
>
> I feel we should mention it is enabling optional libraries,
> and the default is to enable all.
>
> > * Also, while the number of mandatory drivers is unlikely to change much
> > (since there are only 2), it should be fairly safe to do builds using
> > "--enable-drivers". On the other hand, the list of libraries affected by
> > "--enable-libs" is likely to change, so short of each user naming each
> > and every lib they use (and each library those depend on), to the list,
> > it's quite possible that any --enable-libs use could lead to a broken
> > build in future if a library changes from mandatory to optional.
>
> In order to be safe, the user can list all required libs,
> including non-optional ones.
>
Yes, they can, but that is the part I'm concerned about. It should not be
expected for users to know what all libraries should be needed and
dependencies between them. The list of mandatory libraries is quite long.
> > Overall, I'm just concerned that this flag is premature, and would prefer
> > to keep it just to the disable option until we are confident that out
> > "optional library" list is relatively settled.
>
> I see it the opposite way:
> Someone who does not wish to deliver extra libs could use this option
> to list the required libs, so not-required libs will disappear from
> the build once they are declared optional in future releases.
>
Yes, though as-above, I'm concerned about the difficulty of building up
such a list. However, if this option is only for "expert" and
distro-packaging use, then I suppose it's reasonable. Perhaps we should add
a warning to the doc line too, noting that it's only recommended for
advanced users.
/Bruce
On Wed, Nov 17, 2021 at 11:47 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Nov 16, 2021 at 06:25:28PM +0100, Thomas Monjalon wrote:
> > 10/11/2021 18:34, Bruce Richardson:
> > > On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote:
> > > > There is currently no way to know which libraries are optional.
> > > > Introduce a enable_libs option (close to what we have for drivers) so
> > > > that packagers or projects consuming DPDK can more easily select the
> > > > optional libraries that matter to them and disable other optional
> > > > libraries.
> > > >
> > > > Note: the enabled_libs variable is renamed for sake of consistency.
> > > >
> > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > ---
> > > This is the only patch of this set I would have some concerns about. I'm
> > > just not sure that it makes sense to have this option for libraries
> > > compared to drivers.
> > >
> > > Specifically:
> > > * We have over 200 drivers in DPDK (rough count using find), of which 2 are
> > > mandatory, and therefore specifying just 1 or 2 that you want can make
> > > sense.
> > > * On the other hand, we have 53 libraries, of which only 7 or so (after
> > > this patchset) are optional. This means that use of the term
> > > "enable_libs" is misleading - at least to me - in that it's only a very
> > > small proportion of the libs which would be affected by that flag
> > > (compared to 99% of the drivers)
> >
> > The options are described like this:
> >
> > option('disable_libs', type: 'string', value: '', description:
> > 'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
> > +option('enable_libs', type: 'string', value: '', description:
> > + 'Comma-separated list of libraries to explicitly enable.')
> >
> > I feel we should mention it is enabling optional libraries,
> > and the default is to enable all.
> >
> > > * Also, while the number of mandatory drivers is unlikely to change much
> > > (since there are only 2), it should be fairly safe to do builds using
> > > "--enable-drivers". On the other hand, the list of libraries affected by
> > > "--enable-libs" is likely to change, so short of each user naming each
> > > and every lib they use (and each library those depend on), to the list,
> > > it's quite possible that any --enable-libs use could lead to a broken
> > > build in future if a library changes from mandatory to optional.
> >
> > In order to be safe, the user can list all required libs,
> > including non-optional ones.
> >
>
> Yes, they can, but that is the part I'm concerned about. It should not be
> expected for users to know what all libraries should be needed and
> dependencies between them. The list of mandatory libraries is quite long.
>
> > > Overall, I'm just concerned that this flag is premature, and would prefer
> > > to keep it just to the disable option until we are confident that out
> > > "optional library" list is relatively settled.
> >
> > I see it the opposite way:
> > Someone who does not wish to deliver extra libs could use this option
> > to list the required libs, so not-required libs will disappear from
> > the build once they are declared optional in future releases.
> >
>
> Yes, though as-above, I'm concerned about the difficulty of building up
> such a list. However, if this option is only for "expert" and
> distro-packaging use, then I suppose it's reasonable. Perhaps we should add
> a warning to the doc line too, noting that it's only recommended for
> advanced users.
>
Ok, we still need some discussion.
I'll just respin for Thomas to merge patches that are ready.
> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, 17 November 2021 12.27
>
> On Wed, Nov 17, 2021 at 11:47 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Tue, Nov 16, 2021 at 06:25:28PM +0100, Thomas Monjalon wrote:
> > > 10/11/2021 18:34, Bruce Richardson:
> > > > On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote:
> > > > > There is currently no way to know which libraries are optional.
> > > > > Introduce a enable_libs option (close to what we have for
> drivers) so
> > > > > that packagers or projects consuming DPDK can more easily
> select the
> > > > > optional libraries that matter to them and disable other
> optional
> > > > > libraries.
> > > > >
> > > > > Note: the enabled_libs variable is renamed for sake of
> consistency.
> > > > >
> > > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > > ---
> > > > This is the only patch of this set I would have some concerns
> about. I'm
> > > > just not sure that it makes sense to have this option for
> libraries
> > > > compared to drivers.
> > > >
> > > > Specifically:
> > > > * We have over 200 drivers in DPDK (rough count using find), of
> which 2 are
> > > > mandatory, and therefore specifying just 1 or 2 that you want
> can make
> > > > sense.
> > > > * On the other hand, we have 53 libraries, of which only 7 or so
> (after
> > > > this patchset) are optional. This means that use of the term
> > > > "enable_libs" is misleading - at least to me - in that it's
> only a very
> > > > small proportion of the libs which would be affected by that
> flag
> > > > (compared to 99% of the drivers)
> > >
> > > The options are described like this:
> > >
> > > option('disable_libs', type: 'string', value: '', description:
> > > 'Comma-separated list of libraries to explicitly disable.
> [NOTE: not all libs can be disabled]')
> > > +option('enable_libs', type: 'string', value: '', description:
> > > + 'Comma-separated list of libraries to explicitly enable.')
> > >
> > > I feel we should mention it is enabling optional libraries,
> > > and the default is to enable all.
> > >
> > > > * Also, while the number of mandatory drivers is unlikely to
> change much
> > > > (since there are only 2), it should be fairly safe to do builds
> using
> > > > "--enable-drivers". On the other hand, the list of libraries
> affected by
> > > > "--enable-libs" is likely to change, so short of each user
> naming each
> > > > and every lib they use (and each library those depend on), to
> the list,
> > > > it's quite possible that any --enable-libs use could lead to a
> broken
> > > > build in future if a library changes from mandatory to
> optional.
> > >
> > > In order to be safe, the user can list all required libs,
> > > including non-optional ones.
> > >
> >
> > Yes, they can, but that is the part I'm concerned about. It should
> not be
> > expected for users to know what all libraries should be needed and
> > dependencies between them. The list of mandatory libraries is quite
> long.
> >
> > > > Overall, I'm just concerned that this flag is premature, and
> would prefer
> > > > to keep it just to the disable option until we are confident that
> out
> > > > "optional library" list is relatively settled.
> > >
> > > I see it the opposite way:
> > > Someone who does not wish to deliver extra libs could use this
> option
> > > to list the required libs, so not-required libs will disappear from
> > > the build once they are declared optional in future releases.
> > >
> >
> > Yes, though as-above, I'm concerned about the difficulty of building
> up
> > such a list. However, if this option is only for "expert" and
> > distro-packaging use, then I suppose it's reasonable. Perhaps we
> should add
> > a warning to the doc line too, noting that it's only recommended for
> > advanced users.
> >
>
> Ok, we still need some discussion.
> I'll just respin for Thomas to merge patches that are ready.
This patch is a step in the right direction. Thank you.
Adding my opinion to the discussion will be a long rant from a grumpy old minimalist (working on dedicated network appliances, not Linux distros), so here goes:
The goal should be making all non-core libraries optional. And off the top of my head, these are the only core libraries:
EAL, mbuf, mempool, ring.
And yes, that also means that not even the simplest example applications can compile with only the core libraries. However, the ability to build a minimal dpdk.so should not depend on what is required to be able to build any example applications. In other words: If an example application cannot be built due to an omitted library, then do not build that application.
E.g. an Ethernet bridge application does not need any IP or routing libraries, so they are not core libraries.
I consider the mbuf library and the libraries it uses (i.e. mempool and ring) core libraries, because the core purpose of DPDK is to provide a data plane library, and all DPDK data plane application use mbufs. :-)
BTW, I also consider the ethdev library extremely bloated, with Flow, Metering, Traffic Management and ever more being added into one big monolithic library instead of separate libraries or modules.
On Fri, 7 Jan 2022 17:13:04 +0100
Morten Brørup <mb@smartsharesystems.com> wrote:
> > From: David Marchand [mailto:david.marchand@redhat.com]
> > Sent: Wednesday, 17 November 2021 12.27
> >
> > On Wed, Nov 17, 2021 at 11:47 AM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Tue, Nov 16, 2021 at 06:25:28PM +0100, Thomas Monjalon wrote:
> > > > 10/11/2021 18:34, Bruce Richardson:
> > > > > On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote:
> > > > > > There is currently no way to know which libraries are optional.
> > > > > > Introduce a enable_libs option (close to what we have for
> > drivers) so
> > > > > > that packagers or projects consuming DPDK can more easily
> > select the
> > > > > > optional libraries that matter to them and disable other
> > optional
> > > > > > libraries.
> > > > > >
> > > > > > Note: the enabled_libs variable is renamed for sake of
> > consistency.
> > > > > >
> > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > > > ---
> > > > > This is the only patch of this set I would have some concerns
> > about. I'm
> > > > > just not sure that it makes sense to have this option for
> > libraries
> > > > > compared to drivers.
> > > > >
> > > > > Specifically:
> > > > > * We have over 200 drivers in DPDK (rough count using find), of
> > which 2 are
> > > > > mandatory, and therefore specifying just 1 or 2 that you want
> > can make
> > > > > sense.
> > > > > * On the other hand, we have 53 libraries, of which only 7 or so
> > (after
> > > > > this patchset) are optional. This means that use of the term
> > > > > "enable_libs" is misleading - at least to me - in that it's
> > only a very
> > > > > small proportion of the libs which would be affected by that
> > flag
> > > > > (compared to 99% of the drivers)
> > > >
> > > > The options are described like this:
> > > >
> > > > option('disable_libs', type: 'string', value: '', description:
> > > > 'Comma-separated list of libraries to explicitly disable.
> > [NOTE: not all libs can be disabled]')
> > > > +option('enable_libs', type: 'string', value: '', description:
> > > > + 'Comma-separated list of libraries to explicitly enable.')
> > > >
> > > > I feel we should mention it is enabling optional libraries,
> > > > and the default is to enable all.
> > > >
> > > > > * Also, while the number of mandatory drivers is unlikely to
> > change much
> > > > > (since there are only 2), it should be fairly safe to do builds
> > using
> > > > > "--enable-drivers". On the other hand, the list of libraries
> > affected by
> > > > > "--enable-libs" is likely to change, so short of each user
> > naming each
> > > > > and every lib they use (and each library those depend on), to
> > the list,
> > > > > it's quite possible that any --enable-libs use could lead to a
> > broken
> > > > > build in future if a library changes from mandatory to
> > optional.
> > > >
> > > > In order to be safe, the user can list all required libs,
> > > > including non-optional ones.
> > > >
> > >
> > > Yes, they can, but that is the part I'm concerned about. It should
> > not be
> > > expected for users to know what all libraries should be needed and
> > > dependencies between them. The list of mandatory libraries is quite
> > long.
> > >
> > > > > Overall, I'm just concerned that this flag is premature, and
> > would prefer
> > > > > to keep it just to the disable option until we are confident that
> > out
> > > > > "optional library" list is relatively settled.
> > > >
> > > > I see it the opposite way:
> > > > Someone who does not wish to deliver extra libs could use this
> > option
> > > > to list the required libs, so not-required libs will disappear from
> > > > the build once they are declared optional in future releases.
> > > >
> > >
> > > Yes, though as-above, I'm concerned about the difficulty of building
> > up
> > > such a list. However, if this option is only for "expert" and
> > > distro-packaging use, then I suppose it's reasonable. Perhaps we
> > should add
> > > a warning to the doc line too, noting that it's only recommended for
> > > advanced users.
> > >
> >
> > Ok, we still need some discussion.
> > I'll just respin for Thomas to merge patches that are ready.
>
> This patch is a step in the right direction. Thank you.
>
> Adding my opinion to the discussion will be a long rant from a grumpy old minimalist (working on dedicated network appliances, not Linux distros), so here goes:
>
> The goal should be making all non-core libraries optional. And off the top of my head, these are the only core libraries:
> EAL, mbuf, mempool, ring.
>
> And yes, that also means that not even the simplest example applications can compile with only the core libraries. However, the ability to build a minimal dpdk.so should not depend on what is required to be able to build any example applications. In other words: If an example application cannot be built due to an omitted library, then do not build that application.
>
> E.g. an Ethernet bridge application does not need any IP or routing libraries, so they are not core libraries.
>
> I consider the mbuf library and the libraries it uses (i.e. mempool and ring) core libraries, because the core purpose of DPDK is to provide a data plane library, and all DPDK data plane application use mbufs. :-)
>
> BTW, I also consider the ethdev library extremely bloated, with Flow, Metering, Traffic Management and ever more being added into one big monolithic library instead of separate libraries or modules.
>
Agreed, DPDK has become fat and now requires lots of things like meter and telemetry
to build.
@@ -18,7 +18,7 @@ sources = files('main.c')
sources += gen_c_files.process(dpdk_chkinc_headers)
deps = []
-foreach l:enabled_libs
+foreach l:dpdk_libs_enabled
deps += get_variable('static_rte_' + l)
endforeach
@@ -78,17 +78,10 @@ optional_libs = [
'vhost',
]
-disabled_libs = []
-opt_disabled_libs = run_command(list_dir_globs, get_option('disable_libs'),
+enable_libs = run_command(list_dir_globs, get_option('enable_libs'),
+ check: true).stdout().split()
+disable_libs = run_command(list_dir_globs, get_option('disable_libs'),
check: true).stdout().split()
-foreach l:opt_disabled_libs
- if not optional_libs.contains(l)
- warning('Cannot disable mandatory library "@0@"'.format(l))
- continue
- endif
- disabled_libs += l
-endforeach
-
default_cflags = machine_args
default_cflags += ['-DALLOW_EXPERIMENTAL_API']
@@ -98,8 +91,6 @@ if cc.has_argument('-Wno-format-truncation')
default_cflags += '-Wno-format-truncation'
endif
-enabled_libs = [] # used to print summary at the end
-
foreach l:libraries
build = true
reason = '<unknown reason>' # set if build == false to explain why
@@ -123,10 +114,21 @@ foreach l:libraries
deps += ['eal']
endif
- if disabled_libs.contains(l)
- build = false
- reason = 'explicitly disabled via build config'
+ if optional_libs.contains(l)
+ # check for enabled libraries only if one is set in config
+ if enable_libs.length() != 0 and not enable_libs.contains(l)
+ build = false
+ reason = 'not in enabled libraries build config'
+ elif disable_libs.contains(l)
+ build = false
+ reason = 'explicitly disabled via build config'
+ endif
else
+ if disable_libs.contains(l)
+ warning('Cannot disable mandatory library "@0@"'.format(l))
+ endif
+ endif
+ if build
subdir(l)
endif
if name != l
@@ -150,7 +152,7 @@ foreach l:libraries
static_deps += [get_variable('static_rte_' + d)]
endforeach
- enabled_libs += name
+ dpdk_libs_enabled += name
dpdk_conf.set('RTE_LIB_' + name.to_upper(), 1)
install_headers(headers)
install_headers(indirect_headers)
@@ -35,6 +35,7 @@ dpdk_driver_classes = []
dpdk_drivers = []
dpdk_extra_ldflags = []
dpdk_libs_disabled = []
+dpdk_libs_enabled = []
dpdk_drvs_disabled = []
abi_version_file = files('ABI_VERSION')
@@ -102,7 +103,7 @@ subdir('buildtools/pkg-config')
output_message = '\n=================\nLibraries Enabled\n=================\n'
output_message += '\nlibs:\n\t'
output_count = 0
-foreach lib:enabled_libs
+foreach lib:dpdk_libs_enabled
output_message += lib + ', '
output_count += 1
if output_count == 8
@@ -16,6 +16,8 @@ option('enable_docs', type: 'boolean', value: false, description:
'build documentation')
option('enable_drivers', type: 'string', value: '', description:
'Comma-separated list of drivers to build. If unspecified, build all drivers.')
+option('enable_libs', type: 'string', value: '', description:
+ 'Comma-separated list of libraries to explicitly enable.')
option('enable_driver_sdk', type: 'boolean', value: false, description:
'Install headers to build drivers.')
option('enable_kmods', type: 'boolean', value: false, description: