Re: [dpdk-dev] [PATCH v2 6/8] crypto/bcmfs: add session handling and capabilities

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Vikas,

> diff --git a/doc/guides/cryptodevs/features/bcmfs.ini
> b/doc/guides/cryptodevs/features/bcmfs.ini
> new file mode 100644
> index 000000000..82d2c639d
> --- /dev/null
> +++ b/doc/guides/cryptodevs/features/bcmfs.ini
> @@ -0,0 +1,56 @@
> +;
> +; Supported features of the 'bcmfs' crypto driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Symmetric crypto       = Y
> +Sym operation chaining = Y
> +HW Accelerated         = Y
> +Protocol offload       = Y
> +In Place SGL           = Y
> +
> +;
> +; Supported crypto algorithms of the 'bcmfs' crypto driver.
> +;
> +[Cipher]
> +AES CBC (128)  = Y
> +AES CBC (192)  = Y
> +AES CBC (256)  = Y
> +AES CTR (128)  = Y
> +AES CTR (192)  = Y
> +AES CTR (256)  = Y
> +AES XTS (128)  = Y
> +AES XTS (256)  = Y
> +3DES CBC       = Y
> +DES CBC        = Y
> +;
> +; Supported authentication algorithms of the 'bcmfs' crypto driver.
> +;
> +[Auth]
> +MD5 HMAC     = Y
> +SHA1         = Y
> +SHA1 HMAC    = Y
> +SHA224       = Y
> +SHA224 HMAC  = Y
> +SHA256       = Y
> +SHA256 HMAC  = Y
> +SHA384       = Y
> +SHA384 HMAC  = Y
> +SHA512       = Y
> +SHA512 HMAC  = Y
> +AES GMAC     = Y
> +AES CMAC (128) = Y
> +AES CBC = Y

AES CBC is not an auth algo
You should use AES CBC MAC
Please use the same notation as there in default.ini
Check for all the names.

> +AES XCBC = Y
> +
> +;
> +; Supported AEAD algorithms of the 'bcmfs' crypto driver.
> +;
> +[AEAD]
> +AES GCM (128)     = Y
> +AES GCM (192)     = Y
> +AES GCM (256)     = Y
> +AES CCM (128)     = Y
> +AES CCM (192)     = Y
> +AES CCM (256)     = Y

// snip//

> +	{
> +		/* SHA1 HMAC */
> +		.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
> +		{.sym = {
> +			.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
> +			{.auth = {
> +				.algo = RTE_CRYPTO_AUTH_SHA1_HMAC,
> +				.block_size = 64,
> +				.key_size = {
> +					.min = 1,
> +					.max = 64,
> +					.increment = 0

Increment should be 1 for all HMAC cases.

> +				},
> +				.digest_size = {
> +					.min = 20,
> +					.max = 20,
> +					.increment = 0
> +				},
> +				.aad_size = { 0 }
> +			}, }
> +		}, }
> +	},

//snipp//

