From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Bruce Richardson" <bruce.richardson@intel.com>
Cc: <dev@dpdk.org>
Subject: RE: [PATCH 24.03 v2] build: track mandatory rather than optional libs
Date: Mon, 6 Nov 2023 12:37:09 +0100 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9EFD8@smartserver.smartshare.dk> (raw)
In-Reply-To: <ZUjNntcmb0qn/NQe@bricha3-MOBL.ger.corp.intel.com>
> 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.
next prev parent reply other threads:[~2023-11-06 11:37 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-03 16:28 [PATCH 24.03] " 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 [this message]
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-02-01 9:23 ` [PATCH v3 0/3] Improve optional lib support David Marchand
2024-02-01 9:25 ` Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=98CBD80474FA8B44BF855DF32C47DC35E9EFD8@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).