Re: [dpdk-dev] [PATCH v2 1/3] cryptodev: remove LIST_END enumerators

["Kinsella, Ray" <mdr@ashroe.eu>]

On 08/10/2021 21:45, Akhil Goyal wrote:
> Remove *_LIST_END enumerators from asymmetric crypto
> lib to avoid ABI breakage for every new addition in
> enums.
> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> ---
> v2: no change
> 
>  app/test/test_cryptodev_asym.c  | 4 ++--
>  drivers/crypto/qat/qat_asym.c   | 2 +-
>  lib/cryptodev/rte_crypto_asym.h | 4 ----
>  3 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
> index 9d19a6d6d9..603b2e4609 100644
> --- a/app/test/test_cryptodev_asym.c
> +++ b/app/test/test_cryptodev_asym.c
> @@ -541,7 +541,7 @@ test_one_case(const void *test_case, int sessionless)
>  		printf("  %u) TestCase %s %s\n", test_index++,
>  			tc.modex.description, test_msg);
>  	} else {
> -		for (i = 0; i < RTE_CRYPTO_ASYM_OP_LIST_END; i++) {
> +		for (i = 0; i <= RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE; i++) {
>  			if (tc.modex.xform_type == RTE_CRYPTO_ASYM_XFORM_RSA) {
>  				if (tc.rsa_data.op_type_flags & (1 << i)) {
>  					if (tc.rsa_data.key_exp) {
> @@ -1027,7 +1027,7 @@ static inline void print_asym_capa(
>  			rte_crypto_asym_xform_strings[capa->xform_type]);
>  	printf("operation supported -");
>  
> -	for (i = 0; i < RTE_CRYPTO_ASYM_OP_LIST_END; i++) {
> +	for (i = 0; i <= RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE; i++) {
>  		/* check supported operations */
>  		if (rte_cryptodev_asym_xform_capability_check_optype(capa, i))
>  			printf(" %s",
> diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c
> index 85973812a8..026625a4d2 100644
> --- a/drivers/crypto/qat/qat_asym.c
> +++ b/drivers/crypto/qat/qat_asym.c
> @@ -742,7 +742,7 @@ qat_asym_session_configure(struct rte_cryptodev *dev,
>  			err = -EINVAL;
>  			goto error;
>  		}
> -	} else if (xform->xform_type >= RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> +	} else if (xform->xform_type > RTE_CRYPTO_ASYM_XFORM_ECPM
>  			|| xform->xform_type <= RTE_CRYPTO_ASYM_XFORM_NONE) {
>  		QAT_LOG(ERR, "Invalid asymmetric crypto xform");
>  		err = -EINVAL;
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index 9c866f553f..5edf658572 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -94,8 +94,6 @@ enum rte_crypto_asym_xform_type {
>  	 */
>  	RTE_CRYPTO_ASYM_XFORM_ECPM,
>  	/**< Elliptic Curve Point Multiplication */
> -	RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> -	/**< End of list */
>  };
>  
>  /**
> @@ -116,7 +114,6 @@ enum rte_crypto_asym_op_type {
>  	/**< DH Public Key generation operation */
>  	RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
>  	/**< DH Shared Secret compute operation */
> -	RTE_CRYPTO_ASYM_OP_LIST_END
>  };
>  
>  /**
> @@ -133,7 +130,6 @@ enum rte_crypto_rsa_padding_type {
>  	/**< RSA PKCS#1 OAEP padding scheme */
>  	RTE_CRYPTO_RSA_PADDING_PSS,
>  	/**< RSA PKCS#1 PSS padding scheme */
> -	RTE_CRYPTO_RSA_PADDING_TYPE_LIST_END
>  };
>  
>  /**

So I am not sure that this is an improvement.
The cryptodev issue we had, was that _LIST_END was being used to size arrays. 
And that broke when new algorithms got added. Is that an issue, in this case?

I am not sure that swapping out _LIST_END, and then littering the code with
RTE_CRYPTO_ASYM_XFORM_ECPM and RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE, is an improvement here.

My 2c is that from an ABI PoV RTE_CRYPTO_ASYM_OP_LIST_END is not better or worse,
than RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE?

Interested to hear other thoughts.