> From: fengchengwen [mailto:fengcheng...@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);
