Re: [dpdk-dev] [EXT] Re: [PATCH v10] ethdev: add HIGIG2 key field to flow API

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

On 10/23/2019 3:04 PM, Olivier Matz wrote:
> On Wed, Oct 23, 2019 at 11:43:58AM +0000, Kiran Kumar Kokkilagadda wrote:
>>> -----Original Message-----
>>> From: Olivier Matz <olivier.matz@6wind.com>
>>> Sent: Wednesday, October 23, 2019 5:09 PM
>>> To: Raslan Darawsheh <rasland@mellanox.com>
>>> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Kiran Kumar Kokkilagadda
>>> <kirankumark@marvell.com>; Adrien Mazarguil
>>> <adrien.mazarguil@6wind.com>; Wenzhuo Lu <wenzhuo.lu@intel.com>;
>>> Jingjing Wu <jingjing.wu@intel.com>; Bernard Iremonger
>>> <bernard.iremonger@intel.com>; John McNamara
>>> <john.mcnamara@intel.com>; Marko Kovacevic <marko.kovacevic@intel.com>;
>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>>> <arybchenko@solarflare.com>; dev@dpdk.org; ajit.khaparde@broadcom.com
>>> Subject: [EXT] Re: [dpdk-dev] [PATCH v10] ethdev: add HIGIG2 key field to flow
>>> API
>>>
>>> External Email
>>>
>>> ----------------------------------------------------------------------
>>> Hi,
>>>
>>>
>>>
>>> On Wed, Oct 23, 2019 at 10:50:52AM +0000, Raslan Darawsheh wrote:
>>>
>>>> Hi,
>>>
>>>>
>>>
>>>> This patch broke the compilation of MLX5 PMD in debug mode:
>>>
>>>>
>>>
>>>>                  from /root/dpdk/x86_64-native-linux-
>>> gcc/include/rte_ethdev_driver.h:18,
>>>
>>>>                  from /root/dpdk/drivers/net/mlx5/mlx5_mp.c:11:
>>>
>>>> /root/dpdk/x86_64-native-linux-gcc/include/rte_higig.h:112:2: error: type of
>>> bit-field 'opcode' is a GCC extension [-Werror=pedantic]
>>>
>>>>   uint16_t opcode:3;
>>>
>>>>   ^
>>>
>>>> /root/dpdk/x86_64-native-linux-gcc/include/rte_higig.h:113:2: error: type of
>>> bit-field 'resv1' is a GCC extension [-Werror=pedantic]
>>>
>>>>   uint16_t resv1:2;
>>>
>>>>   ^
>>>
>>>> /root/dpdk/x86_64-native-linux-gcc/include/rte_higig.h:114:2: error: type of
>>> bit-field 'src_t' is a GCC extension [-Werror=pedantic]
>>>
>>>>   uint16_t src_t:1;
>>>
>>>>   ^
>>>
>>>> /root/dpdk/x86_64-native-linux-gcc/include/rte_higig.h:115:2: error: type of
>>> bit-field 'pfm' is a GCC extension [-Werror=pedantic]
>>>
>>>>   uint16_t pfm:2;
>>>
>>>>   ^
>>>
>>>> /root/dpdk/x86_64-native-linux-gcc/include/rte_higig.h:116:2: error: type of
>>> bit-field 'resv2' is a GCC extension [-Werror=pedantic]
>>>
>>>>   uint16_t resv2:5;
>>>
>>>>   ^
>>>
>>>> /root/dpdk/x86_64-native-linux-gcc/include/rte_higig.h:117:2: error: type of
>>> bit-field 'hdr_ext_len' is a GCC extension [-Werror=pedantic]
>>>
>>>>   uint16_t hdr_ext_len:3;
>>>
>>>>
>>>
>>>> and this is with gcc 4.8.5
>>>
>>>
>>>
>>> From https://urldefense.proofpoint.com/v2/url?u=https-
>>> 3A__stackoverflow.com_questions_10906238_warning-2Dwhen-2Dusing-
>>> 2Dbitfield-2Dwith-2Dunsigned-
>>> 2Dchar&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=owEKckYY4FTmil1Z6oBUR
>>> wkTThyuRbLAY9LdfiaT6HA&m=GZ-
>>> 6cngPycaUlGJT20VEOf9oTcp5PMwk7j1JV1vAQfs&s=SCg5yVPS4zZa8GSn9bl_eUtI
>>> vBmoDzi35PspWUttIUY&e=
>>>
>>> it seems that it is allowed in c99, so I guess it's a gcc 4.8 bug.
>>>
>>>
>>>
>>> Adding __extension__ above the struct solves the warnings, I suggest to
>>>
>>> add it.
>>
>> /**
>>  *
>>  * higig2 ppt type1 header.
>>  */
>> RTE_STD_C11
>> struct rte_higig2_ppt_type1 {
>>         uint16_t classification;
>>         uint16_t resv;
>>         uint16_t vid;
>> #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
>>         uint16_t opcode:3;
>>         uint16_t resv1:2;
>>         uint16_t src_t:1;
>>         uint16_t pfm:2;
>>         uint16_t resv2:5;
>>         uint16_t hdr_ext_len:3;
>> #elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
>>         uint16_t pfm:2;
>>         uint16_t src_t:1;
>>         uint16_t resv1:2;
>>         uint16_t opcode:3;
>>         uint16_t hdr_ext_len:3;
>>         uint16_t resv2:5;
>> #endif
>> };
>>
>> I have already added it. RTE_STD_C11 , this is a macro for __extension__. 
>> /** C extension macro for environments lacking C11 features. */
>> #if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L
>> #define RTE_STD_C11 __extension__
>> #else
>> #define RTE_STD_C11
>> #endif
> 
> The __extension__ is only added if compiled for < c11.
> But there is apparently a problem with gcc-4.8: even in c11, it does
> not support bitfields on u16. See: https://godbolt.org/z/hRIbgg
> 
> On gcc-4.9, the problem disappeared: https://godbolt.org/z/kkzz9v
> 
> So as a workaround, using __extension__ should work, probably with a short
> explanation.
> 

Thanks Olivier, this was also our finding with Raslan and he sent the fix already.
https://patches.dpdk.org/patch/61748/

Also it looks like gcc4.8 thinks this is not c11 standard but extension only for
uint16_t type, structs with uint32_t bit-fieds works as expected.