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 11:36 AM, Ferruh Yigit wrote:
> 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.

Or only cast enum to 'uint32_t' as suggested by Radu.