Re: [dpdk-dev] [PATCH v2] net/mlx: add meson build support

[Luca Boccassi <bluca@debian.org>]

On Wed, 2018-08-29 at 13:59 +0200, Nélio Laranjeiro wrote:
> On Wed, Aug 29, 2018 at 11:00:54AM +0100, Luca Boccassi wrote:
> > On Tue, 2018-08-28 at 16:45 +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.
> > > 
> > > > 
> > > > 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! :-
> > > )
> > > 
> > > > 
> > > > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > 
> > > > ---
> > > > 
> > > > Changes in v2:
> > > > 
> > > > - dropped patch https://patches.dpdk.org/patch/43897/
> > > > - remove extra_{cflags,ldflags} as already honored by meson
> > > > through
> > > > environment variables.
> > > > ---
> > > >  drivers/net/meson.build      |   2 +
> > > >  drivers/net/mlx4/meson.build |  94 ++++++
> > > >  drivers/net/mlx5/meson.build | 545
> > > > +++++++++++++++++++++++++++++++++++
> > > >  meson_options.txt            |   8 +
> > > >  4 files changed, 649 insertions(+)
> > > >  create mode 100644 drivers/net/mlx4/meson.build
> > > >  create mode 100644 drivers/net/mlx5/meson.build
> > > > 
> > > > diff --git a/drivers/net/meson.build b/drivers/net/meson.build
> > > > index 9c28ed4da..c7a2d0e7d 100644
> > > > --- a/drivers/net/meson.build
> > > > +++ b/drivers/net/meson.build
> > > > @@ -18,6 +18,8 @@ drivers = ['af_packet',
> > > >  	'ixgbe',
> > > >  	'kni',
> > > >  	'liquidio',
> > > > +	'mlx4',
> > > > +	'mlx5',
> > > >  	'mvpp2',
> > > >  	'netvsc',
> > > >  	'nfp',
> > > > diff --git a/drivers/net/mlx4/meson.build
> > > > b/drivers/net/mlx4/meson.build
> > > > new file mode 100644
> > > > index 000000000..debaca5b6
> > > > --- /dev/null
> > > > +++ b/drivers/net/mlx4/meson.build
> > > > @@ -0,0 +1,94 @@
> > > > +# SPDX-License-Identifier: BSD-3-Clause
> > > > +# Copyright 2018 6WIND S.A.
> > > > +# Copyright 2018 Mellanox Technologies, Ltd
> > > > +
> > > > +# As there is no more configuration file to activate/configure
> > > > the
> > > > PMD it will
> > > > +# use some variables here to configure it.
> > > > +pmd_dlopen = get_option('enable_driver_mlx4_glue')
> > > > +build = get_option('enable_driver_mlx4') or pmd_dlopen
> > > 
> > > As stated above, I believe this should be based upon whether you
> > > find
> > > the
> > > "mnl", "mlx4" and "ibverbs" libraries. If we start adding back in
> > > static
> > > options for every driver, then we'll be back to having a mass of
> > > config
> > > options like we had before.
> > 
> > BTW, slightly related to that: ibverbs doesn't ship pkg-config
> > files at
> > the moment which makes the detection slightly more awkward that it
> > could be, so I've sent a PR upstream to add that:
> > 
> > https://github.com/linux-rdma/rdma-core/pull/373
> > 
> > Hope this can be useful!
> 
> Thanks Luca, I was also searching for it, you save me some time, I
> hope
> this can be backported down to RDMA-Core's stable version v15 of
> RDMA-Core it would fully help.

This was merged now, so if you want it backported you can ask the
maintainers if it would be fine to do so.

-- 
Kind regards,
Luca Boccassi