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

[Luca Boccassi <bluca@debian.org>]

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!

-- 
Kind regards,
Luca Boccassi