We need someone from Intel to check on the testing platform please. It can be decided to drop testing of Suse 11 SP2. Thanks
15/01/2018 18:08, Adrien Mazarguil: > On Mon, Jan 15, 2018 at 05:18:37PM +0100, Adrien Mazarguil wrote: > > On Sat, Jan 13, 2018 at 08:14:06PM +0100, Thomas Monjalon wrote: > > > Hi, > > > > > > There is a new compilation error since this commit in LPM: > > > http://dpdk.org/commit/b2e1c99 > > > The brace has been removed because unnecessary with anonymous union. > > > > > > This union is declared with RTE_STD_C11 for compatibility > > > with old compilers: > > > /** 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 > > > > Yes, however not only for old compilers, e.g. explicitly specifying -std=c99 > > on the command-line disables C11 extensions for newer compilers as well. > > > > Not specifying anything (like most applications do) simply defaults to > > whatever standard is deemed "current" for it. > > > > In short, RTE_STD_C11 gets expanded as __extension__ when the compiler isn't > > in C11 mode, and what follows is therefore an extension to the standard in > > use (be it C90 or C99). > > > > __extension__ remains explicitly used in place of RTE_STD_C11 for things > > that are not even found in C11, namely GNU syntax extensions fall under this > > category. Keep in mind the __extension__ keyword is itself a GNU extension. > > > > > Unfortunately, it does not work on Suse 11 SP2 with GCC 4.5.1: > > > lib/librte_lpm/rte_lpm.c: In function ‘add_depth_big_v20’: > > > lib/librte_lpm/rte_lpm.c:886:4: error: > > > unknown field ‘group_idx’ specified in initializer > > > > > > Curiously, the error is exactly the same with ICC 16.0.2: > > > http://dpdk.org/ml/archives/test-report/2018-January/038443.html > > > Is it really using different compilers in those 2 tests? > > > > > > Someone to check the value of __STDC_VERSION__ with those compilers? > > > gcc -dM -E -xc /dev/null | grep STDC_VERSION > > > > > > Thanks for the help > > > > Since this problem only appears in big endian, my suggestion would be to add > > RTE_STD_C11 to the anonymous union of struct rte_lpm_tbl_entry_v20 > > (rte_lpm.h), like its little endian counterpart: > > > > #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN > > [...] > > RTE_STD_C11 > > union { > > uint8_t next_hop; > > uint8_t group_idx; > > }; > > [...] > > #else > > __extension__ > > struct rte_lpm_tbl_entry_v20 { > > uint8_t depth :6; > > uint8_t valid_group :1; > > uint8_t valid :1; > > RTE_STD_C11 // <<< Should be added here > > union { > > uint8_t group_idx; > > uint8_t next_hop; > > }; > > }; > > > > I don't have the adequate test environment to validate this, so please > > report if it helps and/or submit a patch, thanks. > > Looks like I mixed the issue mentioned by the original patch [1] and the one > you found on SuSE, which appears on little endian systems. > > Adding RTE_STD_C11 as suggested above is correct but useless since > __extension__ is part of the parent structure definition anyway, so this is > not the reason. > > Adding -pedantic (but not -std), this issue can be reproduced in a form or > another using GCC 4.4 through 4.9 which all default to C90, while GCC 6.3 > defaults to C11. Without -pendantic, I only managed to reproduce it with GCC > 4.4 (I don't have 4.5 handy, however it can't be reproduced using 4.6). > > The problem with GCC 4.4 and likely 4.5 is basically they do not support the > initialization syntax used in rte_lpm.c. Extra { } are needed even with > unnamed union fields, there's no way around that AFAIK. > > Since we likely don't want to revert [1] and although GCC 4.5 is not > recommended (4.9 minimum according to [2]), I suggest using a more > conventional initialization for this particular field, e.g. replacing: > > struct rte_lpm_tbl_entry_v20 new_tbl24_entry = { > .group_idx = (uint8_t)tbl8_group_index, > .valid = VALID, > .valid_group = 1, > .depth = 0, > }; > > With something like: > > struct rte_lpm_tbl_entry_v20 new_tbl24_entry = { > .valid = VALID, > .valid_group = 1, > .depth = 0, > }; > > /* Anonymous union field initialized outside (GCC < 4.6 compatibility). */ > new_tbl24_entry.group_idx = (uint8_t)tbl8_group_index; > > Your call. > > [1] http://dpdk.org/commit/b2e1c99 > [2] http://dpdk.org/doc/guides/linux_gsg/sys_reqs.html
