30/01/2020 17:09, Ferruh Yigit: > On 1/29/2020 8:13 PM, Akhil Goyal wrote: > > > > > >> > >> On Wed, Jan 29, 2020 at 7:10 PM Anoob Joseph <ano...@marvell.com> wrote: > >>> The asymmetric crypto library is experimental. Changes to experimental > >>> code > >> paths is allowed, right? > >> > >> The asymmetric crypto enum is referenced by a function part of the stable > >> ABI. > >> It is possible to waive this enum, if we are sure no use out of the > >> experimental asym crypto APIs is possible. > >> > >> The rest of the changes touch stable symbols. > >> > >> Adding the abidiff report: > >> > >> [C]'function void rte_cryptodev_info_get(uint8_t, > >> rte_cryptodev_info*)' at rte_cryptodev.c:1115:1 has some indirect > >> sub-type changes: > >> parameter 2 of type 'rte_cryptodev_info*' has sub-type changes: > >> in pointed to type 'struct rte_cryptodev_info' at > >> rte_cryptodev.h:468:1: > >> type size hasn't changed > >> 1 data member change: > >> type of 'const rte_cryptodev_capabilities* > >> rte_cryptodev_info::capabilities' changed: > >> in pointed to type 'const rte_cryptodev_capabilities': > >> in unqualified underlying type 'struct > >> rte_cryptodev_capabilities' at rte_cryptodev.h:176:1: > >> type size hasn't changed > >> 1 data member change: > >> type of '__anonymous_union__ ' changed: > >> type size hasn't changed > >> 1 data member change: > >> type of 'rte_cryptodev_asymmetric_capability > >> __anonymous_union__::asym' changed: > >> type size hasn't changed > >> 1 data member change: > >> type of > >> 'rte_cryptodev_asymmetric_xform_capability > >> rte_cryptodev_asymmetric_capability::xform_capa' changed: > >> type size hasn't changed > >> 1 data member change: > >> type of 'rte_crypto_asym_xform_type > >> rte_cryptodev_asymmetric_xform_capability::xform_type' changed: > >> type size hasn't changed > >> 2 enumerator insertions: > >> > >> 'rte_crypto_asym_xform_type::RTE_CRYPTO_ASYM_XFORM_ECDSA' value '7' > >> > >> 'rte_crypto_asym_xform_type::RTE_CRYPTO_ASYM_XFORM_ECPM' value '8' > >> 1 enumerator change: > >> > >> 'rte_crypto_asym_xform_type::RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END' > >> from > >> value '7' to '9' at rte_crypto_asym.h:60:1 > >> > > > > I believe these enums will be used only in case of ASYM case which is > > experimental. > > Independent from being experiment and not, this shouldn't be a problem, I > think > this is a false positive. > > The ABI break can happen when a struct has been shared between the application > and the library (DPDK) and the layout of that memory know differently by > application and the library. > > Here in all cases, there is no layout/size change. > > As to the value changes of the enums, since application compiled with old > DPDK, > it will know only up to '6', 7 and more means invalid to the application. So > it > won't send these values also it should ignore these values from library. Only > consequence is old application won't able to use new features those new enums > provide but that is expected/normal.
If library give higher value than expected by the application, if the application uses this value as array index, there can be an access out of bounds. > >> [C]'function int > >> rte_cryptodev_get_aead_algo_enum(rte_crypto_aead_algorithm*, const > >> char*)' at rte_cryptodev.c:239:1 has some indirect sub-type changes: > >> parameter 1 of type 'rte_crypto_aead_algorithm*' has sub-type changes: > >> in pointed to type 'enum rte_crypto_aead_algorithm' at > >> rte_crypto_sym.h:346:1: > >> type size hasn't changed > >> 1 enumerator insertion: > >> 'rte_crypto_aead_algorithm::RTE_CRYPTO_AEAD_CHACHA20_POLY1305' > >> value '3' > >> 1 enumerator change: > >> 'rte_crypto_aead_algorithm::RTE_CRYPTO_AEAD_LIST_END' from > >> value '3' to '4' at rte_crypto_sym.h:346:1 > > Same as above, no layout change. > > >> > >> > >> [C]'const char* rte_crypto_aead_algorithm_strings[1]' was changed at > >> rte_crypto_sym.h:358:1: > >> size of symbol (in bytes) changed from 24 to 32 > >> > > The shared memory size changes, but this is global variable in the library, > and > the values application can request 'RTE_CRYPTO_AEAD_AES_CCM' & > 'RTE_CRYPTO_AEAD_AES_GCM' is already there, so there is no backward > compatibility issue here. For this one, I don't know what is the breakage. > > +Fiona and Arek > > > > We may need to revert the chacha-poly patches. > > > > I don't see any ABI break in this case, can someone explain if I am missing > anything here?
