Re: [dpdk-dev] [PATCH v2 2/5] net/mlx4: use mlx4 debug flag instead of NDEBUG

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Jan 24, 2020 at 04:50:51PM +0000, Slava Ovsiienko wrote:
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Friday, January 24, 2020 18:43
> > To: Alexander Kozyrev <akozyrev@mellanox.com>; dev@dpdk.org
> > Cc: Raslan Darawsheh <rasland@mellanox.com>; Matan Azrad
> > <matan@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> > Thomas Monjalon <thomas@monjalon.net>; Bruce Richardson
> > <bruce.richardson@intel.com>
> > Subject: Re: [PATCH v2 2/5] net/mlx4: use mlx4 debug flag instead of NDEBUG
> > 
> > On 1/23/2020 6:20 PM, Alexander Kozyrev wrote:
> > > Define a new MLX4_DEBUG compilation flag to get rid of dependency on
> > > the NDEBUG definition. This is a preparation step to switch from
> > > standard assert clauses to DPDK RTE_ASSERT ones in MLX4 driver.
> > >
> > > Signed-off-by: Alexander Kozyrev <akozyrev@mellanox.com>
> > > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > > ---
> > >  drivers/net/mlx4/Makefile     |  4 ++--
> > >  drivers/net/mlx4/meson.build  |  4 ++--
> > >  drivers/net/mlx4/mlx4.c       |  4 ++--
> > >  drivers/net/mlx4/mlx4_mr.c    |  8 ++++----
> > >  drivers/net/mlx4/mlx4_rxtx.c  | 10 +++++-----
> > > drivers/net/mlx4/mlx4_utils.h |  8 ++++----
> > >  6 files changed, 19 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx4/Makefile b/drivers/net/mlx4/Makefile
> > > index 329569d..043e72f 100644
> > > --- a/drivers/net/mlx4/Makefile
> > > +++ b/drivers/net/mlx4/Makefile
> > > @@ -65,13 +65,13 @@ endif
> > >
> > >  # User-defined CFLAGS.
> > >  ifeq ($(CONFIG_RTE_LIBRTE_MLX4_DEBUG),y)
> > > -CFLAGS += -pedantic -UNDEBUG
> > > +CFLAGS += -pedantic -DMLX4_DEBUG
> > 
> > Can't use 'RTE_LIBRTE_MLX4_DEBUG' directly in the .c files, instead of interim
> > 'MLX4_DEBUG', many other config options used that way.
> > 
> > >  ifneq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)  CFLAGS += -DPEDANTIC  endif
> > > AUTO_CONFIG_CFLAGS += -Wno-pedantic  else -CFLAGS += -DNDEBUG
> > > -UPEDANTIC
> > > +CFLAGS += -UMLX4_DEBUG -UPEDANTIC
> > >  endif
> > >
> > >  include $(RTE_SDK)/mk/rte.lib.mk
> > > diff --git a/drivers/net/mlx4/meson.build
> > > b/drivers/net/mlx4/meson.build index 9eb4988..a15a301 100644
> > > --- a/drivers/net/mlx4/meson.build
> > > +++ b/drivers/net/mlx4/meson.build
> > > @@ -67,9 +67,9 @@ if build
> > >  		endif
> > >  	endforeach
> > >  	if get_option('buildtype').contains('debug')
> > > -		cflags += [ '-pedantic', '-UNDEBUG', '-DPEDANTIC' ]
> > > +		cflags += [ '-pedantic', '-DMLX4_DEBUG', '-DPEDANTIC' ]
> > >  	else
> > > -		cflags += [ '-DNDEBUG', '-UPEDANTIC' ]
> > > +		cflags += [ '-UMLX4_DEBUG', '-UPEDANTIC' ]
> > 
> > Right now there is no way in meson to enable/disable compile time
> > (datapath) debug options in module granularity, it would be good to have
> > them.
> +1, I think we should not drop the module debug options and
> it would be good to support ones with meson.
> 
Discussing with Ferruh offline, one option might be to add a generic
--drivers-debug flag [taking the same parameters as the current
disable-drivers option], and defines a debug flag (or flags) for the
specified driver. To keep things simple for drivers, we could just blindly
define for each requested driver the macros similar to that used in make
such as ..._DEBUG_RX. _DEBUG_TX.

Regards,
/Bruce