Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-dpdk/Makefile.am
line 19
@@ -348,8 +346,10 @@ endif
endif
endif
if ODP_SCHEDULE_SCALABLE
+__LIB__libodp_dpdk_la_SOURCES += ../linux-generic/queue/scalable.c
../linux-generic/queue/scalable.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE
else
+__LIB__libodp_dpdk_la_SOURCES += ../linux-generic/queue/generic.c
../linux-generic/queue/generic.lo: AM_CFLAGS += -DIM_ACTIVE_MODULE
endif
Comment:
This is interesting. I manually added the braces to push forward and now I see
this:
```
CC pktio/dpdk.lo
In file included from ./pktio/dpdk.h:46:0,
from pktio/dpdk.c:35:
/home/bill/linaro/dpdk/x86_64-native-linuxapp-gcc/include/rte_hash_crc.h: In
function ârte_hash_crc_set_algâ:
/home/bill/linaro/dpdk/x86_64-native-linuxapp-gcc/include/rte_hash_crc.h:477:6:
error: this statement may fall through [-Werror=implicit-fallthrough=]
if (! rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T))
^
/home/bill/linaro/dpdk/x86_64-native-linuxapp-gcc/include/rte_hash_crc.h:479:2:
note: here
case CRC32_SSE42:
^~~~
/home/bill/linaro/dpdk/x86_64-native-linuxapp-gcc/include/rte_hash_crc.h:480:6:
error: this statement may fall through [-Werror=implicit-fallthrough=]
if (! rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_2))
^
/home/bill/linaro/dpdk/x86_64-native-linuxapp-gcc/include/rte_hash_crc.h:483:2:
note: here
case CRC32_SW:
^~~~
cc1: all warnings being treated as errors
Makefile:1417: recipe for target 'pktio/dpdk.lo' failed
make[1]: *** [pktio/dpdk.lo] Error 1
```
I still have DPDK 17.02 installed but that clearly isn't compatible with GCC 7
either. These sort of errors suggest that PR #321 posted by @lumag may also
have some compiler-level sensitivities.
> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
> OK, I'm unable to verify that given this other issue. I've verified I see the
> same error on the current 2.0 branch without any `--enable-schedule-xxx`
> options.
>
> @muvarov Any ideas why Travis isn't seeing this? This clearly is an error in
> the code:
> ```
> /* Setup session */
> session = rte_cryptodev_sym_session_create(cdev_id, first_xform);
>
> if (session == NULL)
> /* remove the crypto_session_entry_t */
> memset(entry, 0, sizeof(*entry));
> free_session(entry);
> return -1;
>
> entry->rte_session = (intptr_t)session;
> ```
> That `if` statement should be in braces.
>> nagarahalli wrote
>> This make file change is for not compiling default queue when scalable queue
>> is enabled. This is required since the meta data fields required for default
>> queue are not available when scalable queue is enabled.
>>> nagarahalli wrote
>>> I am using GCC 5.4.0
>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> Hmmm, with that combo I'm seeing a different issue:
>>>> ```
>>>> CC odp_crypto.lo
>>>> odp_crypto.c: In function âodp_crypto_session_createâ:
>>>> odp_crypto.c:901:2: error: this âifâ clause does not guard...
>>>> [-Werror=misleading-indentation]
>>>> if (session == NULL)
>>>> ^~
>>>> odp_crypto.c:904:3: note: ...this statement, but the latter is
>>>> misleadingly indented as if it were guarded by the âifâ
>>>> free_session(entry);
>>>> ^~~~~~~~~~~~
>>>> cc1: all warnings being treated as errors
>>>> Makefile:1417: recipe for target 'odp_crypto.lo' failed
>>>> ```
>>>>
>>>> This looks like the base code is having problem with GCC 7? I'm running
>>>> Ubundu 17.10 which uses GCC 7.2.
>>>>> nagarahalli wrote
>>>>> Without these changes, try the compilation for the following
>>>>> configuration:
>>>>> ./configure --with-platform=linux-dpdk --enable-schedule-scalable
>>>>> --with-sdk-install-path=<dpdk path>
>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> I did some experimenting with this and this seems to compile and run
>>>>>> just fine without these Makefile.am changes. Am I missing something?
>>>>>>> nagarahalli wrote
>>>>>>> As we discussed in the call today, this is temporary. Once the changes,
>>>>>>> not to use packet meta data, to default queue implementation is done,
>>>>>>> these can go away.
>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> Is this intended to be a temporary change? The problem with
>>>>>>>> conditional compilation is that it's very easy to make changes that
>>>>>>>> break another file that isn't compiled unless some special config
>>>>>>>> option is used. That's why in today's config-based scheduler selection
>>>>>>>> we compile all modules and the only thing that's actually conditional
>>>>>>>> is which entry points are plugged into the scheduler interface table.
>>>>>>>>
>>>>>>>> For 2.0 we want to eliminate config-time selections like this, so all
>>>>>>>> variants will necessarily have to be compiled to be included in the
>>>>>>>> single binary and selected dynamically at runtime. So I'm not sure
>>>>>>>> this sort of change is consistent with that goal. This seems to need
>>>>>>>> more discussion as to why we're doing this here.
https://github.com/Linaro/odp/pull/303#discussion_r154744397
updated_at 2017-12-04 19:05:53
