[v3,09/15] crypto/mlx5: adjust to the multiple data unit API
Checks
Commit Message
From: Shiri Kuzin <shirik@nvidia.com>
In AES-XTS the data to be encrypted\decrypted does not have to be
in multiples of 16B size, the unit of data is called data-unit.
As a result of patch [1] a new field is added to the cipher capability,
called dataunit_set, where the devices can report the range of
supported data-unit sizes.
The new field enables saving the data-unit size in the session
structure to the block size pointer variable in order to support
several data-unit sizes.
[1] https://www.mail-archive.com/dev@dpdk.org/msg205337.html
Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
---
drivers/crypto/mlx5/mlx5_crypto.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
Comments
> From: Shiri Kuzin <shirik@nvidia.com>
>
> In AES-XTS the data to be encrypted\decrypted does not have to be
> in multiples of 16B size, the unit of data is called data-unit.
>
> As a result of patch [1] a new field is added to the cipher capability,
> called dataunit_set, where the devices can report the range of
> supported data-unit sizes.
>
> The new field enables saving the data-unit size in the session
> structure to the block size pointer variable in order to support
> several data-unit sizes.
>
> [1] https://www.mail-archive.com/dev@dpdk.org/msg205337.html
>
Since [1] patch is already merged, no need to refer it in the patch description.
Title can be rephrased as
Crypto/mlx5: support multiple data units
Also set feature flag in the code and the documentation in this patch.
I see that you are setting all of them in a single patch in the end.
This is not correct. It should be added where the feature is supported.
Please fix this for all the feature flags.
> Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> ---
> drivers/crypto/mlx5/mlx5_crypto.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> b/drivers/crypto/mlx5/mlx5_crypto.c
> index 18b1a6be88..8cc29ced21 100644
> --- a/drivers/crypto/mlx5/mlx5_crypto.c
> +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> @@ -48,6 +48,11 @@ struct mlx5_crypto_session {
> * bsf_size, bsf_p_type, encryption_order and encryption standard,
> * saved in big endian format.
> */
> + uint32_t bsp_res;
> + /*
> + * crypto_block_size_pointer and reserved 24 bits saved in big endian
> + * format.
> + */
> uint32_t iv_offset:16;
> /* Starting point for Initialisation Vector. */
> struct mlx5_crypto_dek *dek; /* Pointer to dek struct. */
> @@ -171,6 +176,24 @@ mlx5_crypto_sym_session_configure(struct
> rte_cryptodev *dev,
> MLX5_BSF_P_TYPE_CRYPTO <<
> MLX5_BSF_P_TYPE_OFFSET |
> encryption_order <<
> MLX5_ENCRYPTION_ORDER_OFFSET |
> MLX5_ENCRYPTION_STANDARD_AES_XTS);
> + switch (xform->cipher.dataunit_len) {
> + case 0:
> + sess_private_data->bsp_res = 0;
> + break;
> + case 512:
> + sess_private_data->bsp_res = rte_cpu_to_be_32
> +
> ((uint32_t)MLX5_BLOCK_SIZE_512B <<
> + MLX5_BLOCK_SIZE_OFFSET);
> + break;
> + case 4096:
> + sess_private_data->bsp_res = rte_cpu_to_be_32
> +
> ((uint32_t)MLX5_BLOCK_SIZE_4096B <<
> + MLX5_BLOCK_SIZE_OFFSET);
> + break;
> + default:
> + DRV_LOG(ERR, "Cipher data unit length is not supported.");
> + return -ENOTSUP;
> + }
> sess_private_data->iv_offset = cipher->iv.offset;
> sess_private_data->dek_id =
> rte_cpu_to_be_32(sess_private_data->dek->obj->id
> &
> --
> 2.25.1
From: Akhil Goyal
> > From: Shiri Kuzin <shirik@nvidia.com>
> >
> > In AES-XTS the data to be encrypted\decrypted does not have to be in
> > multiples of 16B size, the unit of data is called data-unit.
> >
> > As a result of patch [1] a new field is added to the cipher
> > capability, called dataunit_set, where the devices can report the
> > range of supported data-unit sizes.
> >
> > The new field enables saving the data-unit size in the session
> > structure to the block size pointer variable in order to support
> > several data-unit sizes.
> >
> > [1] https://www.mail-archive.com/dev@dpdk.org/msg205337.html
> >
> Since [1] patch is already merged, no need to refer it in the patch description.
> Title can be rephrased as
> Crypto/mlx5: support multiple data units
Ok.
> Also set feature flag in the code and the documentation in this patch.
> I see that you are setting all of them in a single patch in the end.
> This is not correct. It should be added where the feature is supported.
> Please fix this for all the feature flags.
No, in this stage no feature is really supported, the actual time it will be supported is after the datapath patches and capabilities set.
>
> > Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> > ---
> > drivers/crypto/mlx5/mlx5_crypto.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/crypto/mlx5/mlx5_crypto.c
> > b/drivers/crypto/mlx5/mlx5_crypto.c
> > index 18b1a6be88..8cc29ced21 100644
> > --- a/drivers/crypto/mlx5/mlx5_crypto.c
> > +++ b/drivers/crypto/mlx5/mlx5_crypto.c
> > @@ -48,6 +48,11 @@ struct mlx5_crypto_session {
> > * bsf_size, bsf_p_type, encryption_order and encryption standard,
> > * saved in big endian format.
> > */
> > + uint32_t bsp_res;
> > + /*
> > + * crypto_block_size_pointer and reserved 24 bits saved in big endian
> > + * format.
> > + */
> > uint32_t iv_offset:16;
> > /* Starting point for Initialisation Vector. */
> > struct mlx5_crypto_dek *dek; /* Pointer to dek struct. */ @@
> > -171,6 +176,24 @@ mlx5_crypto_sym_session_configure(struct
> > rte_cryptodev *dev,
> > MLX5_BSF_P_TYPE_CRYPTO <<
> > MLX5_BSF_P_TYPE_OFFSET |
> > encryption_order <<
> > MLX5_ENCRYPTION_ORDER_OFFSET |
> > MLX5_ENCRYPTION_STANDARD_AES_XTS);
> > + switch (xform->cipher.dataunit_len) {
> > + case 0:
> > + sess_private_data->bsp_res = 0;
> > + break;
> > + case 512:
> > + sess_private_data->bsp_res = rte_cpu_to_be_32
> > +
> > ((uint32_t)MLX5_BLOCK_SIZE_512B <<
> > + MLX5_BLOCK_SIZE_OFFSET);
> > + break;
> > + case 4096:
> > + sess_private_data->bsp_res = rte_cpu_to_be_32
> > +
> > ((uint32_t)MLX5_BLOCK_SIZE_4096B <<
> > + MLX5_BLOCK_SIZE_OFFSET);
> > + break;
> > + default:
> > + DRV_LOG(ERR, "Cipher data unit length is not supported.");
> > + return -ENOTSUP;
> > + }
> > sess_private_data->iv_offset = cipher->iv.offset;
> > sess_private_data->dek_id =
> > rte_cpu_to_be_32(sess_private_data->dek->obj->id
> > &
> > --
> > 2.25.1
>
> > Also set feature flag in the code and the documentation in this patch.
> > I see that you are setting all of them in a single patch in the end.
> > This is not correct. It should be added where the feature is supported.
> > Please fix this for all the feature flags.
>
> No, in this stage no feature is really supported, the actual time it will be
> supported is after the datapath patches and capabilities set.
>
Move the patch after adding data path, but documentation should be part
Of this patch.
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Sunday, May 9, 2021 12:19 PM
> To: Matan Azrad <matan@nvidia.com>; dev@dpdk.org
> Cc: Suanming Mou <suanmingm@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Shiri Kuzin <shirik@nvidia.com>
> Subject: RE: [EXT] [PATCH v3 09/15] crypto/mlx5: adjust to the multiple data
> unit API
>
> External email: Use caution opening links or attachments
>
>
> >
> > > Also set feature flag in the code and the documentation in this patch.
> > > I see that you are setting all of them in a single patch in the end.
> > > This is not correct. It should be added where the feature is supported.
> > > Please fix this for all the feature flags.
> >
> > No, in this stage no feature is really supported, the actual time it
> > will be supported is after the datapath patches and capabilities set.
> >
> Move the patch after adding data path, but documentation should be part Of
> this patch.
I will squash this patch to the session commit.
> > >
> > > > Also set feature flag in the code and the documentation in this patch.
> > > > I see that you are setting all of them in a single patch in the end.
> > > > This is not correct. It should be added where the feature is supported.
> > > > Please fix this for all the feature flags.
> > >
> > > No, in this stage no feature is really supported, the actual time it
> > > will be supported is after the datapath patches and capabilities set.
> > >
> > Move the patch after adding data path, but documentation should be part
> Of
> > this patch.
>
> I will squash this patch to the session commit.
I am not asking you to squash the patch to the session commit.
The point is documentation patch should be part of code patch which
Introduced that feature. This methodology has been discussed in the past
and is being followed now. Please discuss this in techboard in case of deviation.
Regards,
Akhil
From: Akhil Goyal
> > > > > Also set feature flag in the code and the documentation in this patch.
> > > > > I see that you are setting all of them in a single patch in the end.
> > > > > This is not correct. It should be added where the feature is supported.
> > > > > Please fix this for all the feature flags.
> > > >
> > > > No, in this stage no feature is really supported, the actual time
> > > > it will be supported is after the datapath patches and capabilities set.
> > > >
> > > Move the patch after adding data path, but documentation should be
> > > part
> > Of
> > > this patch.
> >
> > I will squash this patch to the session commit.
>
> I am not asking you to squash the patch to the session commit.
> The point is documentation patch should be part of code patch which
> Introduced that feature. This methodology has been discussed in the past
> and is being followed now. Please discuss this in techboard in case of
> deviation.
Not sure this is mandatory when you add a new PMD.
For sure you right in general.
Anyway, the session commit is the better place to this code.
> Regards,
> Akhil
> From: Akhil Goyal
> > > > > > Also set feature flag in the code and the documentation in this
> patch.
> > > > > > I see that you are setting all of them in a single patch in the end.
> > > > > > This is not correct. It should be added where the feature is
> supported.
> > > > > > Please fix this for all the feature flags.
> > > > >
> > > > > No, in this stage no feature is really supported, the actual time
> > > > > it will be supported is after the datapath patches and capabilities set.
> > > > >
> > > > Move the patch after adding data path, but documentation should be
> > > > part
> > > Of
> > > > this patch.
> > >
> > > I will squash this patch to the session commit.
> >
> > I am not asking you to squash the patch to the session commit.
> > The point is documentation patch should be part of code patch which
> > Introduced that feature. This methodology has been discussed in the past
> > and is being followed now. Please discuss this in techboard in case of
> > deviation.
>
> Not sure this is mandatory when you add a new PMD.
In case of a new PMD also we try to follow this as much as we can.
IMO, this can be done here.
> For sure you right in general.
>
> Anyway, the session commit is the better place to this code.
>
> > Regards,
> > Akhil
@@ -48,6 +48,11 @@ struct mlx5_crypto_session {
* bsf_size, bsf_p_type, encryption_order and encryption standard,
* saved in big endian format.
*/
+ uint32_t bsp_res;
+ /*
+ * crypto_block_size_pointer and reserved 24 bits saved in big endian
+ * format.
+ */
uint32_t iv_offset:16;
/* Starting point for Initialisation Vector. */
struct mlx5_crypto_dek *dek; /* Pointer to dek struct. */
@@ -171,6 +176,24 @@ mlx5_crypto_sym_session_configure(struct rte_cryptodev *dev,
MLX5_BSF_P_TYPE_CRYPTO << MLX5_BSF_P_TYPE_OFFSET |
encryption_order << MLX5_ENCRYPTION_ORDER_OFFSET |
MLX5_ENCRYPTION_STANDARD_AES_XTS);
+ switch (xform->cipher.dataunit_len) {
+ case 0:
+ sess_private_data->bsp_res = 0;
+ break;
+ case 512:
+ sess_private_data->bsp_res = rte_cpu_to_be_32
+ ((uint32_t)MLX5_BLOCK_SIZE_512B <<
+ MLX5_BLOCK_SIZE_OFFSET);
+ break;
+ case 4096:
+ sess_private_data->bsp_res = rte_cpu_to_be_32
+ ((uint32_t)MLX5_BLOCK_SIZE_4096B <<
+ MLX5_BLOCK_SIZE_OFFSET);
+ break;
+ default:
+ DRV_LOG(ERR, "Cipher data unit length is not supported.");
+ return -ENOTSUP;
+ }
sess_private_data->iv_offset = cipher->iv.offset;
sess_private_data->dek_id =
rte_cpu_to_be_32(sess_private_data->dek->obj->id &