On 1/9/2020 3:27 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yi...@intel.com>
>> Sent: Thursday, January 9, 2020 17:19
>> To: Slava Ovsiienko <viachesl...@mellanox.com>; dev@dpdk.org
>> Cc: Matan Azrad <ma...@mellanox.com>; Raslan Darawsheh
>> <rasl...@mellanox.com>; Ori Kam <or...@mellanox.com>; Thomas
>> Monjalon <tho...@monjalon.net>
>> Subject: Re: [dpdk-dev] [PATCH v2 4/4] net/mlx5: engage free on completion
>> queue
>>
>> On 1/9/2020 10:56 AM, Viacheslav Ovsiienko wrote:
>>> The free on completion queue keeps the indices of elts array, all mbuf
>>> stored below this index should be freed on arrival of normal send
>>> completion. In debug version it also contains an index of completed
>>> transmitting descriptor (WQE) to check queues synchronization.
>>>
>>> Signed-off-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
>>> Acked-by: Matan Azrad <ma...@mellanox.com>
>>
>> <...>
>>
>>> @@ -2108,17 +2108,18 @@ enum mlx5_txcmp_code {
>>> /*
>>> * We are going to fetch all entries with
>>> * MLX5_CQE_SYNDROME_WR_FLUSH_ERR status.
>>> + * The send queue is supposed to be empty.
>>> */
>>> ++ci;
>>> + txq->cq_pi = ci;
>>> + last_cqe = NULL;
>>> continue;
>>> }
>>> /* Normal transmit completion. */
>>> + 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.
