From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by dpdk.org (Postfix) with ESMTP id AC1302B92 for ; Wed, 29 Aug 2018 11:34:22 +0200 (CEST) Received: by mail-wr1-f65.google.com with SMTP id m27-v6so4148227wrf.3 for ; Wed, 29 Aug 2018 02:34:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=xaTiHWo4yWvMIuZSwfPm52VuglUBD/STtzUeWakdi/g=; b=aD595VfSFtTyjraJfgZdVyaFB+dru4OWqbrn+p/W4l6dzdPj6EeQNyuMMyw+H7EhiN arbckwQSe718gTVXCTJBXw/OjTTf4Jqj+buyMOUKLZfJfdMMqHbZZ7WFC9hqGkP3wKnz iEtVZ931cpm6UQOoymLdNSC5AInb04qF2NhP92KKTW/VS9tJsbVCooVLLKft62qpO2Nj QAtl/CXFJ9hPCJwkQ9CJa1YAb7JrroX/SyNxJYaSi03DjOjPAkeWCrlsSqOoMYvPUe7/ 9Ark0XZE0V1einUpo5UV7zUiaA+3vTYOtYMTzIsEdxBEGhHy3EzWvzwcMWEKYVqS3tTh wGOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=xaTiHWo4yWvMIuZSwfPm52VuglUBD/STtzUeWakdi/g=; b=YSZKeOsjcWunUxSrqbkm9R7kq9f3n0tPis5vgVaXRROKXFyP6VFcsBeb2VTWvIxA9e 6R2VT6K78lO52a71mWSWisIy2gdYeEFDtw6s0w6UO0CDuVj4ZxJiOiR7Gat/tO61e0tN DiRSoEofaoRSqRl/T3+Q1WzNxHHxxq2JTdXWYMl5DbjKuFJ6+qT+GTsap0BOsyPgKxGE Fz5+hg+8hGtz+uD7/82qdvk3bBSqtpGCO88SHkaeGfp2bV+U84uLkf1P+0/22QAs05LN AMf2WRmS78SSqmCsdNkDKzngfdXV5jz3UMY/P+LHCDs2jKN+kgehtxaBc2lgcTYR0xw5 fXzg== X-Gm-Message-State: APzg51Dt8FwN5WOd5XCtaBPTG4XRW7IZ4cWKtTnFp1iFsVkYEIsdsIGp QELRhogevLKWaZ6vpwHuvIPT X-Google-Smtp-Source: ANB0VdaSZ3XQzh6MePzA6P92BCUDzGGzjb4iiitP9tR1bXLIczHJMXyYQfC8bnHqr8rYGkiwX1W3uQ== X-Received: by 2002:adf:f391:: with SMTP id m17-v6mr3785167wro.279.1535535262430; Wed, 29 Aug 2018 02:34:22 -0700 (PDT) Received: from laranjeiro-vm.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p89-v6sm6277762wrc.97.2018.08.29.02.34.21 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 29 Aug 2018 02:34:21 -0700 (PDT) Date: Wed, 29 Aug 2018 11:34:10 +0200 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro To: Bruce Richardson Cc: dev@dpdk.org, Yongseok Koh , Shahaf Shuler , Matan Azrad Message-ID: <20180829093410.3epizbi3pmx5ses2@laranjeiro-vm.dev.6wind.com> References: <7812af2267017898332783e934bef9478814ae96.1535361299.git.nelio.laranjeiro@6wind.com> <85a2b398fecdf917dc22810ea0974a91ccca1d13.1535373640.git.nelio.laranjeiro@6wind.com> <20180828154500.GA4208@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180828154500.GA4208@bricha3-MOBL.ger.corp.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2] net/mlx: add meson build support 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: Wed, 29 Aug 2018 09:34:22 -0000 Hi Bruce, Thanks for your comments I have address almost all of them in the v3 by doing what you suggest, I still have some comments, please see below, On Tue, Aug 28, 2018 at 04:45:00PM +0100, Bruce Richardson wrote: > Thanks for this, comments inline below. > > /Bruce > > On Mon, Aug 27, 2018 at 02:42:25PM +0200, Nelio Laranjeiro wrote: > > Mellanox drivers remains un-compiled by default due to third party > > libraries dependencies. They can be enabled through: > > - enable_driver_mlx{4,5}=true or > > - enable_driver_mlx{4,5}_glue=true > > depending on the needs. > > The big reason why we wanted a new build system was to move away from this > sort of static configuration. Instead, detect if the requirements as > present and build the driver if you can. Ok, I am letting only the glue option for both drivers as suggested at the end of your answer. > > To avoid modifying the whole sources and keep the compatibility with > > current build systems (e.g. make), the mlx{4,5}_autoconf.h is still > > generated by invoking DPDK scripts though meson's run_command() instead > > of using has_types, has_members, ... commands. > > > > Meson will try to find the required external libraries. When they are > > not installed system wide, they can be provided though CFLAGS, LDFLAGS > > and LD_LIBRARY_PATH environment variables, example (considering > > RDMA-Core is installed in /tmp/rdma-core): > > > > # CLFAGS=-I/tmp/rdma-core/build/include \ > > LDFLAGS=-L/tmp/rdma-core/build/lib \ > > LD_LIBRARY_PATH=/tmp/rdma-core/build/lib \ > > meson -Denable_driver_mlx4=true output > > > > # CLFAGS=-I/tmp/rdma-core/build/include \ > > LDFLAGS=-L/tmp/rdma-core/build/lib \ > > LD_LIBRARY_PATH=/tmp/rdma-core/build/lib \ > > ninja -C output install > > Once the CFLAGS/LDFLAGS are passed to meson, they should not be needed for > ninja. The LD_LIBRARY_PATH might be - I'm not sure about that one! :-) CFLAGS/LDFLAGS are correctly evaluated and inserted in the build.ninja file, for the LD_LIBRARY_PATH, it is necessary for the run_command stuff generating the mlx*_autoconf.h >[...] > Rather than having your own separate debug option flag, why not set these > based on the "buildtype" option e.g. if buildtype is set to "debug". > > > + # To maintain the compatibility with the make build system > > + # mlx4_autoconf.h file is still generated. > > + r = run_command('sh', '../../../buildtools/auto-config-h.sh', > > + 'mlx4_autoconf.h', > > + 'HAVE_IBV_MLX4_WQE_LSO_SEG', > > + 'infiniband/mlx4dv.h', > > + 'type', 'struct mlx4_wqe_lso_seg') > > + if r.returncode() != 0 > > + error('autoconfiguration fail') > > + endif > > Just to check that you are ok with this only being run at configure time? > If any changes are made to the inputs, ninja won't pick them up. To have it > tracked for input changes, "custom_target" should be used instead of > run_command. It seems to not be possible to have several custom_target on the same output file has this last is used as the target identifier in ninja. This limitation is acceptable for now, when meson will be the default build system, then such autoconf can be removed to use meson built-in functions. > > +endif > > +# Build Glue Library > > +if pmd_dlopen > > + dlopen_name = 'mlx4_glue' > > + dlopen_lib_name = driver_name_fmt.format(dlopen_name) > > + dlopen_so_version = LIB_GLUE_VERSION > > + dlopen_sources = files('mlx4_glue.c') > > + dlopen_install_dir = [ eal_pmd_path + '-glue' ] > > + shared_lib = shared_library( > > + dlopen_lib_name, > > + dlopen_sources, > > + include_directories: global_inc, > > + c_args: cflags, > > + link_args: [ > > + '-Wl,-export-dynamic', > > + '-Wl,-h,@0@'.format(LIB_GLUE), > > + '-lmlx4', > > + '-libverbs', > > While this works, the recommended approach is to save the return value from > cc.find_library() above, and pass that as a dependency directly, rather > than as a linker flag. I tried it, but: drivers/net/mlx5/meson.build:216:8: ERROR: Link_args arguments must be strings. find_library returns a compiler object, I did not found anyway to use directly the output of the find_library which works in places. Thanks, -- Nélio Laranjeiro 6WIND