Re: [dpdk-dev] [PATCH] cryptodev: support multiple cipher block sizes

["Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>]

Hi Matan,

Few comments/questions inline with [Arek]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Matan Azrad
> Sent: Thursday, February 4, 2021 3:34 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Doherty, Declan <declan.doherty@intel.com>;
> Somalapuram Amaranath <asomalap@amd.com>; Ruifeng Wang
> <ruifeng.wang@arm.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>;
> Anoob Joseph <anoobj@marvell.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Griffin, John <john.griffin@intel.com>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Michael Shamis
> <michaelsh@marvell.com>; Nagadheeraj Rottela
> <rnagadheeraj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>;
> Gagandeep Singh <g.singh@nxp.com>; Jay Zhou <jianjay.zhou@huawei.com>
> Subject: [dpdk-dev] [PATCH] cryptodev: support multiple cipher block sizes
> 
> In cryptography, a block cipher is a deterministic algorithm operating on fixed-
> length groups of bits, called blocks.
> 
> A block cipher consists of two paired algorithms, one for encryption and the
> other for decryption. Both algorithms accept two inputs:
> an input block of size n bits and a key of size k bits; and both yield an n-bit output
> block. The decryption algorithm is defined to be the inverse function of the
> encryption.
> 
> Some cipher algorithms support multiple block sizes, e.g. AES-XTS supports any
> block size in range [16B, 2^24B], in this case, A plain-text data, divided into N
> amount of n-bits blocks, which is encrypted to the same data size, cipher-text,
> must be decrypted in the same division of N amount of n-bits blocks in order to
> get the same plain-text data.
[Arek] - Except that the last data block does not need to be n-bit long, beside that and lack of chaining it makes XTS no different to any other block cipher mode of operation.
Block size itself for XTS-AES is always 16 bytes in the first place which is AES constraint.
2^20 * 16B -> 2^24B constraint from IEEE 1619-2017, SP800-38E is data unit length that contains "data unit in bytes/ 16" AES blocks where last one can be incomplete.

> 
> The current cryptodev API doesn't allow the user to select a specific block size
> supported by the devices In addition, there is no definition how the IV is
> detected per block when single operation includes more than one block.

[Arek] - Do you mean tweak increment per data unit? Like one op as a data stream (multiple data units) and tweak incremented by pmd?

> 
> That causes applications to use single operation per block even though all the
> data is continuous in memory what reduces datapath performance.
> 
> Add a new feature flag to support multiple block sizes, called
> RTE_CRYPTODEV_FF_CIPHER_MULITPLE_BLOCKS.
> Add a new field in cipher capability, called bsf - block size flags, where the
> devices can report the range of the supported block sizes.
> Add a new cipher transformation field, called block_size, where the user can
> select one block size from the supported range.
> 
> All the new fields do not change the size of their structures.
> 
> Using flags to report the supported block sizes capability allows the devices to
> report a range simply as same as the user to read it simply.
> Also, thus sizes are usually common and probably will be shared between the
> devices.
> 
> Signed-off-by: Matan Azrad <matan@nvidia.com>
> ---
>  lib/librte_cryptodev/rte_crypto_sym.h | 12 ++++++++++++
> lib/librte_cryptodev/rte_cryptodev.h  | 23 ++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_sym.h
> b/lib/librte_cryptodev/rte_crypto_sym.h
> index 9d572ec..9a1215d 100644
> --- a/lib/librte_cryptodev/rte_crypto_sym.h
> +++ b/lib/librte_cryptodev/rte_crypto_sym.h
> @@ -265,6 +265,18 @@ struct rte_crypto_cipher_xform {
>  		 * which can be in the range 7 to 13 inclusive.
>  		 */
>  	} iv;	/**< Initialisation vector parameters */
> +
> +	uint32_t block_size;
> +	/**< When RTE_CRYPTODEV_FF_CIPHER_MULITPLE_BLOCKS is
> reported, this is
> +	 * the block size of the algorithm, otherwise or when the value is 0,
> +	 * use the default block size provided in the capability.
> +	 * The value should be in the range defined by the bsf field in the
> +	 * cipher capability.

[Arek] - nowadays algorithms rather don't have different block sizes, though I see people set this field even for stream ciphers.
If such algorithm would happen it probably could just get a suffix in crypto_cipher_enum. Otherwise some fixed size array could be added.

> +	 *
> +	 * - For AES-XTS it is the size of data-unit, from IEEE Std 1619-2007.
> +	 * For-each data-unit in the operation, the tweak(IV) value is
> +	 * assigned consecutively starting from the operation assigned tweak.
> +	 */
[Arek] - if data unit would be session value (key scope in xts naming) where the number of units would be taken from, sym_op->len ?
(For standard storage example: data unit size -> logical block size, sym_op->len -> range of consecutive logical blocks.)
If so it probably could be session-less op as this cipher key would be unusable after it.

>  };
> 
>  /** Symmetric Authentication / Hash Algorithms diff --git
> a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
> index ae34f33..60ba839 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -96,6 +96,19 @@ struct rte_crypto_param_range {  };
> 
>  /**
> + * Crypto device supported block size flags for cipher algorithms
> + * Each flag represents single or range of supported block sizes  */
> +#define RTE_CRYPTO_CIPHER_BSF_ALL 0x1
> +/* All the sizes from the algorithm standard */ #define
> +RTE_CRYPTO_CIPHER_BSF_512_BYTES 0x2 #define
> +RTE_CRYPTO_CIPHER_BSF_520_BYTES 0x4 #define
> +RTE_CRYPTO_CIPHER_BSF_4048_BYTES 0x8 #define
> +RTE_CRYPTO_CIPHER_BSF_4096_BYTES 0x10 #define
> +RTE_CRYPTO_CIPHER_BSF_4160_BYTES 0x20 #define
> +RTE_CRYPTO_CIPHER_BSF_1M_BYTES 0x40
[Arek] - when adding constants source should be attached as well.
> +
> +/**
>   * Symmetric Crypto Capability
>   */
>  struct rte_cryptodev_symmetric_capability { @@ -122,11 +135,19 @@ struct
> rte_cryptodev_symmetric_capability {
>  			enum rte_crypto_cipher_algorithm algo;
>  			/**< cipher algorithm */
>  			uint16_t block_size;
> -			/**< algorithm block size */
> +			/**<
> +			 * algorithm block size
> +			 * For algorithms support more than single block size,
> +			 * this is the default block size supported by the
> +			 * driver, all the supported sizes are reflected in the
> +			 * bsf field.
> +			 */
>  			struct rte_crypto_param_range key_size;
>  			/**< cipher key size range */
>  			struct rte_crypto_param_range iv_size;
>  			/**< Initialisation vector data size range */
> +			uint32_t bsf;
> +			/**< Block size flags */
>  		} cipher;
>  		/**< Symmetric Cipher transform capabilities */
>  		struct {
> --
> 1.8.3.1