From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id BB8777CF6 for ; Mon, 4 Sep 2017 17:49:04 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP; 04 Sep 2017 08:49:03 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,475,1498546800"; d="scan'208";a="307803139" Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28]) by fmsmga004.fm.intel.com with ESMTP; 04 Sep 2017 08:49:02 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by irsmsx105.ger.corp.intel.com ([169.254.7.75]) with mapi id 14.03.0319.002; Mon, 4 Sep 2017 16:49:02 +0100 From: "Van Haaren, Harry" To: "Richardson, Bruce" CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 00/17] build DPDK libs and some drivers with meson/ninja Thread-Index: AdMlgGT+YZYkltvOSfWsEQIPzWZbIP//9cAA///ORvA= Date: Mon, 4 Sep 2017 15:49:01 +0000 Message-ID: References: <20170904134230.GA22056@bricha3-MOBL3.ger.corp.intel.com> In-Reply-To: <20170904134230.GA22056@bricha3-MOBL3.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGI5NTFiM2EtOWIwNC00YzVkLWExYTYtYjQ0MjgzMmRmMTQyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkQwU3cwZ0JwS084a2NEeXdoS0p2S0UzVFZ5WHlrOWozNXV0dEFjbmNSS1E9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 00/17] build DPDK libs and some drivers with meson/ninja X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Sep 2017 15:49:05 -0000 > From: Richardson, Bruce > Sent: Monday, September 4, 2017 2:43 PM > To: Van Haaren, Harry > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 00/17] build DPDK libs and some drivers wi= th meson/ninja >=20 > On Mon, Sep 04, 2017 at 02:23:13PM +0100, Van Haaren, Harry wrote: > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > > > Sent: Friday, September 1, 2017 11:04 AM > > > To: dev@dpdk.org > > > Cc: Richardson, Bruce > > > Subject: [dpdk-dev] [PATCH 00/17] build DPDK libs and some drivers wi= th 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 w= ith > > > 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 sc= ripts > > > > > > 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 mov= ing > > > the build logic up one level to the driver/meson.build file. > > > * Added a build option to allow versioning the libraries using the DP= DK > > > version number, rather than individual .so versions. > > > * Made EAL a default dependency for libs, to simplify meson.build fil= es for > > > a number of them. > > > * Made the build variables used for libraries and drivers more consis= tent. > > > * Moved responsibility for determining if a given driver or library s= hould > > > be built to the driver/library's own build file, giving a single pl= ace > > > where all details about that component are placed, and saving havin= g lots > > > of environment detection logic in higher-level build files. > > > * Begun adding in developer documentation to make it easier for drive= r > > > authors/maintainers to contribute. > > > > > > Meson 0.41 and ninja are needed, and ideally meson 0.42 is recommende= d. > > > Ninja is available in most distributions, and meson - if an updated v= ersion > > > 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 o= thers > > > to use and to add the missing functionality to. > > > > git / file stats > > > > A good start - applied cleanly and compiled fine on dpdk-next-build tre= e. > > > > Some notes from experience of testing on an Ubuntu 16.04 system: > > - libpcap wasn't detected successfully - on researching the transitiona= l 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 Me= son or these patches, > just an inconsistency in transitional-packages it seems. >=20 > 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 t= hat 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 p= laced on PATH). > > > > - Ninja install is required with shared-object builds, to enable the dp= dk 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 hand= ler" and rte_panic(). > This is obviously not an issue for static builds - the functionality is l= inked into the > application in that case. >=20 > 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. >=20 > > > > - Some compilers don't correctly expose their capabilities in warning f= lags 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 i= ssues. 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 :-) >=20 > > - Build options are set using mesonconf tool, run it to see current c= onfiguration, or use > it to eg enable static libraries: mesonconf -Ddefault_library=3Dstatic >=20 > Actually, to build static libs, you don't need mesonconf at all. You > can use --default-library=3Dstatic as a flag directly to meson. > Similarly, you can pass the DPDK=3Dspecific flags using -Dx=3Dy syntax to > meson. >=20 > > > > 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 >=20 > Great, thanks for the review. >=20 > /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 t= o 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