[dpdk-dev] [EXT] [PATCH v8 04/16] crypto/mlx5: add basic operations

Akhil Goyal gakhil at marvell.com
Fri Jul 16 21:34:00 CEST 2021


> The basic dev control operations are configure, close and get info.
> 
> Extended the existing support of configure and close:
> 	-mlx5_crypto_dev_configure- function used to configure device.
> 	-mlx5_crypto_dev_close-  function used to close a configured
> 	 device.
> 
> Added config struct to user private data with the fields socket id,
> number of queue pairs and feature flags to be disabled.
> 
> Signed-off-by: Shiri Kuzin <shirik at nvidia.com>
> Acked-by: Matan Azrad <matan at nvidia.com>
> ---
>  drivers/crypto/mlx5/mlx5_crypto.c | 26 +++++++++++++++++++-------
>  drivers/crypto/mlx5/mlx5_crypto.h |  1 +
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> b/drivers/crypto/mlx5/mlx5_crypto.c
> index 3f0c97d081..a7e44deb9e 100644
> --- a/drivers/crypto/mlx5/mlx5_crypto.c
> +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> @@ -105,22 +105,27 @@ mlx5_crypto_dev_infos_get(struct rte_cryptodev
> *dev,
>  	}
>  }
> 
> -static unsigned int
> -mlx5_crypto_sym_session_get_size(struct rte_cryptodev *dev
> __rte_unused)
> -{
> -	return sizeof(struct mlx5_crypto_session);
> -}
> -

I do not get the reason to remove above function, it was introduced in the previous patch.
It looks the patches are not properly split.

>  static int
>  mlx5_crypto_dev_configure(struct rte_cryptodev *dev,
> -		struct rte_cryptodev_config *config __rte_unused)
> +			  struct rte_cryptodev_config *config)
>  {
>  	struct mlx5_crypto_priv *priv = dev->data->dev_private;
> 
> +	if (config == NULL) {
> +		DRV_LOG(ERR, "Invalid crypto dev configure parameters.");
> +		return -EINVAL;
> +	}
> +	if ((config->ff_disable & RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO)
> != 0) {
> +		DRV_LOG(ERR,
> +			"Disabled symmetric crypto feature is not
> supported.");
> +		return -ENOTSUP;
> +	}
>  	if (mlx5_crypto_dek_setup(priv) != 0) {
>  		DRV_LOG(ERR, "Dek hash list creation has failed.");
>  		return -ENOMEM;
>  	}
> +	priv->dev_config = *config;
> +	DRV_LOG(DEBUG, "Device %u was configured.", dev->driver_id);
>  	return 0;
>  }

The patch title and the patch do not match.
Title says, add basic operations, which should introduce the
Configure and close ops. But here the configure and close ops
Were already there and you are introducing some new checks
In them.

> 
> @@ -130,9 +135,16 @@ mlx5_crypto_dev_close(struct rte_cryptodev *dev)
>  	struct mlx5_crypto_priv *priv = dev->data->dev_private;
> 
>  	mlx5_crypto_dek_unset(priv);
> +	DRV_LOG(DEBUG, "Device %u was closed.", dev->driver_id);
>  	return 0;
>  }
Logging could have been added in the patch where dev_close was added.

> 
> +static unsigned int
> +mlx5_crypto_sym_session_get_size(struct rte_cryptodev *dev
> __rte_unused)
> +{
> +	return sizeof(struct mlx5_crypto_session);
> +}
> +
>  static int
>  mlx5_crypto_sym_session_configure(struct rte_cryptodev *dev,
>  				  struct rte_crypto_sym_xform *xform,
> diff --git a/drivers/crypto/mlx5/mlx5_crypto.h
> b/drivers/crypto/mlx5/mlx5_crypto.h
> index 167e9e57ad..a0df775407 100644
> --- a/drivers/crypto/mlx5/mlx5_crypto.h
> +++ b/drivers/crypto/mlx5/mlx5_crypto.h
> @@ -24,6 +24,7 @@ struct mlx5_crypto_priv {
>  	uint32_t pdn; /* Protection Domain number. */
>  	struct ibv_pd *pd;
>  	struct mlx5_hlist *dek_hlist; /* Dek hash list. */
> +	struct rte_cryptodev_config dev_config;
>  };
> 
>  struct mlx5_crypto_dek {
> --
> 2.27.0



More information about the dev mailing list