On Fri, 7 Jan 2022 17:13:04 +0100 Morten Brørup <m...@smartsharesystems.com> wrote:
> > From: David Marchand [mailto:david.march...@redhat.com] > > Sent: Wednesday, 17 November 2021 12.27 > > > > On Wed, Nov 17, 2021 at 11:47 AM Bruce Richardson > > <bruce.richard...@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.march...@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.
