Akhil Goyal <gak...@marvell.com> writes: >> >>> Now added inline APIs for getting the list end which need to be updated >> >>> for each new entry to the enum. This shall help in avoiding ABI break >> >>> for adding new algo. >> >>> >> >> >> >> Hi Akhil, >> >> >> >> *I think* this hides the problem instead of fixing it, and this may be >> >> partially because of the tooling (libabigail) limitation. >> >> >> >> This patch prevents the libabigail warning, true, but it doesn't improve >> >> anything from the application's perspective. >> >> Before or after this patch, application knows a fixed value as END value. >> >> >> >> Not all changes in the END (MAX) enum values cause ABI break, but tool >> >> warns on all, that is why I think this may be tooling limitation [1]. >> >> (Just to stress, I am NOT talking about regular enum values change, I am >> >> talking about only END (MAX) value changes caused by appending new enum >> >> items.) >> >> >> >> As far as I can see (please Dodji, David correct me if I am wrong) ABI >> >> break only happens if application and library exchange enum values in >> >> the API (directly or within a struct). >> > >> > - There is also the following issue: >> > A library publicly exports an array sized against a END (MAX) enum in the >> > API. >> > https://developers.redhat.com/blog/2019/05/06/how-c-array-sizes-become-part-of-the-binary-interface-of-a-library >> > >> >> I see. And Dodji explained this requires source code to detect. >> >> I don't remember seeing a public array whose size is defined by an enum, >> are you aware of any instance of this usage? > > https://patches.dpdk.org/project/dpdk/patch/20241009071151.1106-1-gmuthukri...@marvell.com/ > This was merged yesterday.
I guess the problematic piece of the code is this: diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h index bec947f6d5..aa6ef3a94d 100644 --- a/lib/cryptodev/rte_cryptodev.h +++ b/lib/cryptodev/rte_cryptodev.h @@ -185,6 +185,9 @@ struct rte_cryptodev_asymmetric_xform_capability { * Value 0 means unavailable, and application should pass the required * random value. Otherwise, PMD would internally compute the random number. */ + + uint32_t op_capa[RTE_CRYPTO_ASYM_OP_LIST_END]; + /**< Operation specific capabilities. */ }; Is it possible for the struct rte_cryptodev_asymmetric_xform_capability to be made an opaque struct which definition would be present only in a .c file of the library? Its data members would then be retrieved by getter functions that take a pointer to that struct in parameter. That way, the uint32_t op_capa[RTE_CRYPTO_ASYM_OP_LIST_END] data member would be "private" to the .c file and thus would not be part of the ABI. Any change to the RTE_CRYPTO_ASYM_OP enum would then become harmless to that struct. I hope this helps. -- Dodji