[PATCH 24.03 v2] build: track mandatory rather than optional libs

[Bruce Richardson]

On Mon, Nov 06, 2023 at 12:22:57PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson at 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 at 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 at 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 at 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