Re: [PATCH] [RFC] cryptodev: replace LIST_END enumerators with APIs

[fengchengwen <fengchengwen@huawei.com>]

On 2024/9/5 23:09, Morten Brørup wrote:
>> +++ b/app/test/test_cryptodev_asym.c
>> @@ -581,7 +581,7 @@ static inline void print_asym_capa(
>>  			rte_cryptodev_asym_get_xform_string(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_list_end(); i++) {
> 
>> +++ b/lib/cryptodev/rte_crypto_asym.h
>> +static inline int
>> +rte_crypto_asym_xform_type_list_end(void)
>> +{
>> +	return RTE_CRYPTO_ASYM_XFORM_SM2 + 1;
>> +}
>> +
>>  /**
>>   * Asymmetric crypto operation type variants
>> + * Note: Update rte_crypto_asym_op_list_end for every new type added.
>>   */
>>  enum rte_crypto_asym_op_type {
>>  	RTE_CRYPTO_ASYM_OP_ENCRYPT,
>> @@ -135,9 +141,14 @@ enum rte_crypto_asym_op_type {
>>  	/**< Signature Generation operation */
>>  	RTE_CRYPTO_ASYM_OP_VERIFY,
>>  	/**< Signature Verification operation */
>> -	RTE_CRYPTO_ASYM_OP_LIST_END
>>  };
>>
>> +static inline int
>> +rte_crypto_asym_op_list_end(void)
>> +{
>> +	return RTE_CRYPTO_ASYM_OP_VERIFY + 1;
>> +}
> 
> I like the concept of replacing an "last enum value" with a "last enum function" for API/ABI compatibility purposes.

+1
There are many such define in DPDK, e.g. RTE_ETH_EVENT_MAX

> 
> Here's an idea...
> 
> We can introduce a generic design pattern where we keep the _LIST_END enum value at the end, somehow marking it private (and not part of the API/ABI), and move the _list_end() function inside the C file, so it uses the _LIST_END enum value that the library was built with. E.g. like this:
> 
> 
> In the header file:
> 
> enum rte_crypto_asym_op_type {
> 	RTE_CRYPTO_ASYM_OP_VERIFY,
> 	/**< Signature Verification operation */
> #if RTE_BUILDING_INTERNAL
> 	__RTE_CRYPTO_ASYM_OP_LIST_END /* internal */
> #endif
> }
> 
> int rte_crypto_asym_op_list_end(void);
> 
> 
> And in the associated library code file, when including rte_crypto_asym.h:
> 
> #define RTE_BUILDING_INTERNAL
> #include <cryptodev/rte_crypto_asym.h>
> 
> int
> rte_crypto_asym_op_list_end(void)
> {
> 	return __RTE_CRYPTO_ASYM_OP_LIST_END;
> }

It's more generic, and also keep LIST_END in the define, we just add new enum before it.
But based on my understanding of ABI compatibility, from the point view of application,
this API should return old-value even with the new library, but it will return new-value
with new library. It could also break ABI.

So this API should force inline, just as this patch did. But it seem can't work if move
this API to header file and add static inline.

> 
> .
>