Re: [dpdk-dev] [PATCH] test/test: improve dequeue logic for crypto operation

[Akhil Goyal <akhil.goyal@nxp.com>]

On 4/26/2017 3:08 PM, De Lara Guarch, Pablo wrote:
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
>> Sent: Thursday, April 20, 2017 11:48 AM
>> To: De Lara Guarch, Pablo; dev@dpdk.org
>> Cc: Doherty, Declan; hemant.agrawal@nxp.com
>> Subject: Re: [dpdk-dev] [PATCH] test/test: improve dequeue logic for crypto
>> operation
>>
>> Hi Pablo,
>>
>> On 4/4/2017 8:41 PM, De Lara Guarch, Pablo wrote:
>>> Hi Akhil,
>>>
>>>> -----Original Message-----
>>>> From: akhil.goyal@nxp.com [mailto:akhil.goyal@nxp.com]
>>>> Sent: Monday, April 03, 2017 11:53 AM
>>>> To: dev@dpdk.org
>>>> Cc: Doherty, Declan; De Lara Guarch, Pablo; Akhil Goyal
>>>> Subject: [PATCH] test/test: improve dequeue logic for crypto operation
>>>>
>>>> From: Akhil Goyal <akhil.goyal@nxp.com>
>>>>
>>>> While enqueue/dequeue operations in test_perf_aes_sha,
>>>> the underlying implementation may not be able to dequeue
>>>> the same number of buffers as enqueued. So, it may be
>>>> necessary to perform more dequeue operations if the gap
>>>> is more than pparams->burst_size * NUM_MBUF_SETS.
>>>>
>>>> Other algos may also need to update the logic if required.
>>>>
>>>
>>> In which way this patch improves the dequeue logic?
>>> Is it improving the performance somehow? From what I see, it is unlikely
>> that you are going to
>>> experience the problem, as the internal ring is PERF_NUM_OPS_INFLIGHT,
>> which is 128,
>>> higher than pparams->burst_size * NUM_MBUF_SETS, which is 256.
>>> And even if you do meet that problem, then you would be reusing mbufs,
>>> but that is OK as we are not verifying the output.
>>>
>>>
>>> Thanks,
>>> Pablo
>>>
>> Sorry for the late response. Somehow the reply went to junk in my mail
>> client and it got missed.
>>
>> The problem would arise if the underlying implementation cannot dequeue
>> the same number of ops as were enqueued in a single dequeue command.
>>
>> Here we have a synchronous calls to enqueue and dequeue in the same
>> thread, so it may happen that for every enqueue of 32 ops, there are
>> lesser number of dequeue ops (say 16). There is no thread to dequeue the
>> left over 16 ops. So the difference would increase slowly and gradually
>> and the application will run out of buffers.
>> So we need a mechanism to drain the left over dequeue ops.
>
> Hi Akhil,
>
> I understand, I guess that this won't happen on a software device, but might happen on hardware.
> As said, I think it is OK to reuse an mbuf by two different crypto operations, because we don't check the output.
>
> Anyway, it might be safer to proceed your way. Two things about it, though:
> 1 - This should be extended to the other tests (such as test_perf_openssl) for consistency.
> 2 - Since we have the test-crypto-perf app now, which cover all these tests, I was thinking of removing test_cryptodev_perf.c,
> to avoid duplications. Any concerns on this?
>
Hi Pablo,

yes, this shall be done for other tests also, but I do not have setup to 
test all of them.
And if we are planning to remove this file altogether, then we may not 
need it anyway.
cperf_throughput_test_runner can alone be modified with my changes, but 
I can test on NXP DPAA2 platform only. I can send the patch for this 
after testing it on DPAA2 platform.

Thanks,
Akhil