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.
