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

Message ID 20170403105302.9685-1-akhil.goyal@nxp.com (mailing list archive)
State Rejected, archived
Delegated to: Pablo de Lara Guarch
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Akhil Goyal April 3, 2017, 10:53 a.m. UTC
  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.

Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
---
 test/test/test_cryptodev_perf.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
  

Comments

De Lara Guarch, Pablo April 4, 2017, 3:11 p.m. UTC | #1
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
  
Akhil Goyal April 20, 2017, 10:48 a.m. UTC | #2
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.

Regards,
Akhil
  
De Lara Guarch, Pablo April 26, 2017, 9:38 a.m. UTC | #3
> -----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?

Thanks,
Pablo

> 
> Regards,
> Akhil
> 
>
  
Akhil Goyal April 26, 2017, 9:53 a.m. UTC | #4
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
  
De Lara Guarch, Pablo April 26, 2017, 10:42 a.m. UTC | #5
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Wednesday, April 26, 2017 10:53 AM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Cc: Doherty, Declan; hemant.agrawal@nxp.com
> Subject: Re: [PATCH] test/test: improve dequeue logic for crypto operation
> 
> 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.

For test-crypto-perf, you cannot encounter this problem, because the crypto operation pool is
the same size as the mbuf pool size, so you would run out of crypto operations before hitting this problem.

Thanks,
Pablo

> 
> Thanks,
> Akhil
> 
>
  
Akhil Goyal April 26, 2017, 11:02 a.m. UTC | #6
On 4/26/2017 4:12 PM, De Lara Guarch, Pablo wrote:
>
>
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Wednesday, April 26, 2017 10:53 AM
>> To: De Lara Guarch, Pablo; dev@dpdk.org
>> Cc: Doherty, Declan; hemant.agrawal@nxp.com
>> Subject: Re: [PATCH] test/test: improve dequeue logic for crypto operation
>>
>> 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.
>
> For test-crypto-perf, you cannot encounter this problem, because the crypto operation pool is
> the same size as the mbuf pool size, so you would run out of crypto operations before hitting this problem.
>
> Thanks,
> Pablo
>
As I understand correctly, the pool_sz >= total crypto operations, this 
means, there is no mbuf getting reused. If yes, then yes my patch is not 
required at all.
But, there would be limited number of crypto operations that we can 
perform in test-crypto-perf as we may not be able to allocate so many 
buffers.

Thanks,
Akhil
  
De Lara Guarch, Pablo May 2, 2017, 7:22 a.m. UTC | #7
Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Wednesday, April 26, 2017 12:02 PM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Cc: Doherty, Declan; hemant.agrawal@nxp.com
> Subject: Re: [PATCH] test/test: improve dequeue logic for crypto operation
> 
> On 4/26/2017 4:12 PM, De Lara Guarch, Pablo wrote:
> >
> >
> >> -----Original Message-----
> >> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> >> Sent: Wednesday, April 26, 2017 10:53 AM
> >> To: De Lara Guarch, Pablo; dev@dpdk.org
> >> Cc: Doherty, Declan; hemant.agrawal@nxp.com
> >> Subject: Re: [PATCH] test/test: improve dequeue logic for crypto
> operation
> >>
> >> 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.
> >
> > For test-crypto-perf, you cannot encounter this problem, because the
> crypto operation pool is
> > the same size as the mbuf pool size, so you would run out of crypto
> operations before hitting this problem.
> >
> > Thanks,
> > Pablo
> >
> As I understand correctly, the pool_sz >= total crypto operations, this
> means, there is no mbuf getting reused. If yes, then yes my patch is not
> required at all.
> But, there would be limited number of crypto operations that we can
> perform in test-crypto-perf as we may not be able to allocate so many
> buffers.

Usually, pool size will be smaller than the total number of crypto operations.
But pool size is the same for mbufs and for crypto operations.
Therefore, mbufs will not ever be reused, as there is a 1:1 mapping between crypto operations and mbufs.
You might run out of crypto operations, though, if pool size is very small.
By default, its size is 8192, so it should be big enough (if not, you can change it with the parameter --pool-sz).

Pablo

> 
> Thanks,
> Akhil
> 
>
  

Patch

diff --git a/test/test/test_cryptodev_perf.c b/test/test/test_cryptodev_perf.c
index 9cdbc39..70ee5bc 100644
--- a/test/test/test_cryptodev_perf.c
+++ b/test/test/test_cryptodev_perf.c
@@ -3229,17 +3229,21 @@  test_perf_aes_sha(uint8_t dev_id, uint16_t queue_id,
 			total_enqueued += burst_enqueued;
 		}
 
-		/* dequeue burst */
-		burst_dequeued = rte_cryptodev_dequeue_burst(dev_id, queue_id,
-				proc_ops, pparams->burst_size);
-		if (burst_dequeued == 0)
-			failed_polls++;
-		else {
-			processed += burst_dequeued;
+		do {
+			/* dequeue burst */
+			burst_dequeued = rte_cryptodev_dequeue_burst(dev_id,
+						queue_id, proc_ops,
+						pparams->burst_size);
+			if (burst_dequeued == 0)
+				failed_polls++;
+			else {
+				processed += burst_dequeued;
 
-			for (l = 0; l < burst_dequeued; l++)
-				rte_crypto_op_free(proc_ops[l]);
-		}
+				for (l = 0; l < burst_dequeued; l++)
+					rte_crypto_op_free(proc_ops[l]);
+			}
+		} while ((total_enqueued - processed) >
+			(pparams->burst_size * NUM_MBUF_SETS));
 		j++;
 	}