> -----Original Message----- > From: Thomas Monjalon <tho...@monjalon.net> > Sent: Friday, January 10, 2020 11:07 > To: Slava Ovsiienko <viachesl...@mellanox.com> > Cc: Ferruh Yigit <ferruh.yi...@intel.com>; dev@dpdk.org; Matan Azrad > <ma...@mellanox.com>; Raslan Darawsheh <rasl...@mellanox.com>; Ori > Kam <or...@mellanox.com> > Subject: Re: [dpdk-dev] [PATCH v2 4/4] net/mlx5: engage free on completion > queue > > 09/01/2020 17:22, Slava Ovsiienko: > > From: Ferruh Yigit <ferruh.yi...@intel.com> > > > On 1/9/2020 3:27 PM, Slava Ovsiienko wrote: > > > > From: Ferruh Yigit <ferruh.yi...@intel.com> > > > >> On 1/9/2020 10:56 AM, Viacheslav Ovsiienko wrote: > > > >>> + assert(ci != txq->cq_pi); > > > >>> + assert((txq->fcqs[ci & txq->cqe_m] >> 16) == cqe- > > > >>> wqe_counter); > > > >> > > > >> And same comments on these as previous patches, we spend some > > > >> effort to remove the 'rte_panic' from drivers, this is almost same > > > >> thing. > > > >> > > > >> I think a driver shouldn't decide to exit whole application, it's > > > >> effect should be limited to the driver. > > > >> > > > >> Assert is useful for debug and during development, but not sure > > > >> having them in the production code. > > > > > > > > IIRC, "assert" is standard C function. Compiled only if there is > > > > no NDEBUG > > > defined. > > > > So, assert does exactly what you are saying - provide the debug > > > > break not allowing the bug to evolve. And no this break in production > code. > > > > > > > > > > Since mlx driver is using NDEBUG defined, what you said is right > > > indeed. But why not using RTE_ASSERT to be consistent with rest. > > > There is a specific config option to control assert > > > (RTE_ENABLE_ASSERT) and anyone using it will get different behavior with > mlx5. > > > > We have the dedicated option to control mlx5 debug: > > CONFIG_RTE_ENABLE_ASSERT controls the whole DPDK. > > No, it controls the whole DPDK except mlx PMDs. > > > CONFIG_RTE_LIBRTE_MLX5_DEBUG controls NDEBUG for mlx5 > > > > From my practice - I switch the mlx5 debug option (in the process of > > the debugging/testing datapath and checking the resulting performance, > > by directly defining NDEBUG in mlx5.h and not reconfiguring/rebuilding the > entire DPDK), this fine grained option seems to be useful. > > I don't like having mlx PMDs behave differently. > It make things difficult for newcomers. > And with meson, such options are cleaned up.
Do you mean we should eliminate NDEBUG usage and convert it to some explicit "MLX5_NDEBUG" (and convert "assert" to "MLX5_ASSERT") ? With best regards, Slava
