Re: [dpdk-dev] [PATCH v3] examples/vhost: fix ioat dependency issue

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

On Thu, Nov 12, 2020 at 10:36:50AM +0100, David Marchand wrote:
> On Thu, Nov 12, 2020 at 8:30 AM Cheng Jiang <Cheng1.jiang@intel.com> wrote:
> >
> > Fix vhost-switch compiling issue when ioat dependency is missing.
> > Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file
> > and update Makefile. Clean some codes.
> >
> > Fixes: abec60e7115d ("examples/vhost: support vhost async data path")
> > Fixes: 3a04ecb21420 ("examples/vhost: add async vhost args parsing")
> >
> > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> > ---
> > v3:
> >  * Added fixes lines in commit log.
> >
> > v2:
> >  * Cleaned some codes
> >  * Changed RTE_RAW_IOAT check method in Makefile
> >  * Added ioat function definition when RTE_RAW_IOAT is missing
> >
> >  examples/vhost/Makefile    |  5 +++++
> >  examples/vhost/ioat.h      | 32 +++++++++++++++++++++++++-------
> >  examples/vhost/main.c      | 22 +++++++++++-----------
> >  examples/vhost/meson.build |  2 +-
> >  4 files changed, 42 insertions(+), 19 deletions(-)
> >
> > diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
> > index cec59d0e0..cbe56f742 100644
> > --- a/examples/vhost/Makefile
> > +++ b/examples/vhost/Makefile
> > @@ -28,6 +28,11 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
> >  LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
> >  LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)
> >
> > +HAS_RAW_IOAT=$(shell echo RTE_RAW_IOAT | $(CPP) $(CFLAGS) -P - | tail -1)
> > +ifeq ($(HAS_RAW_IOAT), 1)
> > +SRCS-y += ioat.c
> > +endif
> > +
> >  CFLAGS += -DALLOW_EXPERIMENTAL_API
> >
> >  build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build

<snip>

> I'll let Bruce and Maxime have the last word on this patch.
> But at least it works for me,
> Tested-by: David Marchand <david.marchand@redhat.com>
> 
I don't have a really strong objection to this, but I still would rather
see the ioat detection done in the C code only rather than in the Makefile.
I'm concerned about us adding too much complexity into our makefiles, so
would like to keep them simple as much as possible.

Therefore, I'd like to see ioat.c always included in SRCS-y, and ioat.c
just have #ifdef RTE_RAW_IOAT to block out any ioat-dependent code.

/Bruce