Re: [dpdk-dev] [PATCH v2 4/4] add ABI checks

["Trahe, Fiona" <fiona.trahe@intel.com>]

> > > 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.
[Fiona] All asymmetric APIs are experimental so above shouldn't be a problem.
But for the same issue with sym crypto below, I believe Ferruh's explanation makes
sense and I don't see how there can be an API breakage.
So if an application hasn't compiled against the new lib it will be still using the old value 
which will be within bounds. If it's picking up the higher new value from the lib it must
have been compiled against the lib so shouldn't have problems.
There are also no structs on the API which contain arrays using this for sizing, so I don't see an 
opportunity for an appl to have a mismatch in memory addresses.

 

> > >>   [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?
> 
> 
> 
>