[v8,02/16] crypto/mlx5: add DEK object management
Checks
Commit Message
A DEK(Data encryption Key) is an mlx5 HW object which represents the
cipher algorithm key.
The DEKs are used during data encryption/decryption operations.
In symmetric algorithms like AES-STS, we use the same DEK for both
encryption and decryption.
Use the mlx5 hash-list tool to manage the DEK objects in the PMD.
Provide the compare, create and destroy functions to manage DEKs in
hash-list and introduce an internal API to setup and unset the DEK
management and to prepare and destroy specific DEK object.
The DEK hash-list will be created in dev_configure routine and
destroyed in dev_close routine.
Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
drivers/crypto/mlx5/meson.build | 1 +
drivers/crypto/mlx5/mlx5_crypto.c | 42 ++++---
drivers/crypto/mlx5/mlx5_crypto.h | 51 ++++++++
drivers/crypto/mlx5/mlx5_crypto_dek.c | 161 ++++++++++++++++++++++++++
4 files changed, 239 insertions(+), 16 deletions(-)
create mode 100644 drivers/crypto/mlx5/mlx5_crypto.h
create mode 100644 drivers/crypto/mlx5/mlx5_crypto_dek.c
Comments
> A DEK(Data encryption Key) is an mlx5 HW object which represents the
> cipher algorithm key.
> The DEKs are used during data encryption/decryption operations.
>
> In symmetric algorithms like AES-STS, we use the same DEK for both
> encryption and decryption.
>
> Use the mlx5 hash-list tool to manage the DEK objects in the PMD.
>
> Provide the compare, create and destroy functions to manage DEKs in
> hash-list and introduce an internal API to setup and unset the DEK
> management and to prepare and destroy specific DEK object.
>
> The DEK hash-list will be created in dev_configure routine and
> destroyed in dev_close routine.
>
> Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
> drivers/crypto/mlx5/meson.build | 1 +
> drivers/crypto/mlx5/mlx5_crypto.c | 42 ++++---
> drivers/crypto/mlx5/mlx5_crypto.h | 51 ++++++++
> drivers/crypto/mlx5/mlx5_crypto_dek.c | 161
> ++++++++++++++++++++++++++
> 4 files changed, 239 insertions(+), 16 deletions(-)
> create mode 100644 drivers/crypto/mlx5/mlx5_crypto.h
> create mode 100644 drivers/crypto/mlx5/mlx5_crypto_dek.c
>
> diff --git a/drivers/crypto/mlx5/meson.build
> b/drivers/crypto/mlx5/meson.build
> index 6fd70bc477..d55cdbfe6f 100644
> --- a/drivers/crypto/mlx5/meson.build
> +++ b/drivers/crypto/mlx5/meson.build
> @@ -11,6 +11,7 @@ fmt_name = 'mlx5_crypto'
> deps += ['common_mlx5', 'eal', 'cryptodev']
> sources = files(
> 'mlx5_crypto.c',
> + 'mlx5_crypto_dek.c',
> )
> cflags_options = [
> '-std=c11',
> diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> b/drivers/crypto/mlx5/mlx5_crypto.c
> index fbe3c21aae..d2d82c7b15 100644
> --- a/drivers/crypto/mlx5/mlx5_crypto.c
> +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> @@ -3,12 +3,9 @@
> */
>
> #include <rte_malloc.h>
> -#include <rte_log.h>
> #include <rte_errno.h>
> +#include <rte_log.h>
> #include <rte_pci.h>
> -#include <rte_crypto.h>
> -#include <rte_cryptodev.h>
> -#include <rte_cryptodev_pmd.h>
There is some issue in the splitting of the patches,
The above headers are added in first patch and moved to a header file in this patch.
Take reference of the cnxk crypto driver which got merged recently.
>
> #include <mlx5_glue.h>
> #include <mlx5_common.h>
> @@ -17,6 +14,7 @@
> #include <mlx5_common_os.h>
>
> #include "mlx5_crypto_utils.h"
> +#include "mlx5_crypto.h"
>
> #define MLX5_CRYPTO_DRIVER_NAME mlx5_crypto
> #define MLX5_CRYPTO_LOG_NAME pmd.crypto.mlx5
> @@ -24,16 +22,6 @@
> #define MLX5_CRYPTO_FEATURE_FLAGS \
> RTE_CRYPTODEV_FF_HW_ACCELERATED
>
> -struct mlx5_crypto_priv {
> - TAILQ_ENTRY(mlx5_crypto_priv) next;
> - struct ibv_context *ctx; /* Device context. */
> - struct rte_pci_device *pci_dev;
> - struct rte_cryptodev *crypto_dev;
> - void *uar; /* User Access Region. */
> - uint32_t pdn; /* Protection Domain number. */
> - struct ibv_pd *pd;
> -};
> -
> TAILQ_HEAD(mlx5_crypto_privs, mlx5_crypto_priv) mlx5_crypto_priv_list =
>
> TAILQ_HEAD_INITIALIZER(mlx5_crypto_priv_list);
> static pthread_mutex_t priv_list_lock = PTHREAD_MUTEX_INITIALIZER;
> @@ -51,11 +39,33 @@ static const struct rte_driver mlx5_drv = {
>
> static struct cryptodev_driver mlx5_cryptodev_driver;
>
> +static int
> +mlx5_crypto_dev_configure(struct rte_cryptodev *dev,
> + struct rte_cryptodev_config *config __rte_unused)
> +{
> + struct mlx5_crypto_priv *priv = dev->data->dev_private;
> +
> + if (mlx5_crypto_dek_setup(priv) != 0) {
> + DRV_LOG(ERR, "Dek hash list creation has failed.");
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> +static int
> +mlx5_crypto_dev_close(struct rte_cryptodev *dev)
> +{
> + struct mlx5_crypto_priv *priv = dev->data->dev_private;
> +
> + mlx5_crypto_dek_unset(priv);
> + return 0;
> +}
> +
> static struct rte_cryptodev_ops mlx5_crypto_ops = {
> - .dev_configure = NULL,
> + .dev_configure = mlx5_crypto_dev_configure,
> .dev_start = NULL,
> .dev_stop = NULL,
> - .dev_close = NULL,
> + .dev_close = mlx5_crypto_dev_close,
> .dev_infos_get = NULL,
> .stats_get = NULL,
> .stats_reset = NULL,
> diff --git a/drivers/crypto/mlx5/mlx5_crypto.h
> b/drivers/crypto/mlx5/mlx5_crypto.h
> new file mode 100644
> index 0000000000..167e9e57ad
> --- /dev/null
> +++ b/drivers/crypto/mlx5/mlx5_crypto.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2021 NVIDIA Corporation & Affiliates
> + */
> +
> +#ifndef MLX5_CRYPTO_H_
> +#define MLX5_CRYPTO_H_
> +
> +#include <stdbool.h>
> +
> +#include <rte_cryptodev.h>
> +#include <rte_cryptodev_pmd.h>
> +
> +#include <mlx5_common_utils.h>
> +
> +#define MLX5_CRYPTO_DEK_HTABLE_SZ (1 << 11)
> +#define MLX5_CRYPTO_KEY_LENGTH 80
> +
> +struct mlx5_crypto_priv {
> + TAILQ_ENTRY(mlx5_crypto_priv) next;
> + struct ibv_context *ctx; /* Device context. */
> + struct rte_pci_device *pci_dev;
> + struct rte_cryptodev *crypto_dev;
> + void *uar; /* User Access Region. */
> + uint32_t pdn; /* Protection Domain number. */
> + struct ibv_pd *pd;
> + struct mlx5_hlist *dek_hlist; /* Dek hash list. */
> +};
> +
> +struct mlx5_crypto_dek {
> + struct mlx5_list_entry entry; /* Pointer to DEK hash list entry. */
> + struct mlx5_devx_obj *obj; /* Pointer to DEK DevX object. */
> + uint8_t data[MLX5_CRYPTO_KEY_LENGTH]; /* DEK key data. */
> + bool size_is_48; /* Whether the key\data size is 48 bytes or not. */
> +} __rte_cache_aligned;
> +
> +int
> +mlx5_crypto_dek_destroy(struct mlx5_crypto_priv *priv,
> + struct mlx5_crypto_dek *dek);
> +
> +struct mlx5_crypto_dek *
> +mlx5_crypto_dek_prepare(struct mlx5_crypto_priv *priv,
> + struct rte_crypto_cipher_xform *cipher);
> +
> +int
> +mlx5_crypto_dek_setup(struct mlx5_crypto_priv *priv);
> +
> +void
> +mlx5_crypto_dek_unset(struct mlx5_crypto_priv *priv);
> +
> +#endif /* MLX5_CRYPTO_H_ */
> +
> diff --git a/drivers/crypto/mlx5/mlx5_crypto_dek.c
> b/drivers/crypto/mlx5/mlx5_crypto_dek.c
> new file mode 100644
> index 0000000000..43d1bcc9f8
> --- /dev/null
> +++ b/drivers/crypto/mlx5/mlx5_crypto_dek.c
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (c) 2021 NVIDIA Corporation & Affiliates
> + */
> +
> +#include <rte_ip.h>
> +#include <rte_common.h>
> +#include <rte_errno.h>
> +#include <rte_log.h>
> +
> +#include <mlx5_prm.h>
> +#include <mlx5_devx_cmds.h>
> +
> +#include "mlx5_crypto_utils.h"
> +#include "mlx5_crypto.h"
> +
> +struct mlx5_crypto_dek_ctx {
> + struct rte_crypto_cipher_xform *cipher;
> + struct mlx5_crypto_priv *priv;
> +};
> +
> +int
> +mlx5_crypto_dek_destroy(struct mlx5_crypto_priv *priv,
> + struct mlx5_crypto_dek *dek)
> +{
> + return mlx5_hlist_unregister(priv->dek_hlist, &dek->entry);
> +}
> +
> +struct mlx5_crypto_dek *
> +mlx5_crypto_dek_prepare(struct mlx5_crypto_priv *priv,
> + struct rte_crypto_cipher_xform *cipher)
> +{
> + struct mlx5_hlist *dek_hlist = priv->dek_hlist;
> + struct mlx5_crypto_dek_ctx dek_ctx = {
> + .cipher = cipher,
> + .priv = priv,
> + };
> + struct rte_crypto_cipher_xform *cipher_ctx = cipher;
> + uint64_t key64 = __rte_raw_cksum(cipher_ctx->key.data,
> + cipher_ctx->key.length, 0);
> + struct mlx5_list_entry *entry = mlx5_hlist_register(dek_hlist,
> + key64, &dek_ctx);
> +
> + return entry == NULL ? NULL :
> + container_of(entry, struct mlx5_crypto_dek,
> entry);
> +}
> +
> +static struct mlx5_list_entry *
> +mlx5_crypto_dek_clone_cb(void *tool_ctx __rte_unused,
> + struct mlx5_list_entry *oentry,
> + void *cb_ctx __rte_unused)
> +{
> + struct mlx5_crypto_dek *entry = rte_zmalloc(__func__,
> sizeof(*entry),
> + RTE_CACHE_LINE_SIZE);
> +
> + if (!entry) {
> + DRV_LOG(ERR, "Cannot allocate dek resource memory.");
> + rte_errno = ENOMEM;
> + return NULL;
> + }
> + memcpy(entry, oentry, sizeof(*entry));
> + return &entry->entry;
> +}
> +
> +static void
> +mlx5_crypto_dek_clone_free_cb(void *tool_ctx __rte_unused,
> + struct mlx5_list_entry *entry)
> +{
> + struct mlx5_crypto_dek *dek = container_of(entry,
> + struct mlx5_crypto_dek,
> entry);
> +
> + rte_free(dek);
> +}
> +
> +static int
> +mlx5_crypto_dek_match_cb(void *tool_ctx __rte_unused,
> + struct mlx5_list_entry *entry, void *cb_ctx)
> +{
> + struct mlx5_crypto_dek_ctx *ctx = cb_ctx;
> + struct rte_crypto_cipher_xform *cipher_ctx = ctx->cipher;
> + struct mlx5_crypto_dek *dek =
> + container_of(entry, typeof(*dek), entry);
> + uint32_t key_len = dek->size_is_48 ? 48 : 80;
> +
> + if (key_len != cipher_ctx->key.length)
> + return -1;
> + return memcmp(cipher_ctx->key.data, dek->data, key_len);
> +}
> +
> +static struct mlx5_list_entry *
> +mlx5_crypto_dek_create_cb(void *tool_ctx __rte_unused, void *cb_ctx)
> +{
> + struct mlx5_crypto_dek_ctx *ctx = cb_ctx;
> + struct rte_crypto_cipher_xform *cipher_ctx = ctx->cipher;
> + struct mlx5_crypto_dek *dek = rte_zmalloc(__func__, sizeof(*dek),
> + RTE_CACHE_LINE_SIZE);
> + struct mlx5_devx_dek_attr dek_attr = {
> + .pd = ctx->priv->pdn,
> + .key_purpose = MLX5_CRYPTO_KEY_PURPOSE_AES_XTS,
> + .has_keytag = 1,
> + };
> +
> + if (dek == NULL) {
> + DRV_LOG(ERR, "Failed to allocate dek memory.");
> + return NULL;
> + }
> + switch (cipher_ctx->key.length) {
> + case 48:
> + dek->size_is_48 = true;
> + dek_attr.key_size = MLX5_CRYPTO_KEY_SIZE_128b;
> + break;
> + case 80:
> + dek->size_is_48 = false;
> + dek_attr.key_size = MLX5_CRYPTO_KEY_SIZE_256b;
> + break;
> + default:
> + DRV_LOG(ERR, "Key size not supported.");
> + return NULL;
> + }
> + memcpy(&dek_attr.key, cipher_ctx->key.data, cipher_ctx-
> >key.length);
> + dek->obj = mlx5_devx_cmd_create_dek_obj(ctx->priv->ctx,
> &dek_attr);
> + if (dek->obj == NULL) {
> + rte_free(dek);
> + return NULL;
> + }
> + memcpy(&dek->data, cipher_ctx->key.data, cipher_ctx->key.length);
> + return &dek->entry;
> +}
> +
> +static void
> +mlx5_crypto_dek_remove_cb(void *tool_ctx __rte_unused,
> + struct mlx5_list_entry *entry)
> +{
> + struct mlx5_crypto_dek *dek =
> + container_of(entry, typeof(*dek), entry);
> +
> + claim_zero(mlx5_devx_cmd_destroy(dek->obj));
> + rte_free(dek);
> +}
> +
> +
Extra line...
> +int
> +mlx5_crypto_dek_setup(struct mlx5_crypto_priv *priv)
> +{
> + priv->dek_hlist = mlx5_hlist_create("dek_hlist",
> + MLX5_CRYPTO_DEK_HTABLE_SZ,
> + 0, 1, NULL, mlx5_crypto_dek_create_cb,
> + mlx5_crypto_dek_match_cb,
> + mlx5_crypto_dek_remove_cb,
> + mlx5_crypto_dek_clone_cb,
> + mlx5_crypto_dek_clone_free_cb);
> + if (priv->dek_hlist == NULL)
> + return -1;
> + return 0;
> +}
> +
> +void
> +mlx5_crypto_dek_unset(struct mlx5_crypto_priv *priv)
> +{
> + mlx5_hlist_destroy(priv->dek_hlist);
> + priv->dek_hlist = NULL;
> +}
> --
> 2.27.0
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Saturday, July 17, 2021 3:26 AM
> To: Shiri Kuzin <shirik@nvidia.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Suanming Mou
> <suanmingm@nvidia.com>; david.marchand@redhat.com
> Subject: RE: [EXT] [PATCH v8 02/16] crypto/mlx5: add DEK object management
>
> > A DEK(Data encryption Key) is an mlx5 HW object which represents the
> > cipher algorithm key.
> > The DEKs are used during data encryption/decryption operations.
> >
> > In symmetric algorithms like AES-STS, we use the same DEK for both
> > encryption and decryption.
> >
> > Use the mlx5 hash-list tool to manage the DEK objects in the PMD.
> >
> > Provide the compare, create and destroy functions to manage DEKs in
> > hash-list and introduce an internal API to setup and unset the DEK
> > management and to prepare and destroy specific DEK object.
> >
> > The DEK hash-list will be created in dev_configure routine and
> > destroyed in dev_close routine.
> >
> > Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> > Acked-by: Matan Azrad <matan@nvidia.com>
> > ---
> > drivers/crypto/mlx5/meson.build | 1 +
> > drivers/crypto/mlx5/mlx5_crypto.c | 42 ++++---
> > drivers/crypto/mlx5/mlx5_crypto.h | 51 ++++++++
> > drivers/crypto/mlx5/mlx5_crypto_dek.c | 161
> > ++++++++++++++++++++++++++
> > 4 files changed, 239 insertions(+), 16 deletions(-) create mode
> > 100644 drivers/crypto/mlx5/mlx5_crypto.h create mode 100644
> > drivers/crypto/mlx5/mlx5_crypto_dek.c
> >
> > diff --git a/drivers/crypto/mlx5/meson.build
> > b/drivers/crypto/mlx5/meson.build index 6fd70bc477..d55cdbfe6f 100644
> > --- a/drivers/crypto/mlx5/meson.build
> > +++ b/drivers/crypto/mlx5/meson.build
> > @@ -11,6 +11,7 @@ fmt_name = 'mlx5_crypto'
> > deps += ['common_mlx5', 'eal', 'cryptodev'] sources = files(
> > 'mlx5_crypto.c',
> > + 'mlx5_crypto_dek.c',
> > )
> > cflags_options = [
> > '-std=c11',
> > diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> > b/drivers/crypto/mlx5/mlx5_crypto.c
> > index fbe3c21aae..d2d82c7b15 100644
> > --- a/drivers/crypto/mlx5/mlx5_crypto.c
> > +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> > @@ -3,12 +3,9 @@
> > */
> >
> > #include <rte_malloc.h>
> > -#include <rte_log.h>
> > #include <rte_errno.h>
> > +#include <rte_log.h>
> > #include <rte_pci.h>
> > -#include <rte_crypto.h>
> > -#include <rte_cryptodev.h>
> > -#include <rte_cryptodev_pmd.h>
>
> There is some issue in the splitting of the patches, The above headers are added
> in first patch and moved to a header file in this patch.
> Take reference of the cnxk crypto driver which got merged recently.
The main reason is that in the patch we add a new c file: drivers/crypto/mlx5/mlx5_crypto_dek.c
Now mlx5_crypto.c and that new c file both share the new h file mlx5_crypto.h, so all the common includes are moved to the mlx5_crypto.h file.
The header files include are changed due to the new add codes.
>
> >
> > > A DEK(Data encryption Key) is an mlx5 HW object which represents the
> > > cipher algorithm key.
> > > The DEKs are used during data encryption/decryption operations.
> > >
> > > In symmetric algorithms like AES-STS, we use the same DEK for both
> > > encryption and decryption.
> > >
> > > Use the mlx5 hash-list tool to manage the DEK objects in the PMD.
> > >
> > > Provide the compare, create and destroy functions to manage DEKs in
> > > hash-list and introduce an internal API to setup and unset the DEK
> > > management and to prepare and destroy specific DEK object.
> > >
> > > The DEK hash-list will be created in dev_configure routine and
> > > destroyed in dev_close routine.
> > >
> > > Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> > > Acked-by: Matan Azrad <matan@nvidia.com>
> > > ---
> > > drivers/crypto/mlx5/meson.build | 1 +
> > > drivers/crypto/mlx5/mlx5_crypto.c | 42 ++++---
> > > drivers/crypto/mlx5/mlx5_crypto.h | 51 ++++++++
> > > drivers/crypto/mlx5/mlx5_crypto_dek.c | 161
> > > ++++++++++++++++++++++++++
> > > 4 files changed, 239 insertions(+), 16 deletions(-) create mode
> > > 100644 drivers/crypto/mlx5/mlx5_crypto.h create mode 100644
> > > drivers/crypto/mlx5/mlx5_crypto_dek.c
> > >
> > > diff --git a/drivers/crypto/mlx5/meson.build
> > > b/drivers/crypto/mlx5/meson.build index 6fd70bc477..d55cdbfe6f
> 100644
> > > --- a/drivers/crypto/mlx5/meson.build
> > > +++ b/drivers/crypto/mlx5/meson.build
> > > @@ -11,6 +11,7 @@ fmt_name = 'mlx5_crypto'
> > > deps += ['common_mlx5', 'eal', 'cryptodev'] sources = files(
> > > 'mlx5_crypto.c',
> > > + 'mlx5_crypto_dek.c',
> > > )
> > > cflags_options = [
> > > '-std=c11',
> > > diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> > > b/drivers/crypto/mlx5/mlx5_crypto.c
> > > index fbe3c21aae..d2d82c7b15 100644
> > > --- a/drivers/crypto/mlx5/mlx5_crypto.c
> > > +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> > > @@ -3,12 +3,9 @@
> > > */
> > >
> > > #include <rte_malloc.h>
> > > -#include <rte_log.h>
> > > #include <rte_errno.h>
> > > +#include <rte_log.h>
> > > #include <rte_pci.h>
> > > -#include <rte_crypto.h>
> > > -#include <rte_cryptodev.h>
> > > -#include <rte_cryptodev_pmd.h>
> >
> > There is some issue in the splitting of the patches, The above headers are
> added
> > in first patch and moved to a header file in this patch.
> > Take reference of the cnxk crypto driver which got merged recently.
>
> The main reason is that in the patch we add a new c file:
> drivers/crypto/mlx5/mlx5_crypto_dek.c
> Now mlx5_crypto.c and that new c file both share the new h file
> mlx5_crypto.h, so all the common includes are moved to the mlx5_crypto.h
> file.
> The header files include are changed due to the new add codes.
>
Is it not good to add these headers to mlx5_crypto.h in the first place?
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, July 20, 2021 4:36 PM
> To: Suanming Mou <suanmingm@nvidia.com>; Shiri Kuzin <shirik@nvidia.com>;
> dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; david.marchand@redhat.com
> Subject: RE: [EXT] [PATCH v8 02/16] crypto/mlx5: add DEK object management
>
> > > > A DEK(Data encryption Key) is an mlx5 HW object which represents
> > > > the cipher algorithm key.
> > > > The DEKs are used during data encryption/decryption operations.
> > > >
> > > > In symmetric algorithms like AES-STS, we use the same DEK for both
> > > > encryption and decryption.
> > > >
> > > > Use the mlx5 hash-list tool to manage the DEK objects in the PMD.
> > > >
> > > > Provide the compare, create and destroy functions to manage DEKs
> > > > in hash-list and introduce an internal API to setup and unset the
> > > > DEK management and to prepare and destroy specific DEK object.
> > > >
> > > > The DEK hash-list will be created in dev_configure routine and
> > > > destroyed in dev_close routine.
> > > >
> > > > Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> > > > Acked-by: Matan Azrad <matan@nvidia.com>
> > > > ---
> > > > drivers/crypto/mlx5/meson.build | 1 +
> > > > drivers/crypto/mlx5/mlx5_crypto.c | 42 ++++---
> > > > drivers/crypto/mlx5/mlx5_crypto.h | 51 ++++++++
> > > > drivers/crypto/mlx5/mlx5_crypto_dek.c | 161
> > > > ++++++++++++++++++++++++++
> > > > 4 files changed, 239 insertions(+), 16 deletions(-) create mode
> > > > 100644 drivers/crypto/mlx5/mlx5_crypto.h create mode 100644
> > > > drivers/crypto/mlx5/mlx5_crypto_dek.c
> > > >
> > > > diff --git a/drivers/crypto/mlx5/meson.build
> > > > b/drivers/crypto/mlx5/meson.build index 6fd70bc477..d55cdbfe6f
> > 100644
> > > > --- a/drivers/crypto/mlx5/meson.build
> > > > +++ b/drivers/crypto/mlx5/meson.build
> > > > @@ -11,6 +11,7 @@ fmt_name = 'mlx5_crypto'
> > > > deps += ['common_mlx5', 'eal', 'cryptodev'] sources = files(
> > > > 'mlx5_crypto.c',
> > > > + 'mlx5_crypto_dek.c',
> > > > )
> > > > cflags_options = [
> > > > '-std=c11',
> > > > diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> > > > b/drivers/crypto/mlx5/mlx5_crypto.c
> > > > index fbe3c21aae..d2d82c7b15 100644
> > > > --- a/drivers/crypto/mlx5/mlx5_crypto.c
> > > > +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> > > > @@ -3,12 +3,9 @@
> > > > */
> > > >
> > > > #include <rte_malloc.h>
> > > > -#include <rte_log.h>
> > > > #include <rte_errno.h>
> > > > +#include <rte_log.h>
> > > > #include <rte_pci.h>
> > > > -#include <rte_crypto.h>
> > > > -#include <rte_cryptodev.h>
> > > > -#include <rte_cryptodev_pmd.h>
> > >
> > > There is some issue in the splitting of the patches, The above
> > > headers are
> > added
> > > in first patch and moved to a header file in this patch.
> > > Take reference of the cnxk crypto driver which got merged recently.
> >
> > The main reason is that in the patch we add a new c file:
> > drivers/crypto/mlx5/mlx5_crypto_dek.c
> > Now mlx5_crypto.c and that new c file both share the new h file
> > mlx5_crypto.h, so all the common includes are moved to the
> > mlx5_crypto.h file.
> > The header files include are changed due to the new add codes.
> >
> Is it not good to add these headers to mlx5_crypto.h in the first place?
As we can see the single c file satisfied everything in the first patch, add an extra h file with only with includes seems weird in the first patch.
Then in this patch, we have two c file, we adjust the includes to the new shared h file. Everything is changed based on the new added codes.
Please let us know if you insist on that or not.
> > > > > A DEK(Data encryption Key) is an mlx5 HW object which represents
> > > > > the cipher algorithm key.
> > > > > The DEKs are used during data encryption/decryption operations.
> > > > >
> > > > > In symmetric algorithms like AES-STS, we use the same DEK for both
> > > > > encryption and decryption.
> > > > >
> > > > > Use the mlx5 hash-list tool to manage the DEK objects in the PMD.
> > > > >
> > > > > Provide the compare, create and destroy functions to manage DEKs
> > > > > in hash-list and introduce an internal API to setup and unset the
> > > > > DEK management and to prepare and destroy specific DEK object.
> > > > >
> > > > > The DEK hash-list will be created in dev_configure routine and
> > > > > destroyed in dev_close routine.
> > > > >
> > > > > Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> > > > > Acked-by: Matan Azrad <matan@nvidia.com>
> > > > > ---
> > > > > drivers/crypto/mlx5/meson.build | 1 +
> > > > > drivers/crypto/mlx5/mlx5_crypto.c | 42 ++++---
> > > > > drivers/crypto/mlx5/mlx5_crypto.h | 51 ++++++++
> > > > > drivers/crypto/mlx5/mlx5_crypto_dek.c | 161
> > > > > ++++++++++++++++++++++++++
> > > > > 4 files changed, 239 insertions(+), 16 deletions(-) create mode
> > > > > 100644 drivers/crypto/mlx5/mlx5_crypto.h create mode 100644
> > > > > drivers/crypto/mlx5/mlx5_crypto_dek.c
> > > > >
> > > > > diff --git a/drivers/crypto/mlx5/meson.build
> > > > > b/drivers/crypto/mlx5/meson.build index 6fd70bc477..d55cdbfe6f
> > > 100644
> > > > > --- a/drivers/crypto/mlx5/meson.build
> > > > > +++ b/drivers/crypto/mlx5/meson.build
> > > > > @@ -11,6 +11,7 @@ fmt_name = 'mlx5_crypto'
> > > > > deps += ['common_mlx5', 'eal', 'cryptodev'] sources = files(
> > > > > 'mlx5_crypto.c',
> > > > > + 'mlx5_crypto_dek.c',
> > > > > )
> > > > > cflags_options = [
> > > > > '-std=c11',
> > > > > diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> > > > > b/drivers/crypto/mlx5/mlx5_crypto.c
> > > > > index fbe3c21aae..d2d82c7b15 100644
> > > > > --- a/drivers/crypto/mlx5/mlx5_crypto.c
> > > > > +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> > > > > @@ -3,12 +3,9 @@
> > > > > */
> > > > >
> > > > > #include <rte_malloc.h>
> > > > > -#include <rte_log.h>
> > > > > #include <rte_errno.h>
> > > > > +#include <rte_log.h>
> > > > > #include <rte_pci.h>
> > > > > -#include <rte_crypto.h>
> > > > > -#include <rte_cryptodev.h>
> > > > > -#include <rte_cryptodev_pmd.h>
> > > >
> > > > There is some issue in the splitting of the patches, The above
> > > > headers are
> > > added
> > > > in first patch and moved to a header file in this patch.
> > > > Take reference of the cnxk crypto driver which got merged recently.
> > >
> > > The main reason is that in the patch we add a new c file:
> > > drivers/crypto/mlx5/mlx5_crypto_dek.c
> > > Now mlx5_crypto.c and that new c file both share the new h file
> > > mlx5_crypto.h, so all the common includes are moved to the
> > > mlx5_crypto.h file.
> > > The header files include are changed due to the new add codes.
> > >
> > Is it not good to add these headers to mlx5_crypto.h in the first place?
>
> As we can see the single c file satisfied everything in the first patch, add an
> extra h file with only with includes seems weird in the first patch.
> Then in this patch, we have two c file, we adjust the includes to the new
> shared h file. Everything is changed based on the new added codes.
> Please let us know if you insist on that or not.
When you add the base infrastructure patch, it places all the necessary pieces which are
required for the subsequent patches so that when needed, code can be added and
unnecessary pieces of code are not moved again and again which is happening in the
current case.
This looks cleaner and easy to understand while reviewing. Frequent movement of code
Looks complex and difficult to understand.
@@ -11,6 +11,7 @@ fmt_name = 'mlx5_crypto'
deps += ['common_mlx5', 'eal', 'cryptodev']
sources = files(
'mlx5_crypto.c',
+ 'mlx5_crypto_dek.c',
)
cflags_options = [
'-std=c11',
@@ -3,12 +3,9 @@
*/
#include <rte_malloc.h>
-#include <rte_log.h>
#include <rte_errno.h>
+#include <rte_log.h>
#include <rte_pci.h>
-#include <rte_crypto.h>
-#include <rte_cryptodev.h>
-#include <rte_cryptodev_pmd.h>
#include <mlx5_glue.h>
#include <mlx5_common.h>
@@ -17,6 +14,7 @@
#include <mlx5_common_os.h>
#include "mlx5_crypto_utils.h"
+#include "mlx5_crypto.h"
#define MLX5_CRYPTO_DRIVER_NAME mlx5_crypto
#define MLX5_CRYPTO_LOG_NAME pmd.crypto.mlx5
@@ -24,16 +22,6 @@
#define MLX5_CRYPTO_FEATURE_FLAGS \
RTE_CRYPTODEV_FF_HW_ACCELERATED
-struct mlx5_crypto_priv {
- TAILQ_ENTRY(mlx5_crypto_priv) next;
- struct ibv_context *ctx; /* Device context. */
- struct rte_pci_device *pci_dev;
- struct rte_cryptodev *crypto_dev;
- void *uar; /* User Access Region. */
- uint32_t pdn; /* Protection Domain number. */
- struct ibv_pd *pd;
-};
-
TAILQ_HEAD(mlx5_crypto_privs, mlx5_crypto_priv) mlx5_crypto_priv_list =
TAILQ_HEAD_INITIALIZER(mlx5_crypto_priv_list);
static pthread_mutex_t priv_list_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -51,11 +39,33 @@ static const struct rte_driver mlx5_drv = {
static struct cryptodev_driver mlx5_cryptodev_driver;
+static int
+mlx5_crypto_dev_configure(struct rte_cryptodev *dev,
+ struct rte_cryptodev_config *config __rte_unused)
+{
+ struct mlx5_crypto_priv *priv = dev->data->dev_private;
+
+ if (mlx5_crypto_dek_setup(priv) != 0) {
+ DRV_LOG(ERR, "Dek hash list creation has failed.");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+static int
+mlx5_crypto_dev_close(struct rte_cryptodev *dev)
+{
+ struct mlx5_crypto_priv *priv = dev->data->dev_private;
+
+ mlx5_crypto_dek_unset(priv);
+ return 0;
+}
+
static struct rte_cryptodev_ops mlx5_crypto_ops = {
- .dev_configure = NULL,
+ .dev_configure = mlx5_crypto_dev_configure,
.dev_start = NULL,
.dev_stop = NULL,
- .dev_close = NULL,
+ .dev_close = mlx5_crypto_dev_close,
.dev_infos_get = NULL,
.stats_get = NULL,
.stats_reset = NULL,
new file mode 100644
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2021 NVIDIA Corporation & Affiliates
+ */
+
+#ifndef MLX5_CRYPTO_H_
+#define MLX5_CRYPTO_H_
+
+#include <stdbool.h>
+
+#include <rte_cryptodev.h>
+#include <rte_cryptodev_pmd.h>
+
+#include <mlx5_common_utils.h>
+
+#define MLX5_CRYPTO_DEK_HTABLE_SZ (1 << 11)
+#define MLX5_CRYPTO_KEY_LENGTH 80
+
+struct mlx5_crypto_priv {
+ TAILQ_ENTRY(mlx5_crypto_priv) next;
+ struct ibv_context *ctx; /* Device context. */
+ struct rte_pci_device *pci_dev;
+ struct rte_cryptodev *crypto_dev;
+ void *uar; /* User Access Region. */
+ uint32_t pdn; /* Protection Domain number. */
+ struct ibv_pd *pd;
+ struct mlx5_hlist *dek_hlist; /* Dek hash list. */
+};
+
+struct mlx5_crypto_dek {
+ struct mlx5_list_entry entry; /* Pointer to DEK hash list entry. */
+ struct mlx5_devx_obj *obj; /* Pointer to DEK DevX object. */
+ uint8_t data[MLX5_CRYPTO_KEY_LENGTH]; /* DEK key data. */
+ bool size_is_48; /* Whether the key\data size is 48 bytes or not. */
+} __rte_cache_aligned;
+
+int
+mlx5_crypto_dek_destroy(struct mlx5_crypto_priv *priv,
+ struct mlx5_crypto_dek *dek);
+
+struct mlx5_crypto_dek *
+mlx5_crypto_dek_prepare(struct mlx5_crypto_priv *priv,
+ struct rte_crypto_cipher_xform *cipher);
+
+int
+mlx5_crypto_dek_setup(struct mlx5_crypto_priv *priv);
+
+void
+mlx5_crypto_dek_unset(struct mlx5_crypto_priv *priv);
+
+#endif /* MLX5_CRYPTO_H_ */
+
new file mode 100644
@@ -0,0 +1,161 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2021 NVIDIA Corporation & Affiliates
+ */
+
+#include <rte_ip.h>
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+
+#include <mlx5_prm.h>
+#include <mlx5_devx_cmds.h>
+
+#include "mlx5_crypto_utils.h"
+#include "mlx5_crypto.h"
+
+struct mlx5_crypto_dek_ctx {
+ struct rte_crypto_cipher_xform *cipher;
+ struct mlx5_crypto_priv *priv;
+};
+
+int
+mlx5_crypto_dek_destroy(struct mlx5_crypto_priv *priv,
+ struct mlx5_crypto_dek *dek)
+{
+ return mlx5_hlist_unregister(priv->dek_hlist, &dek->entry);
+}
+
+struct mlx5_crypto_dek *
+mlx5_crypto_dek_prepare(struct mlx5_crypto_priv *priv,
+ struct rte_crypto_cipher_xform *cipher)
+{
+ struct mlx5_hlist *dek_hlist = priv->dek_hlist;
+ struct mlx5_crypto_dek_ctx dek_ctx = {
+ .cipher = cipher,
+ .priv = priv,
+ };
+ struct rte_crypto_cipher_xform *cipher_ctx = cipher;
+ uint64_t key64 = __rte_raw_cksum(cipher_ctx->key.data,
+ cipher_ctx->key.length, 0);
+ struct mlx5_list_entry *entry = mlx5_hlist_register(dek_hlist,
+ key64, &dek_ctx);
+
+ return entry == NULL ? NULL :
+ container_of(entry, struct mlx5_crypto_dek, entry);
+}
+
+static struct mlx5_list_entry *
+mlx5_crypto_dek_clone_cb(void *tool_ctx __rte_unused,
+ struct mlx5_list_entry *oentry,
+ void *cb_ctx __rte_unused)
+{
+ struct mlx5_crypto_dek *entry = rte_zmalloc(__func__, sizeof(*entry),
+ RTE_CACHE_LINE_SIZE);
+
+ if (!entry) {
+ DRV_LOG(ERR, "Cannot allocate dek resource memory.");
+ rte_errno = ENOMEM;
+ return NULL;
+ }
+ memcpy(entry, oentry, sizeof(*entry));
+ return &entry->entry;
+}
+
+static void
+mlx5_crypto_dek_clone_free_cb(void *tool_ctx __rte_unused,
+ struct mlx5_list_entry *entry)
+{
+ struct mlx5_crypto_dek *dek = container_of(entry,
+ struct mlx5_crypto_dek, entry);
+
+ rte_free(dek);
+}
+
+static int
+mlx5_crypto_dek_match_cb(void *tool_ctx __rte_unused,
+ struct mlx5_list_entry *entry, void *cb_ctx)
+{
+ struct mlx5_crypto_dek_ctx *ctx = cb_ctx;
+ struct rte_crypto_cipher_xform *cipher_ctx = ctx->cipher;
+ struct mlx5_crypto_dek *dek =
+ container_of(entry, typeof(*dek), entry);
+ uint32_t key_len = dek->size_is_48 ? 48 : 80;
+
+ if (key_len != cipher_ctx->key.length)
+ return -1;
+ return memcmp(cipher_ctx->key.data, dek->data, key_len);
+}
+
+static struct mlx5_list_entry *
+mlx5_crypto_dek_create_cb(void *tool_ctx __rte_unused, void *cb_ctx)
+{
+ struct mlx5_crypto_dek_ctx *ctx = cb_ctx;
+ struct rte_crypto_cipher_xform *cipher_ctx = ctx->cipher;
+ struct mlx5_crypto_dek *dek = rte_zmalloc(__func__, sizeof(*dek),
+ RTE_CACHE_LINE_SIZE);
+ struct mlx5_devx_dek_attr dek_attr = {
+ .pd = ctx->priv->pdn,
+ .key_purpose = MLX5_CRYPTO_KEY_PURPOSE_AES_XTS,
+ .has_keytag = 1,
+ };
+
+ if (dek == NULL) {
+ DRV_LOG(ERR, "Failed to allocate dek memory.");
+ return NULL;
+ }
+ switch (cipher_ctx->key.length) {
+ case 48:
+ dek->size_is_48 = true;
+ dek_attr.key_size = MLX5_CRYPTO_KEY_SIZE_128b;
+ break;
+ case 80:
+ dek->size_is_48 = false;
+ dek_attr.key_size = MLX5_CRYPTO_KEY_SIZE_256b;
+ break;
+ default:
+ DRV_LOG(ERR, "Key size not supported.");
+ return NULL;
+ }
+ memcpy(&dek_attr.key, cipher_ctx->key.data, cipher_ctx->key.length);
+ dek->obj = mlx5_devx_cmd_create_dek_obj(ctx->priv->ctx, &dek_attr);
+ if (dek->obj == NULL) {
+ rte_free(dek);
+ return NULL;
+ }
+ memcpy(&dek->data, cipher_ctx->key.data, cipher_ctx->key.length);
+ return &dek->entry;
+}
+
+static void
+mlx5_crypto_dek_remove_cb(void *tool_ctx __rte_unused,
+ struct mlx5_list_entry *entry)
+{
+ struct mlx5_crypto_dek *dek =
+ container_of(entry, typeof(*dek), entry);
+
+ claim_zero(mlx5_devx_cmd_destroy(dek->obj));
+ rte_free(dek);
+}
+
+
+int
+mlx5_crypto_dek_setup(struct mlx5_crypto_priv *priv)
+{
+ priv->dek_hlist = mlx5_hlist_create("dek_hlist",
+ MLX5_CRYPTO_DEK_HTABLE_SZ,
+ 0, 1, NULL, mlx5_crypto_dek_create_cb,
+ mlx5_crypto_dek_match_cb,
+ mlx5_crypto_dek_remove_cb,
+ mlx5_crypto_dek_clone_cb,
+ mlx5_crypto_dek_clone_free_cb);
+ if (priv->dek_hlist == NULL)
+ return -1;
+ return 0;
+}
+
+void
+mlx5_crypto_dek_unset(struct mlx5_crypto_priv *priv)
+{
+ mlx5_hlist_destroy(priv->dek_hlist);
+ priv->dek_hlist = NULL;
+}