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

["Morten Brørup" <mb@smartsharesystems.com>]

> From: fengchengwen [mailto:fengchengwen@huawei.com]
> Sent: Friday, 6 September 2024 08.33
> 
> 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.

Maybe a combination, returning the lowest end of the two versions of the list, would work...

----------------------------------
Common header file (rte_common.h):
----------------------------------

/* Add at end of enum list in the header file. */
#define RTE_ENUM_LIST_END(name) \
_ # name # _ENUM_LIST_END /**< @internal */

/* Add somewhere in header file, preferably after the enum list. */
#define rte_declare_enum_list_end(name) \
/** @internal */ \
int _# name # _enum_list_end(void); \
\
static int name # _enum_list_end(void) \
{ \
	static int cached = 0; \
\
	if (likely(cached != 0)) \
		return cached; \
\
	return cached = RTE_MIN( \
			RTE_ENUM_LIST_END(name), \
			_ # name # _enum_list_end()); \
} \
\
int _# name # _enum_list_end(void)

/* Add in the library/driver implementation. */
#define rte_define_enum_list_end(name) \
int _# name # _enum_list_end(void) \
{ \
	return RTE_ENUM_LIST_END(name); \
} \
\
int _# name # _enum_list_end(void)

--------------------
Library header file:
--------------------

enum rte_crypto_asym_op_type {
	RTE_CRYPTO_ASYM_OP_VERIFY,
	/**< Signature Verification operation */
	RTE_ENUM_LIST_END(rte_crypto_asym_op)
}

rte_declare_enum_list_end(rte_crypto_asym_op);

---------------
Library C file:
---------------

rte_define_enum_list_end(rte_crypto_asym_op);