On 10/4/2024 8:04 AM, David Marchand wrote: > On Fri, Oct 4, 2024 at 5:55 AM Ferruh Yigit <ferruh.yi...@amd.com> wrote: >> >> On 9/5/2024 11:14 AM, Akhil Goyal wrote: >>> Replace *_LIST_END enumerators from asymmetric crypto >>> lib to avoid ABI breakage for every new addition in >>> enums with inline APIs. >>> >>> Signed-off-by: Akhil Goyal <gak...@marvell.com> >>> --- >>> This patch was discussed in ML long time back. >>> https://patches.dpdk.org/project/dpdk/patch/20211008204516.3497060-1-gak...@marvell.com/ >>> 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? > I had made comments for an issue in the cryptodev library in the past: > https://inbox.dpdk.org/dev/cajfav8xs5cvde2xwrtaxk5ve_piqmv5ly5tkstk3r1gourt...@mail.gmail.com/ > > > - Sorry to deviate from the _END enum discussion that tries to define > a solution for all cases, but all I see in the cryptodev patch is a > need for an enumerator... for an internal unit test. > From the RFC patch, I would simply change the > rte_crypto_asym_xform_type_list_end helper into a non inline symbol > and mark it __rte_internal, or move this helper to a non public header > used by the unit test. > > Or add a (internal) rte_crypto_asym_xform_type_next_op() used through > a a RTE_CRYPTO_FOREACH_ASYM_OP() macro. > >