> +	{
> +		/* AES CMAC */
> +		.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
> +		{.sym = {
> +			.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
> +			{.auth = {
> +				.algo = RTE_CRYPTO_AUTH_AES_CMAC,
> +				.block_size = 16,
> +				.key_size = {
> +					.min = 1,
> +					.max = 16,
> +					.increment = 0

Do you only support key sizes of 1 and 16? I see increment =0 in many cases.

> +				},
> +				.digest_size = {
> +					.min = 16,
> +					.max = 16,
> +					.increment = 0
> +				},
> +				.aad_size = { 0 }
> +			}, }
> +		}, }
> +	},
> +	{

//snip//


> +
> +const struct rte_cryptodev_capabilities *
> +bcmfs_sym_get_capabilities(void)
> +{
> +	return bcmfs_sym_capabilities;
> +}
> diff --git a/drivers/crypto/bcmfs/bcmfs_sym_capabilities.h
> b/drivers/crypto/bcmfs/bcmfs_sym_capabilities.h
> new file mode 100644
> index 000000000..3ff61b7d2
> --- /dev/null
> +++ b/drivers/crypto/bcmfs/bcmfs_sym_capabilities.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Broadcom
> + * All rights reserved.
> + */
> +
> +#ifndef _BCMFS_SYM_CAPABILITIES_H_
> +#define _BCMFS_SYM_CAPABILITIES_H_
> +
> +/*
> + * Get capabilities list for the device
> + *
> + */
> +const struct rte_cryptodev_capabilities *bcmfs_sym_get_capabilities(void);
> +
> +#endif /* _BCMFS_SYM_CAPABILITIES_H__ */
> +
> diff --git a/drivers/crypto/bcmfs/bcmfs_sym_defs.h
> b/drivers/crypto/bcmfs/bcmfs_sym_defs.h
> new file mode 100644
> index 000000000..d94446d35
> --- /dev/null
> +++ b/drivers/crypto/bcmfs/bcmfs_sym_defs.h
> @@ -0,0 +1,170 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Broadcom
> + * All rights reserved.
> + */
> +
> +#ifndef _BCMFS_SYM_DEFS_H_
> +#define _BCMFS_SYM_DEFS_H_
> +
> +/*
> + * Max block size of hash algorithm
> + * currently SHA3 supports max block size
> + * of 144 bytes
> + */
> +#define BCMFS_MAX_KEY_SIZE	144
> +#define BCMFS_MAX_IV_SIZE	16
> +#define BCMFS_MAX_DIGEST_SIZE	64
> +
> +/** Symmetric Cipher Direction */
> +enum bcmfs_crypto_cipher_op {
> +	/** Encrypt cipher operation */
> +	BCMFS_CRYPTO_CIPHER_OP_ENCRYPT,
> +
> +	/** Decrypt cipher operation */
> +	BCMFS_CRYPTO_CIPHER_OP_DECRYPT,
> +};
> +

Why are these enums needed, Aren't these replica of rte_sym_crypto.h

Are these enum values getting filled in some HW desc/registers. If so, then
Probably move it to the hw folder.

> +/** Symmetric Cipher Algorithms */
> +enum bcmfs_crypto_cipher_algorithm {
> +	/** NULL cipher algorithm. No mode applies to the NULL algorithm. */
> +	BCMFS_CRYPTO_CIPHER_NONE = 0,
> +
> +	/** Triple DES algorithm in CBC mode */
> +	BCMFS_CRYPTO_CIPHER_DES_CBC,
> +
> +	/** Triple DES algorithm in ECB mode */
> +	BCMFS_CRYPTO_CIPHER_DES_ECB,
> +
> +	/** Triple DES algorithm in CBC mode */
> +	BCMFS_CRYPTO_CIPHER_3DES_CBC,
> +
> +	/** Triple DES algorithm in ECB mode */
> +	BCMFS_CRYPTO_CIPHER_3DES_ECB,
> +
> +	/** AES algorithm in CBC mode */
> +	BCMFS_CRYPTO_CIPHER_AES_CBC,
> +
> +	/** AES algorithm in CCM mode. */
> +	BCMFS_CRYPTO_CIPHER_AES_CCM,
> +
> +	/** AES algorithm in Counter mode */
> +	BCMFS_CRYPTO_CIPHER_AES_CTR,
> +
> +	/** AES algorithm in ECB mode */
> +	BCMFS_CRYPTO_CIPHER_AES_ECB,
> +
> +	/** AES algorithm in GCM mode. */
> +	BCMFS_CRYPTO_CIPHER_AES_GCM,
> +
> +	/** AES algorithm in XTS mode */
> +	BCMFS_CRYPTO_CIPHER_AES_XTS,
> +
> +	/** AES algorithm in OFB mode */
> +	BCMFS_CRYPTO_CIPHER_AES_OFB,
> +};
> +