Re: [PATCH 5/5] build: select optional libraries

[Stephen Hemminger <stephen@networkplumber.org>]

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.