Re: [dpdk-dev] [PATCH v6 0/8] add Nitrox crypto device support

[Nagadheeraj Rottela <rnagadheeraj@marvell.com>]

Hi Gavin,

Thanks for your comments. Your suggestions are applicable in this case.
I will remove the usage of rte_wmb() and rte_rmb() in all places as they are not required.
I will add rte_smp_wmb() before pending count update in enqueue operation and add rte_smp_rmb() before reading softreq from pending queue.
I will add rte_io_wmb() before the ring doorbell to ensure all the DMA buffers & descriptors stores are completed.
We check command completion based on the completion code update (which is the last word in the output buffer) and hence rte_io_rmb() is not required.

Best Regards,
Dheeraj

> -----Original Message-----
> From: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>
> Sent: Saturday, September 28, 2019 8:17 PM
> To: Nagadheeraj Rottela <rnagadheeraj@marvell.com>;
> Akhil.goyal@nxp.com; pablo.de.lara.guarch@intel.com
> Cc: Srikanth Jampala <jsrikanth@marvell.com>; dev@dpdk.org; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; Gavin Hu (Arm Technology
> China) <Gavin.Hu@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: [EXT] RE: [PATCH v6 0/8] add Nitrox crypto device support
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Nagadheeraj,
> 
> I am no expert in crypto dev, maybe you can educate me if I am wrong:
> I got an impression in this series, the barriers were used too much, too
> heavily and unnecessarily.
> 
> For enqueue operations, I understand they are stores to the DMA buffer,
> the queue will be fetched and updated by the crypto device after processing,
> then dequeued by the other CPU cores. So for enqueue operations, an
> rte_io_wmb is required before the doorbell ringing,  and an rte_smp_wmb is
> required to ensure the enqueue operations were done before the consumer
> on the other side(who dequeues) sees the updated pending_count. For
> dequeue operations, rte_smp_rmb is required after reading the
> pending_count to ensure reading the intact content from the queue(if the
> queue entries were not handled yet by the crypto dev, the status will show
> that, maybe an rte_io_rmb is required to ensure the status is read out first).
> 
> The rte_smp_xmb can even be optimized with C11 atomics, but it can be
> next step.
> 
> Best Regards,
> Gavin