Re: [dpdk-dev] [PATCH v3 06/10] drivers/crypto: move snow3g PMD to IPsec-mb framework

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

Hi Ciara,

> -----Original Message-----
> From: Power, Ciara <ciara.power@intel.com>
> Sent: Wednesday, September 29, 2021 5:31 PM
> To: dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Bronowski, PiotrX
> <piotrx.bronowski@intel.com>; gakhil@marvell.com; Power, Ciara
> <ciara.power@intel.com>; Thomas Monjalon <thomas@monjalon.net>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Ray Kinsella
> <mdr@ashroe.eu>
> Subject: [PATCH v3 06/10] drivers/crypto: move snow3g PMD to IPsec-mb
> framework
> 
> From: Piotr Bronowski <piotrx.bronowski@intel.com>
> 
> This patch removes the crypto/snow3g folder and gathers all snow3g PMD
> implementation specific details into a single file, pmd_snow3g.c in the
> crypto/ipsec_mb folder.
> 
> Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> 
> ---
> v3: Removed extra empty lines.
> v2: Updated maintainers file.
> ---
>  MAINTAINERS                                   |   8 +-
>  doc/guides/cryptodevs/snow3g.rst              |   3 +-
>  drivers/crypto/ipsec_mb/meson.build           |   3 +-
>  .../pmd_snow3g.c}                             | 457 ++++++++----------
>  .../ipsec_mb/rte_ipsec_mb_pmd_private.h       |   7 +
>  drivers/crypto/meson.build                    |   1 -
>  drivers/crypto/snow3g/meson.build             |  24 -
>  drivers/crypto/snow3g/rte_snow3g_pmd_ops.c    | 323 -------------
>  drivers/crypto/snow3g/snow3g_pmd_private.h    |  84 ----
>  drivers/crypto/snow3g/version.map             |   3 -
>  10 files changed, 205 insertions(+), 708 deletions(-)  rename
> drivers/crypto/{snow3g/rte_snow3g_pmd.c => ipsec_mb/pmd_snow3g.c} (57%)
> delete mode 100644 drivers/crypto/snow3g/meson.build  delete mode 100644
> drivers/crypto/snow3g/rte_snow3g_pmd_ops.c
>  delete mode 100644 drivers/crypto/snow3g/snow3g_pmd_private.h
>  delete mode 100644 drivers/crypto/snow3g/version.map
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 794bad11c2..28855222d6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS

...

> -	case SNOW3G_OP_AUTH_CIPHER:
> +	case IPSEC_MB_OP_HASH_VERIFY_THEN_DECRYPT:
> +	case IPSEC_MB_OP_HASH_GEN_THEN_ENCRYPT:
>  		processed_ops = process_snow3g_hash_op(qp, ops, session,
>  				num_ops);
>  		process_snow3g_cipher_op(qp, ops, session, processed_ops);
> @@ -358,9 +343,9 @@ process_ops(struct rte_crypto_op **ops, struct
> snow3g_session *session,
>  		}
>  	}
> 
> -	enqueued_ops = rte_ring_enqueue_burst(qp->processed_ops,
> +	enqueued_ops = rte_ring_enqueue_burst(qp->ingress_queue,
>  			(void **)ops, processed_ops, NULL);

Looks like there is a bug here. We don't need to enqueue operations back in the ring here.
We used to enqueue in the ring when crypto processing was done in enqueue, but now this is part of dequeue
and we already dequeued the operations from the ring.
As far as I know, the only enqueue operation in the ring should be done in enqueue_burst.

Thanks,
Pablo