[dpdk-dev] [EXT] [PATCH v8 02/16] crypto/mlx5: add DEK object management

Akhil Goyal gakhil at marvell.com
Tue Jul 20 10:55:34 CEST 2021


> > > > > 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 at nvidia.com>
> > > > > Acked-by: Matan Azrad <matan at 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.




More information about the dev mailing list