30/01/2020 17:09, Ferruh Yigit:

On 1/29/2020 8:13 PM, Akhil Goyal wrote:

quoted

quoted

On Wed, Jan 29, 2020 at 7:10 PM Anoob Joseph [off-list ref] wrote:

quoted

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.

quoted

quoted

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

quoted

quoted

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

quoted

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