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

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Jun 05, 2020 at 12:00:56PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Thursday, June 4, 2020 3:42 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>; arybchenko@solarflare.com; dev@dpdk.org;
> > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH] ethdev: fix dpdk gcc build on Arm
> > 
> > On Thu, Jun 04, 2020 at 12:55:40PM +0000, 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.
> > >
> > > I'll try Meson build a few times. How can I enable those two config options in
> > Meson?
> > >
> > 
> > The MLX drivers will be enabled automatically if the required dependencies are
> > found. There is no meson build option to switch to using 16B descriptors in the
> > i40e driver, since we are really trying to limit the build-time configurability in
> > meson. However, you should be able to enable it by setting "-
> > DRTE_LIBRTE_I40E_16BYTE_RX_DESC" in CFLAGS before calling meson.
> 
> Thanks Bruce, I'll try the passing the cflag. Seems to be working without it and I'm curious to see whether this will have any impact.

Yes, functionally everything should be the same with or without the 16B
descriptor flag, using the smaller descriptors can just save a small amount
of PCI bandwidth and give a smaller cache footprint. For interests sake,
please let us know if you see any perf difference in your app with or
without the setting.

Thanks,
/Bruce