[dpdk-dev] [PATCH] crypto/aesni_gcm: do crypto op in dequeue function

[Doherty, Declan]

-----Original Message-----
From: De Lara Guarch, Pablo 
Sent: Thursday, March 30, 2017 12:54 PM
To: Doherty, Declan <declan.doherty at intel.com>; Gonzalez Monroy, Sergio <sergio.gonzalez.monroy at intel.com>; dev at dpdk.org
Cc: stable at dpdk.org
Subject: RE: [PATCH] crypto/aesni_gcm: do crypto op in dequeue function



> -----Original Message-----
> From: Doherty, Declan
> Sent: Thursday, March 30, 2017 12:45 PM
> To: Gonzalez Monroy, Sergio; dev at dpdk.org
> Cc: De Lara Guarch, Pablo; stable at dpdk.org
> Subject: Re: [PATCH] crypto/aesni_gcm: do crypto op in dequeue 
> function
> 
> On 29/03/17 14:42, Sergio Gonzalez Monroy wrote:
> > There is bug when more crypto ops are enqueued than dequeued.
> > The return value is not checked when trying to enqueue the processed 
> > crypto op into the internal ring, which in the case of being full 
> > will results in crypto ops and mbufs being leaked.
> > The issue is more obvious with different cores doing enqueue/dequeue.
> >
> > This patch moves the crypto operation to the dequeue function which 
> > fixes the above issue without having to check for the number of free 
> > entries in the ring.
> >
> > Fixes: eec136f3c54f ("aesni_gcm: add driver for AES-GCM crypto
> operations")
> >
> > Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy at intel.com>
> > ---
> ...
> >
>

Hi Declan,

> Hey Sergio, this looks good but it needs to be rebased for the 
> rte_ring API change. I also see a marginally performance increase with 
> this change using the crypto-perf app which wasn't expected.

Unless there is other change necessary, I can make that change when merging.

Thanks,
Pablo

> 
> 
> Acked-by: Declan Doherty <declan.doherty at intel.com>

Cool, no, that's the only issue.

Cheers
Declan