Re: [dpdk-dev] [PATCH] ethdev: fix dpdk gcc build on Arm

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

On 6/4/2020 1:55 PM, Juraj Linkeš wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, June 4, 2020 2:23 PM
>> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Thomas Monjalon
>> <thomas@monjalon.net>
>> Cc: arybchenko@solarflare.com; dev@dpdk.org; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>
>> Subject: Re: [PATCH] ethdev: fix dpdk gcc build on Arm
>>
>> On 6/4/2020 11:36 AM, Juraj Linkeš wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Wednesday, June 3, 2020 1:41 PM
>>>> To: Thomas Monjalon <thomas@monjalon.net>; Juraj Linkeš
>>>> <juraj.linkes@pantheon.tech>
>>>> Cc: arybchenko@solarflare.com; dev@dpdk.org
>>>> Subject: Re: [PATCH] ethdev: fix dpdk gcc build on Arm
>>>>
>>>> On 6/3/2020 11:16 AM, Thomas Monjalon wrote:
>>>>> 03/06/2020 11:48, Juraj Linkeš:
>>>>>> Directive #include <file> in gcc implementation searches for files
>>>>>> in a standard list of system directories, which leads to a
>>>>>> sporadici build error on Taishan arm machines:
>>>>>> /tmp/openvpp-testing/dpdk/lib/librte_ethdev/rte_ethdev.h:4287:10:
>>>>>> fatal error: rte_ethdev_core.h:
>>>>>> No such file or directory #include <rte_ethdev_core.h>
>>>>>
>>>>> Would be interesting to know why nobody else hit such error?
>>>>
>>>>
>>>> I can't see why this is happening, in the 'mk/rte.lib.mk' we have following:
>>>>
>>>> "
>>>> install: _preinstall build _postinstall
>>>> build: _preinstall
>>>> "
>>>>
>>>> Which should cause the library header files installed before building
>>>> .c files in that library.
>>>> So when compiling 'rte_class_eth.c', the header files should be
>>>> already in install folder.
>>>>
>>>>
>>>> I can see how/why changing to "" fixes the issue but I am not sure about this
>> fix.
>>>> "rte_ethdev.h" is a public header file, that applications will
>>>> include it in their applications. In the public library it is more
>>>> proper to have other includes from system folder, using format <>.
>>>> Again, I can't see why it is failing but I believe we should find
>>>> another solution for _internal_ build error.
>>>>
>>>>
>>>> A very simple solution can be following, but that is also not good,
>>>> since it solves the issue by creating a dependency to the order of the header
>> includes:
>>>>  diff --git a/lib/librte_ethdev/rte_class_eth.c
>>>> b/lib/librte_ethdev/rte_class_eth.c
>>>>  index 6338355e25..3030c49020 100644
>>>>  --- a/lib/librte_ethdev/rte_class_eth.c
>>>>  +++ b/lib/librte_ethdev/rte_class_eth.c
>>>>  @@ -10,8 +10,8 @@
>>>>   #include <rte_kvargs.h>
>>>>   #include <rte_log.h>
>>>>
>>>>  -#include "rte_ethdev.h"
>>>>   #include "rte_ethdev_core.h"
>>>>  +#include "rte_ethdev.h"
>>>>   #include "rte_ethdev_driver.h"
>>>>   #include "ethdev_private.h"
>>>>
>>>
>>> Thomas, Ferruh, what should be the solution? I'm not an expert on this and I
>> can't really offer anything better, but I'd like that this gets fixed.
>>
>> First we need to root cause this before trying to solve it. Honnappa also said he
>> can reproduce this but our CI builds can't (we are talking about tens of builds
>> daily on various platforms), need to understand why.
>> Also from Makefile I can't see how this is happening, I am feeling uneasy to fix
>> something before figuring out how/why it is failing.
>>
>> Can you please try to collect more data on when/how this happens, initial
>> questions I can think of:
>> - Can you reproduce this with meson build?
>> - Is it bare DPDK build, or build part of other project (I guess I saw fd.io on the
>> link)
>> - - If this is not bare DPDK build what changes has been done to build system?
>> - Do you see this with fresh build (new clone) or rebuild of existing clone?
>> - Can you confirm you have correct RTE_SDK and RTE_TARGET environment
>> variables?
>> - Can you please share your build command?
>>
> 
> I sent an e-mail to dpdk dev a few days back asking for help where I outlined what we're doing:
> We're not doing anything special, just downloading and extracting the archive, then setting CONFIG_RTE_LIBRTE_MLX5_PMD and CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC to y in config/common_base and then running make install T=arm64-armv8a-linuxapp-gcc -j. As mentioned in the subject, the build server is a Taishan ARM server.
> 
> We're doing a fresh rebuild everytime. The error doesn't happen everytime, just sometimes - it seems to be random.
> 
> We don't set RTE_SDK nor RTE_TARGET since https://doc.dpdk.org/guides/linux_gsg/build_dpdk.html#installation-of-dpdk-target-environment-using-make doesn't mention those.

Can you please try with "V=1",
"make install T=arm64-armv8a-linuxapp-gcc -j V=1",
this will generate verbose output.

And _before_ lines compiling the 'rte_class_eth.c' you should see something like
following lines, can you please check:
"
SYMLINK-FILE include/rte_ethdev_core.h

ln -nsf `.../buildtools/relpath.sh .../lib/librte_ethdev/rte_ethdev_core.h
.../x86_64-native-linux-gcc/include` .../x86_64-native-linux-gcc/include
"

Also for the failed case, after fail can you please check if
a) '.../arm64-armv8a-linuxapp-gcc/build/lib/librte_ethdev/_preinstall' file exists
b) '.../x86_64-native-linux-gcc/include/rte_ethdev_core.h' exists and it linked
to correct file '../../lib/librte_ethdev/rte_ethdev_core.h'


> 
> I'll try Meson build a few times. How can I enable those two config options in Meson?
> 
>> Thanks,
>> ferruh
>>
>>>
>>>>>
>>>>>
>>>>>> Fix this by replacing the directive with #include "file" which
>>>>>> searches for files first in the directory containing the current
>>>>>> file and other directories only after that.
>>>>>>
>>>>>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>>>>>
>>>>> What about #include <rte_ethdev_trace.h> ?
>>>>>
>>>>> Except this,
>>>>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>>>>
>>>>>
>>>>> I would change the title to be more generic (nothing specific to gcc
>>>>> or Arm
>>>> here):
>>>>> 	ethdev: fix local include
>>>>>
>>>>> It needs to be backported, so these lines are required:
>>>>> 	Fixes: 7f0a669e7b04 ("ethdev: add allocation helper for virtual drivers")
>>>>> 	Fixes: dcd5c8112bc3 ("ethdev: add PCI driver helpers")
>>>>> 	Fixes: ffc905f3b856 ("ethdev: separate driver APIs")
>>>>> 	Fixes: 331c447ad913 ("ethdev: separate internal structures into own
>>>> header")
>>>>> 	Cc: stable@dpdk.org
>>>>>
>>>>>
>>>>>
>>>
>