Re: [dpdk-dev] [PATCH v13 4/7] net/iavf: add iAVF IPsec inline crypto support

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 11/1/2021 10:45 AM, Ferruh Yigit wrote:
> On 10/30/2021 9:41 PM, David Marchand wrote:
>> On Thu, Oct 28, 2021 at 6:21 PM Radu Nicolau <radu.nicolau@intel.com> wrote:
>>> +static const struct rte_cryptodev_symmetric_capability *
>>> +get_capability(struct iavf_security_ctx *iavf_sctx,
>>> +       uint32_t algo, uint32_t type)
>>> +{
>>> +       const struct rte_cryptodev_capabilities *capability;
>>> +       int i = 0;
>>> +
>>> +       capability = &iavf_sctx->crypto_capabilities[i];
>>> +
>>> +       while (capability->op != RTE_CRYPTO_OP_TYPE_UNDEFINED) {
>>> +               if (capability->op == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
>>> +                       capability->sym.xform_type == type &&
>>> +                       capability->sym.cipher.algo == algo)
>>> +                       return &capability->sym;
>>> +               /** try next capability */
>>> +               capability = &iavf_crypto_capabilities[i++];
>>> +       }
>>> +
>>> +       return NULL;
>>> +}
>>
>> As of cc13af13c8e6 ("net/ngbe: support Tx done cleanup"), next-net
>> build is still KO for Windows:
>> http://mails.dpdk.org/archives/test-report/2021-October/236938.html
>>
>> FAILED: drivers/libtmp_rte_net_iavf.a.p/net_iavf_iavf_ipsec_crypto.c.obj
>> "clang" "-Idrivers\libtmp_rte_net_iavf.a.p" "-Idrivers" "-I..\drivers"
>> "-Idrivers\net\iavf" "-I..\drivers\net\iavf" "-Idrivers\common\iavf"
>> "-I..\drivers\common\iavf" "-Ilib\ethdev" "-I..\lib\ethdev" "-I."
>> "-I.." "-Iconfig" "-I..\config" "-Ilib\eal\include"
>> "-I..\lib\eal\include" "-Ilib\eal\windows\include"
>> "-I..\lib\eal\windows\include" "-Ilib\eal\x86\include"
>> "-I..\lib\eal\x86\include" "-Ilib\eal\common" "-I..\lib\eal\common"
>> "-Ilib\eal" "-I..\lib\eal" "-Ilib\kvargs" "-I..\lib\kvargs"
>> "-Ilib\net" "-I..\lib\net" "-Ilib\mbuf" "-I..\lib\mbuf"
>> "-Ilib\mempool" "-I..\lib\mempool" "-Ilib\ring" "-I..\lib\ring"
>> "-Ilib\metrics" "-I..\lib\metrics" "-Ilib\telemetry"
>> "-I..\lib\telemetry" "-Ilib\meter" "-I..\lib\meter"
>> "-Idrivers\bus\pci" "-I..\drivers\bus\pci"
>> "-I..\drivers\bus\pci\windows" "-Ilib\pci" "-I..\lib\pci"
>> "-Idrivers\bus\vdev" "-I..\drivers\bus\vdev" "-Ilib\security"
>> "-I..\lib\security" "-Ilib\cryptodev" "-I..\lib\cryptodev" "-Ilib\rcu"
>> "-I..\lib\rcu" "-Xclang" "-fcolor-diagnostics" "-pipe"
>> "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-Werror" "-O3"
>> "-include" "rte_config.h" "-Wextra" "-Wcast-qual" "-Wdeprecated"
>> "-Wformat" "-Wformat-nonliteral" "-Wformat-security"
>> "-Wmissing-declarations" "-Wmissing-prototypes" "-Wnested-externs"
>> "-Wold-style-definition" "-Wpointer-arith" "-Wsign-compare"
>> "-Wstrict-prototypes" "-Wundef" "-Wwrite-strings"
>> "-Wno-address-of-packed-member" "-Wno-missing-field-initializers"
>> "-D_GNU_SOURCE" "-D_WIN32_WINNT=0x0A00" "-D_CRT_SECURE_NO_WARNINGS"
>> "-march=native" "-DALLOW_EXPERIMENTAL_API" "-DALLOW_INTERNAL_API"
>> "-Wno-strict-aliasing" "-DCC_AVX2_SUPPORT" "-DCC_AVX512_SUPPORT"
>> "-DRTE_LOG_DEFAULT_LOGTYPE=pmd.net.iavf" -MD -MQ
>> drivers/libtmp_rte_net_iavf.a.p/net_iavf_iavf_ipsec_crypto.c.obj -MF
>> "drivers\libtmp_rte_net_iavf.a.p\net_iavf_iavf_ipsec_crypto.c.obj.d"
>> -o drivers/libtmp_rte_net_iavf.a.p/net_iavf_iavf_ipsec_crypto.c.obj
>> "-c" ../drivers/net/iavf/iavf_ipsec_crypto.c
>> ../drivers/net/iavf/iavf_ipsec_crypto.c:111:31: error: comparison of
>> integers of different signs: 'const enum rte_crypto_sym_xform_type'
>> and 'uint32_t' (aka 'unsigned int') [-Werror,-Wsign-compare]
>>                          capability->sym.xform_type == type &&
>>                          ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~
>> ../drivers/net/iavf/iavf_ipsec_crypto.c:112:32: error: comparison of
>> integers of different signs: 'const enum rte_crypto_cipher_algorithm'
>> and 'uint32_t' (aka 'unsigned int') [-Werror,-Wsign-compare]
>>                          capability->sym.cipher.algo == algo)
>>                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~
>> 2 errors generated.
>>
>>
> 
> Thanks for the report, I will update in next-net as following:
> 
> diff --git a/drivers/net/iavf/iavf_ipsec_crypto.c b/drivers/net/iavf/iavf_ipsec_crypto.c
> index 19e703e6895d..935f436ac4f1 100644
> --- a/drivers/net/iavf/iavf_ipsec_crypto.c
> +++ b/drivers/net/iavf/iavf_ipsec_crypto.c
> @@ -108,8 +108,8 @@ get_capability(struct iavf_security_ctx *iavf_sctx,
> 
>          while (capability->op != RTE_CRYPTO_OP_TYPE_UNDEFINED) {
>                  if (capability->op == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
> -                       capability->sym.xform_type == type &&
> -                       capability->sym.cipher.algo == algo)
> +                       capability->sym.xform_type == (int)type &&
> +                       capability->sym.cipher.algo == (int)algo)
>                          return &capability->sym;
>                  /** try next capability */
>                  capability = &iavf_crypto_capabilities[i++];

Hmm, with this some other compilers are failing with same error,
it looks like compilers can't agree on the sign of the enum.

According standard, enum type is implementation specific

'
N1570 Committee Draft — April 12, 2011 ISO/IEC 9899:201x

6.7.2.2 Enumeration specifiers
4 Each enumerated type shall be compatible with char, a signed integer type, or an
unsigned integer type. The choice of type is implementation-defined,128) but shall be
capable of representing the values of all the members of the enumeration.
'

I will cast both enum and value to 'int', although it looks ugly.