[dpdk-dev] [PATCH 00/17] build DPDK libs and some drivers with meson/ninja

[Van Haaren, Harry]

> From: Richardson, Bruce
> Sent: Monday, September 4, 2017 2:43 PM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 00/17] build DPDK libs and some drivers with meson/ninja
> 
> On Mon, Sep 04, 2017 at 02:23:13PM +0100, Van Haaren, Harry wrote:
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson
> > > Sent: Friday, September 1, 2017 11:04 AM
> > > To: dev at dpdk.org
> > > Cc: Richardson, Bruce <bruce.richardson at intel.com>
> > > Subject: [dpdk-dev] [PATCH 00/17] build DPDK libs and some drivers with meson/ninja
> > >
> > > Following on from the two previous RFCs [1] [2], here is a cleaned up
> > > patchset to serve as a start-point for getting all of DPDK building with
> > > meson and ninja.
> > >
> > > What's covered:
> > > * Basic infrastructure for feature detection and DPDK compilation
> > > * Building of all DPDK libraries - as either static or shared
> > > * Compilation of igb_uio driver for Linux
> > > * Building a number of mempool, crypto and net drivers.
> > > * Installation of compiled libraries and headers
> > > * Installation of usertools scripts
> > > * Compilation of testpmd as dpdk-testpmd and install of same
> > > * Generation and installation of pkgconfig file for DPDK
> > > * Contributors guide document addition describing how to add build scripts
> > >
> > > What's not implemented:
> > > * Just about everything else :-), including
> > > * Support for non-x86 architectures, including cross-compilation
> > > * Lots of PMDs
> > > * Support for building and running the unit tests
> > >
> > > Some key differences from RFC2:
> > > * Removed duplication between the different driver meson files by moving
> > >   the build logic up one level to the driver/meson.build file.
> > > * Added a build option to allow versioning the libraries using the DPDK
> > >   version number, rather than individual .so versions.
> > > * Made EAL a default dependency for libs, to simplify meson.build files for
> > >   a number of them.
> > > * Made the build variables used for libraries and drivers more consistent.
> > > * Moved responsibility for determining if a given driver or library should
> > >   be built to the driver/library's own build file, giving a single place
> > >   where all details about that component are placed, and saving having lots
> > >   of environment detection logic in higher-level build files.
> > > * Begun adding in developer documentation to make it easier for driver
> > >   authors/maintainers to contribute.
> > >
> > > Meson 0.41 and ninja are needed, and ideally meson 0.42 is recommended.
> > > Ninja is available in most distributions, and meson - if an updated version
> > > is not available on your distro of choice - can be easily got using
> > > 	"pip3 install meson"
> > >
> > > To build and install then use:
> > >
> > > 	meson build # use default compiler and shared libs
> > > 	cd build
> > > 	ninja
> > > 	sudo ninja install
> > >
> > > Thereafter to use DPDK in other build systems one can use:
> > >
> > > 	pkg-config --cflags DPDK
> > > 	pkt-config --libs DPDK
> > >
> > > to query the needed DPDK build parameters.
> > >
> > > Once reviewed and tested a bit, I hope to apply this set - or a new
> > > revision of it - to the build-next tree, to serve as a baseline for others
> > > to use and to add the missing functionality to.
> >
> > <snip> git / file stats
> >
> > A good start - applied cleanly and compiled fine on dpdk-next-build tree.
> >
> > Some notes from experience of testing on an Ubuntu 16.04 system:
> > - libpcap wasn't detected successfully - on researching the transitional package "libpcap-
> dev" was installed, but that didn't actually install any of the required files. Installing
> "libpcap0.8-dev" enabled pcap to be detected successfully. No fault of Meson or these patches,
> just an inconsistency in transitional-packages it seems.
> 
> The great thing about all this is that it didn't break your build - it
> just didn't compile the pcap driver for you. :-)
> >
> > - Binaries after a compile are in a different location (compared to mk build system). eg:
> testpmd now resides in app/test-pmd/dpdk-testpmd. No issue, just a note that the path to the
> binaries changes. With the very-easy "ninja install" and "ninja uninstall", dpdk applications
> can just be run directly from the installed location (assuming binaries placed on PATH).
> >
> > - Ninja install is required with shared-object builds, to enable the dpdk binary (eg:
> testpmd) to find the .so objects. Doing a local compile (without install) and running
> ./app/test-pmd/dpdk-testpmd  will print "MBUF: error setting mempool handler" and rte_panic().
> This is obviously not an issue for static builds - the functionality is linked into the
> application in that case.
> 
> Yep, this is an issue, but I'm not sure of a good way round it just now.
> I made this set very much with the idea in mind that people would use
> "ninja install" a lot to expose their binaries when running DPDK.
> If this is not the case and we need a better solution, I'm open to
> ideas.
> 
> >
> > - Some compilers don't correctly expose their capabilities in warning flags causing Meson to
> believe that it should turn of these warnings. In the current Meson build code, these two
> warnings are nullified globally: -Wno-format-truncation and -Wno-address-of-packed-member. GCC
> 5.4.0 and 4.8.5 suffer from both incorrectly exposed. Gcc 7 does not have an issue with -Wno-
> format-truncation, but the other remains. Clang 3.8.0 does not have any issues. Just something
> to be aware of - no issues here either.
> >
> A little unclear what the problem is here? We record the flags that may
> be needed to compile up the code, and then apply those to the compilers
> that support them. It may be that some versions of a compiler don't need
> the flag when compiling the code, while others do, but this is hopefully
> rare enough that the simplicity gained from just using the flag when
> supported well out-weighs the downsides, of unnecessarily disabling a
> few warnings. The alternative of tracking different warnings for
> different compiler versions seems rather scary to me :-)
> 
> > - Build options are set using  mesonconf  tool, run it to see current configuration, or use
> it to eg enable static libraries:   mesonconf -Ddefault_library=static
> 
> Actually, to build static libs, you don't need mesonconf at all. You
> can use --default-library=static as a flag directly to meson.
> Similarly, you can pass the DPDK=specific flags using -Dx=y syntax to
> meson.
> 
> >
> > Summary so far;
> > - Only very minor issues found - resolved easily
> > - Configure and Build speeds still a breath of fresh air to me :)
> >
> > On to reviewing the patches themselves, -Harry
> 
> Great, thanks for the review.
> 
> /Bruce

Post-review notes;
- Series compiles DPDK perfectly here
- Code is in a readable and well layed out structure
- Some minor nitpick improvements identified, but I'd prefer send a patch to fix than have to request multiple V+1 respins.

A V2 addressing the few inline comments from this review would be great,
but then next steps is to commit the V2 patches to the dpdk-next-build tree, enabling easy testing and patching of the meson files.

Series-Reviewed-by: Harry van Haaren <harry.van.haaren at intel.com>