On 1/30/2020 8:18 PM, Thomas Monjalon wrote:

quoted

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

quoted

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.

First this concern is not an ABI break concern, but application should ignore
any value bigger than the MAX value it knows.
Otherwise this would mean we can't add any new enum or define to the project,
which is wrong I believe.

quoted

quoted

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.

Reading through this report, I am also don't see why it is considered as ABI breakage.
Yes, size of rte_crypto_aead_algorithm_strings[] has changed, but this array is not public one.
Also I don't see any place where we use RTE_CRYPTO_AEAD_LIST_END to define array size
in our public API.
At first glance it looks like false positive to me.
Do I miss something obvious here?
Konstantin

quoted

quoted

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?