> From: Richardson, Bruce > Sent: Monday, September 4, 2017 2:43 PM > To: Van Haaren, Harry <harry.van.haa...@intel.com> > Cc: dev@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-boun...@dpdk.org] On Behalf Of Bruce Richardson > > > Sent: Friday, September 1, 2017 11:04 AM > > > To: dev@dpdk.org > > > Cc: Richardson, Bruce <bruce.richard...@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.haa...@intel.com>
