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

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

[Cheng Jiang]

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.

Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
---
 examples/vhost/Makefile    | 5 +++++
 examples/vhost/ioat.h      | 2 +-
 examples/vhost/meson.build | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
index cec59d0e0f..505e443217 100644
--- a/examples/vhost/Makefile
+++ b/examples/vhost/Makefile
@@ -5,7 +5,12 @@
 APP = vhost-switch
 
 # all source are stored in SRCS-y
+IOAT_PATH = $(shell pkg-config --cflags-only-I libdpdk | sed -e "s/^..//")/rte_ioat_rawdev.h
+ifeq ($(IOAT_PATH), $(wildcard $(IOAT_PATH)))
+SRCS-y := main.c virtio_net.c ioat.c
+else
 SRCS-y := main.c virtio_net.c
+endif
 
 # Build using pkg-config variables if possible
 ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
index 9664fcc3ac..d6d0f7c18a 100644
--- a/examples/vhost/ioat.h
+++ b/examples/vhost/ioat.h
@@ -24,7 +24,7 @@ struct dma_for_vhost {
 	uint16_t nr;
 };
 
-#ifdef RTE_ARCH_X86
+#ifdef RTE_RAW_IOAT
 int open_ioat(const char *value);
 #else
 static int open_ioat(const char *value __rte_unused)
diff --git a/examples/vhost/meson.build b/examples/vhost/meson.build
index 24f1f71313..d5388a795a 100644
--- a/examples/vhost/meson.build
+++ b/examples/vhost/meson.build
@@ -15,7 +15,7 @@ sources = files(
 	'main.c', 'virtio_net.c'
 )
 
-if dpdk_conf.has('RTE_ARCH_X86')
+if dpdk_conf.has('RTE_RAW_IOAT')
 	deps += 'raw_ioat'
 	sources += files('ioat.c')
 endif
-- 
2.29.2

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

[David Marchand]

On Wed, Nov 11, 2020 at 12:29 PM 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.

Still failing for me.
98b4c65506 - (HEAD) examples/vhost: fix ioat dependency issue (28
seconds ago) <Cheng Jiang>
a8adac0bc0 - (origin/main) doc: add instructions for building 32-bit
DPDK (5 days ago) <Bruce Richardson>

Please try the reproducer I gave:

rm -f build/vhost-switch build/vhost-switch-static build/vhost-switch-shared
test -d build && rmdir -p build || true
cc -O3 -I/home/dmarchan/dpdk/installdir/usr/local/include -include
rte_config.h -march=native -I/usr/usr/include
-DALLOW_EXPERIMENTAL_API main.c virtio_net.c -o
build/vhost-switch-shared -pthread -Wl,--as-needed
-L/home/dmarchan/dpdk/installdir/usr/local/lib64 -lrte_node
-lrte_graph -lrte_bpf -lrte_flow_classify -lrte_pipeline -lrte_table
-lrte_port -lrte_fib -lrte_ipsec -lrte_vhost -lrte_stack
-lrte_security -lrte_sched -lrte_reorder -lrte_rib -lrte_regexdev
-lrte_rawdev -lrte_pdump -lrte_power -lrte_member -lrte_lpm
-lrte_latencystats -lrte_kni -lrte_jobstats -lrte_ip_frag -lrte_gso
-lrte_gro -lrte_eventdev -lrte_efd -lrte_distributor -lrte_cryptodev
-lrte_compressdev -lrte_cfgfile -lrte_bitratestats -lrte_bbdev
-lrte_acl -lrte_timer -lrte_hash -lrte_metrics -lrte_cmdline -lrte_pci
-lrte_ethdev -lrte_meter -lrte_net -lrte_mbuf -lrte_mempool -lrte_rcu
-lrte_ring -lrte_eal -lrte_telemetry -lrte_kvargs -lbsd
/usr/bin/ld: /tmp/ccqE1W9S.o: in function `new_device':
main.c:(.text+0x173): undefined reference to `ioat_transfer_data_cb'
/usr/bin/ld: main.c:(.text+0x178): undefined reference to
`ioat_check_completed_copies_cb'
/usr/bin/ld: /tmp/ccqE1W9S.o: in function `main':
main.c:(.text.startup+0x25e): undefined reference to `open_ioat'
collect2: error: ld returned 1 exit status
make: *** [Makefile:39: build/vhost-switch-shared] Error 1


There are at least two problems:
- the code in main.c unconditionally expects the ioat stuff to be
available (this is why the link fails above),
- the Makefile unconditionally compiles ioat.c, which you fixed with this patch,

There is a potential additional problem:
I would expect a need for linking against the rte_raw_ioat driver, but
we are "lucky" that all that is used in this example are inlines.
So I guess it works without adding anything to LDFLAGS_SHARED.


>
> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> ---
>  examples/vhost/Makefile    | 5 +++++
>  examples/vhost/ioat.h      | 2 +-
>  examples/vhost/meson.build | 2 +-
>  3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
> index cec59d0e0f..505e443217 100644
> --- a/examples/vhost/Makefile
> +++ b/examples/vhost/Makefile
> @@ -5,7 +5,12 @@
>  APP = vhost-switch
>
>  # all source are stored in SRCS-y
> +IOAT_PATH = $(shell pkg-config --cflags-only-I libdpdk | sed -e "s/^..//")/rte_ioat_rawdev.h
> +ifeq ($(IOAT_PATH), $(wildcard $(IOAT_PATH)))
> +SRCS-y := main.c virtio_net.c ioat.c
> +else
>  SRCS-y := main.c virtio_net.c
> +endif

I'd rather rely on the driver define since the C code relies on it:
Something like:

 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
 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


>
>  # Build using pkg-config variables if possible
>  ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
> diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
> index 9664fcc3ac..d6d0f7c18a 100644
> --- a/examples/vhost/ioat.h
> +++ b/examples/vhost/ioat.h
> @@ -24,7 +24,7 @@ struct dma_for_vhost {
>         uint16_t nr;
>  };
>
> -#ifdef RTE_ARCH_X86
> +#ifdef RTE_RAW_IOAT
>  int open_ioat(const char *value);

main.c should check for RTE_RAW_IOAT before including ioat.h.
And then in this header, you can remove this stub too.


>  #else
>  static int open_ioat(const char *value __rte_unused)


-- 
David Marchand

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

[Bruce Richardson]

On Wed, Nov 11, 2020 at 03:36:25PM +0100, David Marchand wrote:
> On Wed, Nov 11, 2020 at 12:29 PM 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.
> 
> Still failing for me.
> 98b4c65506 - (HEAD) examples/vhost: fix ioat dependency issue (28
> seconds ago) <Cheng Jiang>
> a8adac0bc0 - (origin/main) doc: add instructions for building 32-bit
> DPDK (5 days ago) <Bruce Richardson>
> 
> Please try the reproducer I gave:
> 
> rm -f build/vhost-switch build/vhost-switch-static build/vhost-switch-shared
> test -d build && rmdir -p build || true
> cc -O3 -I/home/dmarchan/dpdk/installdir/usr/local/include -include
> rte_config.h -march=native -I/usr/usr/include
> -DALLOW_EXPERIMENTAL_API main.c virtio_net.c -o
> build/vhost-switch-shared -pthread -Wl,--as-needed
> -L/home/dmarchan/dpdk/installdir/usr/local/lib64 -lrte_node
> -lrte_graph -lrte_bpf -lrte_flow_classify -lrte_pipeline -lrte_table
> -lrte_port -lrte_fib -lrte_ipsec -lrte_vhost -lrte_stack
> -lrte_security -lrte_sched -lrte_reorder -lrte_rib -lrte_regexdev
> -lrte_rawdev -lrte_pdump -lrte_power -lrte_member -lrte_lpm
> -lrte_latencystats -lrte_kni -lrte_jobstats -lrte_ip_frag -lrte_gso
> -lrte_gro -lrte_eventdev -lrte_efd -lrte_distributor -lrte_cryptodev
> -lrte_compressdev -lrte_cfgfile -lrte_bitratestats -lrte_bbdev
> -lrte_acl -lrte_timer -lrte_hash -lrte_metrics -lrte_cmdline -lrte_pci
> -lrte_ethdev -lrte_meter -lrte_net -lrte_mbuf -lrte_mempool -lrte_rcu
> -lrte_ring -lrte_eal -lrte_telemetry -lrte_kvargs -lbsd
> /usr/bin/ld: /tmp/ccqE1W9S.o: in function `new_device':
> main.c:(.text+0x173): undefined reference to `ioat_transfer_data_cb'
> /usr/bin/ld: main.c:(.text+0x178): undefined reference to
> `ioat_check_completed_copies_cb'
> /usr/bin/ld: /tmp/ccqE1W9S.o: in function `main':
> main.c:(.text.startup+0x25e): undefined reference to `open_ioat'
> collect2: error: ld returned 1 exit status
> make: *** [Makefile:39: build/vhost-switch-shared] Error 1
> 
> 
> There are at least two problems:
> - the code in main.c unconditionally expects the ioat stuff to be
> available (this is why the link fails above),
> - the Makefile unconditionally compiles ioat.c, which you fixed with this patch,
> 
> There is a potential additional problem:
> I would expect a need for linking against the rte_raw_ioat driver, but
> we are "lucky" that all that is used in this example are inlines.
> So I guess it works without adding anything to LDFLAGS_SHARED.
> 

It's not actually luck! :-)

> 
> >
> > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> > ---
> >  examples/vhost/Makefile    | 5 +++++
> >  examples/vhost/ioat.h      | 2 +-
> >  examples/vhost/meson.build | 2 +-
> >  3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
> > index cec59d0e0f..505e443217 100644
> > --- a/examples/vhost/Makefile
> > +++ b/examples/vhost/Makefile
> > @@ -5,7 +5,12 @@
> >  APP = vhost-switch
> >
> >  # all source are stored in SRCS-y
> > +IOAT_PATH = $(shell pkg-config --cflags-only-I libdpdk | sed -e "s/^..//")/rte_ioat_rawdev.h
> > +ifeq ($(IOAT_PATH), $(wildcard $(IOAT_PATH)))
> > +SRCS-y := main.c virtio_net.c ioat.c
> > +else
> >  SRCS-y := main.c virtio_net.c
> > +endif
> 
> I'd rather rely on the driver define since the C code relies on it:
> Something like:
> 
>  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
>  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
> 

A better solution again, I think, would be to handle this in C, and have
the file itself have #ifdef RTE_RAW_IOAT .. #endif around its contents.
Then it can just be blindly in the compilation list without make having to
do any dependency tracking, which starts us down quite a slippery slope.

> 
> >
> >  # Build using pkg-config variables if possible
> >  ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
> > diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
> > index 9664fcc3ac..d6d0f7c18a 100644
> > --- a/examples/vhost/ioat.h
> > +++ b/examples/vhost/ioat.h
> > @@ -24,7 +24,7 @@ struct dma_for_vhost {
> >         uint16_t nr;
> >  };
> >
> > -#ifdef RTE_ARCH_X86
> > +#ifdef RTE_RAW_IOAT
> >  int open_ioat(const char *value);
> 
> main.c should check for RTE_RAW_IOAT before including ioat.h.
> And then in this header, you can remove this stub too.
> 

+1 to this.

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

[Jiang, Cheng1]

Hi,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, November 11, 2020 10:36 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>; dev <dev@dpdk.org>; Fu, Patrick
> <patrick.fu@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Hu,
> Jiayu <jiayu.hu@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [PATCH v1] examples/vhost: fix ioat dependency issue
> 
> On Wed, Nov 11, 2020 at 12:29 PM 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.
> 
> Still failing for me.
> 98b4c65506 - (HEAD) examples/vhost: fix ioat dependency issue (28
> seconds ago) <Cheng Jiang>
> a8adac0bc0 - (origin/main) doc: add instructions for building 32-bit
> DPDK (5 days ago) <Bruce Richardson>
> 
> Please try the reproducer I gave:
> 
> rm -f build/vhost-switch build/vhost-switch-static build/vhost-switch-shared
> test -d build && rmdir -p build || true
> cc -O3 -I/home/dmarchan/dpdk/installdir/usr/local/include -include
> rte_config.h -march=native -I/usr/usr/include
> -DALLOW_EXPERIMENTAL_API main.c virtio_net.c -o
> build/vhost-switch-shared -pthread -Wl,--as-needed
> -L/home/dmarchan/dpdk/installdir/usr/local/lib64 -lrte_node
> -lrte_graph -lrte_bpf -lrte_flow_classify -lrte_pipeline -lrte_table
> -lrte_port -lrte_fib -lrte_ipsec -lrte_vhost -lrte_stack
> -lrte_security -lrte_sched -lrte_reorder -lrte_rib -lrte_regexdev
> -lrte_rawdev -lrte_pdump -lrte_power -lrte_member -lrte_lpm
> -lrte_latencystats -lrte_kni -lrte_jobstats -lrte_ip_frag -lrte_gso
> -lrte_gro -lrte_eventdev -lrte_efd -lrte_distributor -lrte_cryptodev
> -lrte_compressdev -lrte_cfgfile -lrte_bitratestats -lrte_bbdev
> -lrte_acl -lrte_timer -lrte_hash -lrte_metrics -lrte_cmdline -lrte_pci
> -lrte_ethdev -lrte_meter -lrte_net -lrte_mbuf -lrte_mempool -lrte_rcu
> -lrte_ring -lrte_eal -lrte_telemetry -lrte_kvargs -lbsd
> /usr/bin/ld: /tmp/ccqE1W9S.o: in function `new_device':
> main.c:(.text+0x173): undefined reference to `ioat_transfer_data_cb'
> /usr/bin/ld: main.c:(.text+0x178): undefined reference to
> `ioat_check_completed_copies_cb'
> /usr/bin/ld: /tmp/ccqE1W9S.o: in function `main':
> main.c:(.text.startup+0x25e): undefined reference to `open_ioat'
> collect2: error: ld returned 1 exit status
> make: *** [Makefile:39: build/vhost-switch-shared] Error 1
> 
> 
> There are at least two problems:
> - the code in main.c unconditionally expects the ioat stuff to be
> available (this is why the link fails above),
> - the Makefile unconditionally compiles ioat.c, which you fixed with this patch,
> 
> There is a potential additional problem:
> I would expect a need for linking against the rte_raw_ioat driver, but
> we are "lucky" that all that is used in this example are inlines.
> So I guess it works without adding anything to LDFLAGS_SHARED.
> 
> 
> >
> > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> > ---
> >  examples/vhost/Makefile    | 5 +++++
> >  examples/vhost/ioat.h      | 2 +-
> >  examples/vhost/meson.build | 2 +-
> >  3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
> > index cec59d0e0f..505e443217 100644
> > --- a/examples/vhost/Makefile
> > +++ b/examples/vhost/Makefile
> > @@ -5,7 +5,12 @@
> >  APP = vhost-switch
> >
> >  # all source are stored in SRCS-y
> > +IOAT_PATH = $(shell pkg-config --cflags-only-I libdpdk | sed -e
> "s/^..//")/rte_ioat_rawdev.h
> > +ifeq ($(IOAT_PATH), $(wildcard $(IOAT_PATH)))
> > +SRCS-y := main.c virtio_net.c ioat.c
> > +else
> >  SRCS-y := main.c virtio_net.c
> > +endif
> 
> I'd rather rely on the driver define since the C code relies on it:
> Something like:
> 
>  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
>  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
> 

Sure, I will fix it in the next version, thanks a lot.

> 
> >
> >  # Build using pkg-config variables if possible
> >  ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
> > diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
> > index 9664fcc3ac..d6d0f7c18a 100644
> > --- a/examples/vhost/ioat.h
> > +++ b/examples/vhost/ioat.h
> > @@ -24,7 +24,7 @@ struct dma_for_vhost {
> >         uint16_t nr;
> >  };
> >
> > -#ifdef RTE_ARCH_X86
> > +#ifdef RTE_RAW_IOAT
> >  int open_ioat(const char *value);
> 
> main.c should check for RTE_RAW_IOAT before including ioat.h.
> And then in this header, you can remove this stub too.
> 

As for this one, ioat.h don't have dependency on IOAT driver, it is needed by the example regardless of whether the raw/ioat driver exists. And I added more RTE_RAW_IOAT check in ioat.h. Now it can be compiled.

Thanks,
Cheng

> 
> >  #else
> >  static int open_ioat(const char *value __rte_unused)
> 
> 
> --
> David Marchand

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

[Bruce Richardson]

On Thu, Nov 12, 2020 at 07:14:26AM +0000, Jiang, Cheng1 wrote:
> Hi,
> 
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Wednesday, November 11, 2020 10:36 PM
> > To: Jiang, Cheng1 <cheng1.jiang@intel.com>
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> > <chenbo.xia@intel.com>; dev <dev@dpdk.org>; Fu, Patrick
> > <patrick.fu@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>; Hu,
> > Jiayu <jiayu.hu@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> > Yigit, Ferruh <ferruh.yigit@intel.com>
> > Subject: Re: [PATCH v1] examples/vhost: fix ioat dependency issue
> > 
> > On Wed, Nov 11, 2020 at 12:29 PM 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.
> > 
> > Still failing for me.
> > 98b4c65506 - (HEAD) examples/vhost: fix ioat dependency issue (28
> > seconds ago) <Cheng Jiang>
> > a8adac0bc0 - (origin/main) doc: add instructions for building 32-bit
> > DPDK (5 days ago) <Bruce Richardson>
> > 
> > Please try the reproducer I gave:
> > 
> > rm -f build/vhost-switch build/vhost-switch-static build/vhost-switch-shared
> > test -d build && rmdir -p build || true
> > cc -O3 -I/home/dmarchan/dpdk/installdir/usr/local/include -include
> > rte_config.h -march=native -I/usr/usr/include
> > -DALLOW_EXPERIMENTAL_API main.c virtio_net.c -o
> > build/vhost-switch-shared -pthread -Wl,--as-needed
> > -L/home/dmarchan/dpdk/installdir/usr/local/lib64 -lrte_node
> > -lrte_graph -lrte_bpf -lrte_flow_classify -lrte_pipeline -lrte_table
> > -lrte_port -lrte_fib -lrte_ipsec -lrte_vhost -lrte_stack
> > -lrte_security -lrte_sched -lrte_reorder -lrte_rib -lrte_regexdev
> > -lrte_rawdev -lrte_pdump -lrte_power -lrte_member -lrte_lpm
> > -lrte_latencystats -lrte_kni -lrte_jobstats -lrte_ip_frag -lrte_gso
> > -lrte_gro -lrte_eventdev -lrte_efd -lrte_distributor -lrte_cryptodev
> > -lrte_compressdev -lrte_cfgfile -lrte_bitratestats -lrte_bbdev
> > -lrte_acl -lrte_timer -lrte_hash -lrte_metrics -lrte_cmdline -lrte_pci
> > -lrte_ethdev -lrte_meter -lrte_net -lrte_mbuf -lrte_mempool -lrte_rcu
> > -lrte_ring -lrte_eal -lrte_telemetry -lrte_kvargs -lbsd
> > /usr/bin/ld: /tmp/ccqE1W9S.o: in function `new_device':
> > main.c:(.text+0x173): undefined reference to `ioat_transfer_data_cb'
> > /usr/bin/ld: main.c:(.text+0x178): undefined reference to
> > `ioat_check_completed_copies_cb'
> > /usr/bin/ld: /tmp/ccqE1W9S.o: in function `main':
> > main.c:(.text.startup+0x25e): undefined reference to `open_ioat'
> > collect2: error: ld returned 1 exit status
> > make: *** [Makefile:39: build/vhost-switch-shared] Error 1
> > 
> > 
> > There are at least two problems:
> > - the code in main.c unconditionally expects the ioat stuff to be
> > available (this is why the link fails above),
> > - the Makefile unconditionally compiles ioat.c, which you fixed with this patch,
> > 
> > There is a potential additional problem:
> > I would expect a need for linking against the rte_raw_ioat driver, but
> > we are "lucky" that all that is used in this example are inlines.
> > So I guess it works without adding anything to LDFLAGS_SHARED.
> > 
> > 
> > >
> > > Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> > > ---
> > >  examples/vhost/Makefile    | 5 +++++
> > >  examples/vhost/ioat.h      | 2 +-
> > >  examples/vhost/meson.build | 2 +-
> > >  3 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
> > > index cec59d0e0f..505e443217 100644
> > > --- a/examples/vhost/Makefile
> > > +++ b/examples/vhost/Makefile
> > > @@ -5,7 +5,12 @@
> > >  APP = vhost-switch
> > >
> > >  # all source are stored in SRCS-y
> > > +IOAT_PATH = $(shell pkg-config --cflags-only-I libdpdk | sed -e
> > "s/^..//")/rte_ioat_rawdev.h
> > > +ifeq ($(IOAT_PATH), $(wildcard $(IOAT_PATH)))
> > > +SRCS-y := main.c virtio_net.c ioat.c
> > > +else
> > >  SRCS-y := main.c virtio_net.c
> > > +endif
> > 
> > I'd rather rely on the driver define since the C code relies on it:
> > Something like:
> > 
> >  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
> >  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
> > 
> 
> Sure, I will fix it in the next version, thanks a lot.
> 
> > 
> > >
> > >  # Build using pkg-config variables if possible
> > >  ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
> > > diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
> > > index 9664fcc3ac..d6d0f7c18a 100644
> > > --- a/examples/vhost/ioat.h
> > > +++ b/examples/vhost/ioat.h
> > > @@ -24,7 +24,7 @@ struct dma_for_vhost {
> > >         uint16_t nr;
> > >  };
> > >
> > > -#ifdef RTE_ARCH_X86
> > > +#ifdef RTE_RAW_IOAT
> > >  int open_ioat(const char *value);
> > 
> > main.c should check for RTE_RAW_IOAT before including ioat.h.
> > And then in this header, you can remove this stub too.
> > 
> 
> As for this one, ioat.h don't have dependency on IOAT driver, it is needed by the example regardless of whether the raw/ioat driver exists. And I added more RTE_RAW_IOAT check in ioat.h. Now it can be compiled.
> 

The trouble is that the ioat header file won't be installed if the driver
is not built. They will be available for building inside the source tree,
but not on a system with dpdk installed using "ninja install"

/Bruce

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

[David Marchand]

On Thu, Nov 12, 2020 at 10:31 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > > main.c should check for RTE_RAW_IOAT before including ioat.h.
> > > And then in this header, you can remove this stub too.
> > >
> >
> > As for this one, ioat.h don't have dependency on IOAT driver, it is needed by the example regardless of whether the raw/ioat driver exists. And I added more RTE_RAW_IOAT check in ioat.h. Now it can be compiled.
> >
>
> The trouble is that the ioat header file won't be installed if the driver
> is not built. They will be available for building inside the source tree,
> but not on a system with dpdk installed using "ninja install"

I tested disabling the driver, and I can see:
installdir/usr/local/share/dpdk/examples/vhost/ioat.h

-- 
David Marchand

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

[Bruce Richardson]

On Thu, Nov 12, 2020 at 10:39:33AM +0100, David Marchand wrote:
> On Thu, Nov 12, 2020 at 10:31 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > > > main.c should check for RTE_RAW_IOAT before including ioat.h.
> > > > And then in this header, you can remove this stub too.
> > > >
> > >
> > > As for this one, ioat.h don't have dependency on IOAT driver, it is needed by the example regardless of whether the raw/ioat driver exists. And I added more RTE_RAW_IOAT check in ioat.h. Now it can be compiled.
> > >
> >
> > The trouble is that the ioat header file won't be installed if the driver
> > is not built. They will be available for building inside the source tree,
> > but not on a system with dpdk installed using "ninja install"
> 
> I tested disabling the driver, and I can see:
> installdir/usr/local/share/dpdk/examples/vhost/ioat.h
> 
My mistake, I assumed the reference was to the rte_ioat_rawdev.h file from
the ioat driver.
Please ignore my comment so.

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

[Cheng Jiang]

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.

Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
---
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
diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
index 9664fcc3a..d6e1e2e07 100644
--- a/examples/vhost/ioat.h
+++ b/examples/vhost/ioat.h
@@ -24,14 +24,8 @@ struct dma_for_vhost {
 	uint16_t nr;
 };

-#ifdef RTE_ARCH_X86
+#ifdef RTE_RAW_IOAT
 int open_ioat(const char *value);
-#else
-static int open_ioat(const char *value __rte_unused)
-{
-	return -1;
-}
-#endif

 uint32_t
 ioat_transfer_data_cb(int vid, uint16_t queue_id,
@@ -42,4 +36,28 @@ uint32_t
 ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		struct rte_vhost_async_status *opaque_data,
 		uint16_t max_packets);
+#else
+static int open_ioat(const char *value __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_transfer_data_cb(int vid __rte_unused, uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_desc *descs __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t count __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_check_completed_copies_cb(int vid __rte_unused,
+		uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t max_packets __rte_unused)
+{
+	return -1;
+}
+#endif
 #endif /* _IOAT_H_ */
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 59a1aff07..4dc6102ab 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1294,13 +1294,6 @@ new_device(int vid)
 	int lcore, core_add = 0;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
-
-	struct rte_vhost_async_channel_ops channel_ops = {
-		.transfer_data = ioat_transfer_data_cb,
-		.check_completed_copies = ioat_check_completed_copies_cb
-	};
-	struct rte_vhost_async_features f;
-
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
 	if (vdev == NULL) {
 		RTE_LOG(INFO, VHOST_DATA,
@@ -1342,10 +1335,17 @@ new_device(int vid)
 		vid, vdev->coreid);

 	if (async_vhost_driver) {
-		f.async_inorder = 1;
-		f.async_threshold = 256;
-		return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
-			f.intval, &channel_ops);
+		struct rte_vhost_async_features f;
+		struct rte_vhost_async_channel_ops channel_ops;
+		if (strncmp(dma_type, "ioat", 4) == 0) {
+			channel_ops.transfer_data = ioat_transfer_data_cb;
+			channel_ops.check_completed_copies =
+				ioat_check_completed_copies_cb;
+			f.async_inorder = 1;
+			f.async_threshold = 256;
+			return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
+				f.intval, &channel_ops);
+		}
 	}

 	return 0;
diff --git a/examples/vhost/meson.build b/examples/vhost/meson.build
index 24f1f7131..d5388a795 100644
--- a/examples/vhost/meson.build
+++ b/examples/vhost/meson.build
@@ -15,7 +15,7 @@ sources = files(
 	'main.c', 'virtio_net.c'
 )

-if dpdk_conf.has('RTE_ARCH_X86')
+if dpdk_conf.has('RTE_RAW_IOAT')
 	deps += 'raw_ioat'
 	sources += files('ioat.c')
 endif
--
2.29.2

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

[Cheng Jiang]

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
diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
index 9664fcc3a..d6e1e2e07 100644
--- a/examples/vhost/ioat.h
+++ b/examples/vhost/ioat.h
@@ -24,14 +24,8 @@ struct dma_for_vhost {
 	uint16_t nr;
 };

-#ifdef RTE_ARCH_X86
+#ifdef RTE_RAW_IOAT
 int open_ioat(const char *value);
-#else
-static int open_ioat(const char *value __rte_unused)
-{
-	return -1;
-}
-#endif

 uint32_t
 ioat_transfer_data_cb(int vid, uint16_t queue_id,
@@ -42,4 +36,28 @@ uint32_t
 ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		struct rte_vhost_async_status *opaque_data,
 		uint16_t max_packets);
+#else
+static int open_ioat(const char *value __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_transfer_data_cb(int vid __rte_unused, uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_desc *descs __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t count __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_check_completed_copies_cb(int vid __rte_unused,
+		uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t max_packets __rte_unused)
+{
+	return -1;
+}
+#endif
 #endif /* _IOAT_H_ */
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 59a1aff07..4dc6102ab 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1294,13 +1294,6 @@ new_device(int vid)
 	int lcore, core_add = 0;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
-
-	struct rte_vhost_async_channel_ops channel_ops = {
-		.transfer_data = ioat_transfer_data_cb,
-		.check_completed_copies = ioat_check_completed_copies_cb
-	};
-	struct rte_vhost_async_features f;
-
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
 	if (vdev == NULL) {
 		RTE_LOG(INFO, VHOST_DATA,
@@ -1342,10 +1335,17 @@ new_device(int vid)
 		vid, vdev->coreid);

 	if (async_vhost_driver) {
-		f.async_inorder = 1;
-		f.async_threshold = 256;
-		return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
-			f.intval, &channel_ops);
+		struct rte_vhost_async_features f;
+		struct rte_vhost_async_channel_ops channel_ops;
+		if (strncmp(dma_type, "ioat", 4) == 0) {
+			channel_ops.transfer_data = ioat_transfer_data_cb;
+			channel_ops.check_completed_copies =
+				ioat_check_completed_copies_cb;
+			f.async_inorder = 1;
+			f.async_threshold = 256;
+			return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
+				f.intval, &channel_ops);
+		}
 	}

 	return 0;
diff --git a/examples/vhost/meson.build b/examples/vhost/meson.build
index 24f1f7131..d5388a795 100644
--- a/examples/vhost/meson.build
+++ b/examples/vhost/meson.build
@@ -15,7 +15,7 @@ sources = files(
 	'main.c', 'virtio_net.c'
 )

-if dpdk_conf.has('RTE_ARCH_X86')
+if dpdk_conf.has('RTE_RAW_IOAT')
 	deps += 'raw_ioat'
 	sources += files('ioat.c')
 endif
--
2.29.2

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

[David Marchand]

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
> diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
> index 9664fcc3a..d6e1e2e07 100644
> --- a/examples/vhost/ioat.h
> +++ b/examples/vhost/ioat.h
> @@ -24,14 +24,8 @@ struct dma_for_vhost {
>         uint16_t nr;
>  };
>
> -#ifdef RTE_ARCH_X86
> +#ifdef RTE_RAW_IOAT
>  int open_ioat(const char *value);
> -#else
> -static int open_ioat(const char *value __rte_unused)
> -{
> -       return -1;
> -}
> -#endif
>
>  uint32_t
>  ioat_transfer_data_cb(int vid, uint16_t queue_id,
> @@ -42,4 +36,28 @@ uint32_t
>  ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
>                 struct rte_vhost_async_status *opaque_data,
>                 uint16_t max_packets);
> +#else
> +static int open_ioat(const char *value __rte_unused)
> +{
> +       return -1;
> +}
> +
> +static uint32_t
> +ioat_transfer_data_cb(int vid __rte_unused, uint16_t queue_id __rte_unused,
> +               struct rte_vhost_async_desc *descs __rte_unused,
> +               struct rte_vhost_async_status *opaque_data __rte_unused,
> +               uint16_t count __rte_unused)
> +{
> +       return -1;
> +}
> +
> +static uint32_t
> +ioat_check_completed_copies_cb(int vid __rte_unused,
> +               uint16_t queue_id __rte_unused,
> +               struct rte_vhost_async_status *opaque_data __rte_unused,
> +               uint16_t max_packets __rte_unused)
> +{
> +       return -1;
> +}
> +#endif
>  #endif /* _IOAT_H_ */
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 59a1aff07..4dc6102ab 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -1294,13 +1294,6 @@ new_device(int vid)
>         int lcore, core_add = 0;
>         uint32_t device_num_min = num_devices;
>         struct vhost_dev *vdev;
> -
> -       struct rte_vhost_async_channel_ops channel_ops = {
> -               .transfer_data = ioat_transfer_data_cb,
> -               .check_completed_copies = ioat_check_completed_copies_cb
> -       };
> -       struct rte_vhost_async_features f;
> -
>         vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
>         if (vdev == NULL) {
>                 RTE_LOG(INFO, VHOST_DATA,
> @@ -1342,10 +1335,17 @@ new_device(int vid)
>                 vid, vdev->coreid);
>
>         if (async_vhost_driver) {
> -               f.async_inorder = 1;
> -               f.async_threshold = 256;
> -               return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
> -                       f.intval, &channel_ops);
> +               struct rte_vhost_async_features f;
> +               struct rte_vhost_async_channel_ops channel_ops;
> +               if (strncmp(dma_type, "ioat", 4) == 0) {
> +                       channel_ops.transfer_data = ioat_transfer_data_cb;
> +                       channel_ops.check_completed_copies =
> +                               ioat_check_completed_copies_cb;
> +                       f.async_inorder = 1;
> +                       f.async_threshold = 256;
> +                       return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
> +                               f.intval, &channel_ops);
> +               }
>         }
>
>         return 0;
> diff --git a/examples/vhost/meson.build b/examples/vhost/meson.build
> index 24f1f7131..d5388a795 100644
> --- a/examples/vhost/meson.build
> +++ b/examples/vhost/meson.build
> @@ -15,7 +15,7 @@ sources = files(
>         'main.c', 'virtio_net.c'
>  )
>
> -if dpdk_conf.has('RTE_ARCH_X86')
> +if dpdk_conf.has('RTE_RAW_IOAT')
>         deps += 'raw_ioat'
>         sources += files('ioat.c')
>  endif
> --
> 2.29.2
>

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>


-- 
David Marchand

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

[Bruce Richardson]

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

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

[Jiang, Cheng1]

Hi Bruce,

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, November 12, 2020 6:28 PM
> To: David Marchand <david.marchand@redhat.com>
> Cc: Jiang, Cheng1 <cheng1.jiang@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>;
> dev <dev@dpdk.org>; Fu, Patrick <patrick.fu@intel.com>; Yang, YvonneX
> <yvonnex.yang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] examples/vhost: fix ioat dependency
> issue
> 
> 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

Tomorrow is the deadline for RC4, it's a little bit rush to prepare a new framework to cover the conditional compiling. Considering we have test efforts and CI check already on-going based on current patch, I would prefer to keep current code structure. Plus, pure C code change could also introduce more readability problem when we have more than one async callback implementations. 

Thanks,
Cheng

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

[Bruce Richardson]

On Thu, Nov 12, 2020 at 11:29:56AM +0000, Jiang, Cheng1 wrote:
> Hi Bruce,
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Thursday, November 12, 2020 6:28 PM
> > To: David Marchand <david.marchand@redhat.com>
> > Cc: Jiang, Cheng1 <cheng1.jiang@intel.com>; Maxime Coquelin
> > <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>;
> > dev <dev@dpdk.org>; Fu, Patrick <patrick.fu@intel.com>; Yang, YvonneX
> > <yvonnex.yang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v3] examples/vhost: fix ioat dependency
> > issue
> >
> > 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
> 
> Tomorrow is the deadline for RC4, it's a little bit rush to prepare a new framework to cover the conditional compiling. Considering we have test efforts and CI check already on-going based on current patch, I would prefer to keep current code structure. Plus, pure C code change could also introduce more readability problem when we have more than one async callback implementations.
>

I don't see any readability problems at all, in fact the result is cleaner
and shorter IMHO - especially the Makefile. I just tested with the
following changes applied on top of your patch, and all seemed to build ok.

Do you see any implementation problems with the below solution?

/Bruce

diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
index cbe56f742..8c969caaa 100644
--- a/examples/vhost/Makefile
+++ b/examples/vhost/Makefile
@@ -5,7 +5,7 @@
 APP = vhost-switch

 # all source are stored in SRCS-y
-SRCS-y := main.c virtio_net.c
+SRCS-y := main.c virtio_net.c ioat.c

 # Build using pkg-config variables if possible
 ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
@@ -28,11 +28,6 @@ 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
diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index b2c74f653..7c1a4f40d 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -8,6 +8,8 @@
 #include "ioat.h"
 #include "main.h"

+#ifdef RTE_RAW_IOAT
+
 struct dma_for_vhost dma_bind[MAX_VHOST_DEVICE];

 struct packet_tracker {
@@ -199,3 +201,5 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
        /* Opaque data is not supported */
        return -1;
 }
+
+#endif /* RTE_RAW_IOAT */

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

[Jiang, Cheng1]

Submitted v4 patch as per Bruce's suggestion.

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, November 12, 2020 8:02 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>
> Cc: David Marchand <david.marchand@redhat.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>;
> dev <dev@dpdk.org>; Fu, Patrick <patrick.fu@intel.com>; Yang, YvonneX
> <yvonnex.yang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] examples/vhost: fix ioat dependency
> issue
> 
> On Thu, Nov 12, 2020 at 11:29:56AM +0000, Jiang, Cheng1 wrote:
> > Hi Bruce,
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Thursday, November 12, 2020 6:28 PM
> > > To: David Marchand <david.marchand@redhat.com>
> > > Cc: Jiang, Cheng1 <cheng1.jiang@intel.com>; Maxime Coquelin
> > > <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>;
> > > dev <dev@dpdk.org>; Fu, Patrick <patrick.fu@intel.com>; Yang, YvonneX
> > > <yvonnex.yang@intel.com>; Hu, Jiayu <jiayu.hu@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH v3] examples/vhost: fix ioat dependency
> > > issue
> > >
> > > 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
> >
> > Tomorrow is the deadline for RC4, it's a little bit rush to prepare a new
> framework to cover the conditional compiling. Considering we have test
> efforts and CI check already on-going based on current patch, I would prefer
> to keep current code structure. Plus, pure C code change could also
> introduce more readability problem when we have more than one async
> callback implementations.
> >
> 
> I don't see any readability problems at all, in fact the result is cleaner
> and shorter IMHO - especially the Makefile. I just tested with the
> following changes applied on top of your patch, and all seemed to build ok.
> 
> Do you see any implementation problems with the below solution?
> 
> /Bruce
> 
> diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
> index cbe56f742..8c969caaa 100644
> --- a/examples/vhost/Makefile
> +++ b/examples/vhost/Makefile
> @@ -5,7 +5,7 @@
>  APP = vhost-switch
> 
>  # all source are stored in SRCS-y
> -SRCS-y := main.c virtio_net.c
> +SRCS-y := main.c virtio_net.c ioat.c
> 
>  # Build using pkg-config variables if possible
>  ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
> @@ -28,11 +28,6 @@ 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
> diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
> index b2c74f653..7c1a4f40d 100644
> --- a/examples/vhost/ioat.c
> +++ b/examples/vhost/ioat.c
> @@ -8,6 +8,8 @@
>  #include "ioat.h"
>  #include "main.h"
> 
> +#ifdef RTE_RAW_IOAT
> +
>  struct dma_for_vhost dma_bind[MAX_VHOST_DEVICE];
> 
>  struct packet_tracker {
> @@ -199,3 +201,5 @@ ioat_check_completed_copies_cb(int vid, uint16_t
> queue_id,
>         /* Opaque data is not supported */
>         return -1;
>  }
> +
> +#endif /* RTE_RAW_IOAT */

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

[Cheng Jiang]

Fix vhost-switch compiling issue when ioat dependency is missing.
Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file.
Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
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>
---
v4:
 * Use macros in ioat.c for conditional compilation instead of changing Makefile.

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    |  2 +-
 examples/vhost/ioat.c      |  6 ++++++
 examples/vhost/ioat.h      | 32 +++++++++++++++++++++++++-------
 examples/vhost/main.c      | 22 +++++++++++-----------
 examples/vhost/meson.build |  2 +-
 5 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
index cec59d0e0..8c969caaa 100644
--- a/examples/vhost/Makefile
+++ b/examples/vhost/Makefile
@@ -5,7 +5,7 @@
 APP = vhost-switch

 # all source are stored in SRCS-y
-SRCS-y := main.c virtio_net.c
+SRCS-y := main.c virtio_net.c ioat.c

 # Build using pkg-config variables if possible
 ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index b2c74f653..6f87d7bba 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -1,13 +1,17 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2010-2020 Intel Corporation
  */
+#ifdef RTE_RAW_IOAT
 #include <rte_rawdev.h>
 #include <rte_ioat_rawdev.h>
+#endif /* RTE_RAW_IOAT */
 #include <sys/uio.h>

 #include "ioat.h"
 #include "main.h"

+#ifdef RTE_RAW_IOAT
+
 struct dma_for_vhost dma_bind[MAX_VHOST_DEVICE];

 struct packet_tracker {
@@ -199,3 +203,5 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 	/* Opaque data is not supported */
 	return -1;
 }
+
+#endif /* RTE_RAW_IOAT */
diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
index 9664fcc3a..d6e1e2e07 100644
--- a/examples/vhost/ioat.h
+++ b/examples/vhost/ioat.h
@@ -24,14 +24,8 @@ struct dma_for_vhost {
 	uint16_t nr;
 };

-#ifdef RTE_ARCH_X86
+#ifdef RTE_RAW_IOAT
 int open_ioat(const char *value);
-#else
-static int open_ioat(const char *value __rte_unused)
-{
-	return -1;
-}
-#endif

 uint32_t
 ioat_transfer_data_cb(int vid, uint16_t queue_id,
@@ -42,4 +36,28 @@ uint32_t
 ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		struct rte_vhost_async_status *opaque_data,
 		uint16_t max_packets);
+#else
+static int open_ioat(const char *value __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_transfer_data_cb(int vid __rte_unused, uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_desc *descs __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t count __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_check_completed_copies_cb(int vid __rte_unused,
+		uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t max_packets __rte_unused)
+{
+	return -1;
+}
+#endif
 #endif /* _IOAT_H_ */
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 59a1aff07..4dc6102ab 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1294,13 +1294,6 @@ new_device(int vid)
 	int lcore, core_add = 0;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
-
-	struct rte_vhost_async_channel_ops channel_ops = {
-		.transfer_data = ioat_transfer_data_cb,
-		.check_completed_copies = ioat_check_completed_copies_cb
-	};
-	struct rte_vhost_async_features f;
-
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
 	if (vdev == NULL) {
 		RTE_LOG(INFO, VHOST_DATA,
@@ -1342,10 +1335,17 @@ new_device(int vid)
 		vid, vdev->coreid);

 	if (async_vhost_driver) {
-		f.async_inorder = 1;
-		f.async_threshold = 256;
-		return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
-			f.intval, &channel_ops);
+		struct rte_vhost_async_features f;
+		struct rte_vhost_async_channel_ops channel_ops;
+		if (strncmp(dma_type, "ioat", 4) == 0) {
+			channel_ops.transfer_data = ioat_transfer_data_cb;
+			channel_ops.check_completed_copies =
+				ioat_check_completed_copies_cb;
+			f.async_inorder = 1;
+			f.async_threshold = 256;
+			return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
+				f.intval, &channel_ops);
+		}
 	}

 	return 0;
diff --git a/examples/vhost/meson.build b/examples/vhost/meson.build
index 24f1f7131..d5388a795 100644
--- a/examples/vhost/meson.build
+++ b/examples/vhost/meson.build
@@ -15,7 +15,7 @@ sources = files(
 	'main.c', 'virtio_net.c'
 )

-if dpdk_conf.has('RTE_ARCH_X86')
+if dpdk_conf.has('RTE_RAW_IOAT')
 	deps += 'raw_ioat'
 	sources += files('ioat.c')
 endif
--
2.29.2

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

[Bruce Richardson]

On Thu, Nov 12, 2020 at 01:47:54PM +0000, Cheng Jiang wrote:
> Fix vhost-switch compiling issue when ioat dependency is missing.
> Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file.
> Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
> 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>
> ---
> v4:
>  * Use macros in ioat.c for conditional compilation instead of changing Makefile.
> 
> 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    |  2 +-
>  examples/vhost/ioat.c      |  6 ++++++
>  examples/vhost/ioat.h      | 32 +++++++++++++++++++++++++-------
>  examples/vhost/main.c      | 22 +++++++++++-----------
>  examples/vhost/meson.build |  2 +-
>  5 files changed, 44 insertions(+), 20 deletions(-)
> 
> diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
> index cec59d0e0..8c969caaa 100644
> --- a/examples/vhost/Makefile
> +++ b/examples/vhost/Makefile
> @@ -5,7 +5,7 @@
>  APP = vhost-switch
> 
>  # all source are stored in SRCS-y
> -SRCS-y := main.c virtio_net.c
> +SRCS-y := main.c virtio_net.c ioat.c
> 
>  # Build using pkg-config variables if possible
>  ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
> diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
> index b2c74f653..6f87d7bba 100644
> --- a/examples/vhost/ioat.c
> +++ b/examples/vhost/ioat.c
> @@ -1,13 +1,17 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright(c) 2010-2020 Intel Corporation
>   */
> +#ifdef RTE_RAW_IOAT
>  #include <rte_rawdev.h>
>  #include <rte_ioat_rawdev.h>
> +#endif /* RTE_RAW_IOAT */
>  #include <sys/uio.h>
> 
>  #include "ioat.h"
>  #include "main.h"
> 
> +#ifdef RTE_RAW_IOAT
> +

Minor nit is that we generally include system header files before DPDK
headers. Following that policy, we can move sys/uio.h up to be before the
first #ifdef, and then we can just merge the two #ifdefs together to just
have one at the start and one at the end.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

[Jiang, Cheng1]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, November 12, 2020 11:02 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>
> Cc: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; Yang, YvonneX
> <yvonnex.yang@intel.com>; david.marchand@redhat.com; Hu, Jiayu
> <jiayu.hu@intel.com>
> Subject: Re: [PATCH v4] examples/vhost: fix ioat dependency issue
> 
> On Thu, Nov 12, 2020 at 01:47:54PM +0000, Cheng Jiang wrote:
> > Fix vhost-switch compiling issue when ioat dependency is missing.
> > Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file.
> > Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
> > 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>
> > ---
> > v4:
> >  * Use macros in ioat.c for conditional compilation instead of changing
> Makefile.
> >
> > 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    |  2 +-
> >  examples/vhost/ioat.c      |  6 ++++++
> >  examples/vhost/ioat.h      | 32 +++++++++++++++++++++++++-------
> >  examples/vhost/main.c      | 22 +++++++++++-----------
> >  examples/vhost/meson.build |  2 +-
> >  5 files changed, 44 insertions(+), 20 deletions(-)
> >
> > diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile index
> > cec59d0e0..8c969caaa 100644
> > --- a/examples/vhost/Makefile
> > +++ b/examples/vhost/Makefile
> > @@ -5,7 +5,7 @@
> >  APP = vhost-switch
> >
> >  # all source are stored in SRCS-y
> > -SRCS-y := main.c virtio_net.c
> > +SRCS-y := main.c virtio_net.c ioat.c
> >
> >  # Build using pkg-config variables if possible  ifneq ($(shell
> > pkg-config --exists libdpdk && echo 0),0) diff --git
> > a/examples/vhost/ioat.c b/examples/vhost/ioat.c index
> > b2c74f653..6f87d7bba 100644
> > --- a/examples/vhost/ioat.c
> > +++ b/examples/vhost/ioat.c
> > @@ -1,13 +1,17 @@
> >  /* SPDX-License-Identifier: BSD-3-Clause
> >   * Copyright(c) 2010-2020 Intel Corporation
> >   */
> > +#ifdef RTE_RAW_IOAT
> >  #include <rte_rawdev.h>
> >  #include <rte_ioat_rawdev.h>
> > +#endif /* RTE_RAW_IOAT */
> >  #include <sys/uio.h>
> >
> >  #include "ioat.h"
> >  #include "main.h"
> >
> > +#ifdef RTE_RAW_IOAT
> > +
> 
> Minor nit is that we generally include system header files before DPDK
> headers. Following that policy, we can move sys/uio.h up to be before the
> first #ifdef, and then we can just merge the two #ifdefs together to just have
> one at the start and one at the end.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Sure, fixed in the next version, thanks a lot.

Cheng

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

[Cheng Jiang]

Fix vhost-switch compiling issue when ioat dependency is missing.
Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file.
Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
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>
---
v5:
 * Cleaned macro conditional compilation in ioat.c.

v4:
 * Use macros in ioat.c for conditional compilation instead of changing Makefile.

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    |  2 +-
 examples/vhost/ioat.c      |  6 +++++-
 examples/vhost/ioat.h      | 32 +++++++++++++++++++++++++-------
 examples/vhost/main.c      | 22 +++++++++++-----------
 examples/vhost/meson.build |  2 +-
 5 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/examples/vhost/Makefile b/examples/vhost/Makefile
index cec59d0e0..8c969caaa 100644
--- a/examples/vhost/Makefile
+++ b/examples/vhost/Makefile
@@ -5,7 +5,7 @@
 APP = vhost-switch

 # all source are stored in SRCS-y
-SRCS-y := main.c virtio_net.c
+SRCS-y := main.c virtio_net.c ioat.c

 # Build using pkg-config variables if possible
 ifneq ($(shell pkg-config --exists libdpdk && echo 0),0)
diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index b2c74f653..fcd0597ea 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -1,9 +1,11 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2010-2020 Intel Corporation
  */
+
+#include <sys/uio.h>
+#ifdef RTE_RAW_IOAT
 #include <rte_rawdev.h>
 #include <rte_ioat_rawdev.h>
-#include <sys/uio.h>

 #include "ioat.h"
 #include "main.h"
@@ -199,3 +201,5 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 	/* Opaque data is not supported */
 	return -1;
 }
+
+#endif /* RTE_RAW_IOAT */
diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
index 9664fcc3a..d6e1e2e07 100644
--- a/examples/vhost/ioat.h
+++ b/examples/vhost/ioat.h
@@ -24,14 +24,8 @@ struct dma_for_vhost {
 	uint16_t nr;
 };

-#ifdef RTE_ARCH_X86
+#ifdef RTE_RAW_IOAT
 int open_ioat(const char *value);
-#else
-static int open_ioat(const char *value __rte_unused)
-{
-	return -1;
-}
-#endif

 uint32_t
 ioat_transfer_data_cb(int vid, uint16_t queue_id,
@@ -42,4 +36,28 @@ uint32_t
 ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		struct rte_vhost_async_status *opaque_data,
 		uint16_t max_packets);
+#else
+static int open_ioat(const char *value __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_transfer_data_cb(int vid __rte_unused, uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_desc *descs __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t count __rte_unused)
+{
+	return -1;
+}
+
+static uint32_t
+ioat_check_completed_copies_cb(int vid __rte_unused,
+		uint16_t queue_id __rte_unused,
+		struct rte_vhost_async_status *opaque_data __rte_unused,
+		uint16_t max_packets __rte_unused)
+{
+	return -1;
+}
+#endif
 #endif /* _IOAT_H_ */
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 59a1aff07..4dc6102ab 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -1294,13 +1294,6 @@ new_device(int vid)
 	int lcore, core_add = 0;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
-
-	struct rte_vhost_async_channel_ops channel_ops = {
-		.transfer_data = ioat_transfer_data_cb,
-		.check_completed_copies = ioat_check_completed_copies_cb
-	};
-	struct rte_vhost_async_features f;
-
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
 	if (vdev == NULL) {
 		RTE_LOG(INFO, VHOST_DATA,
@@ -1342,10 +1335,17 @@ new_device(int vid)
 		vid, vdev->coreid);

 	if (async_vhost_driver) {
-		f.async_inorder = 1;
-		f.async_threshold = 256;
-		return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
-			f.intval, &channel_ops);
+		struct rte_vhost_async_features f;
+		struct rte_vhost_async_channel_ops channel_ops;
+		if (strncmp(dma_type, "ioat", 4) == 0) {
+			channel_ops.transfer_data = ioat_transfer_data_cb;
+			channel_ops.check_completed_copies =
+				ioat_check_completed_copies_cb;
+			f.async_inorder = 1;
+			f.async_threshold = 256;
+			return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
+				f.intval, &channel_ops);
+		}
 	}

 	return 0;
diff --git a/examples/vhost/meson.build b/examples/vhost/meson.build
index 24f1f7131..d5388a795 100644
--- a/examples/vhost/meson.build
+++ b/examples/vhost/meson.build
@@ -15,7 +15,7 @@ sources = files(
 	'main.c', 'virtio_net.c'
 )

-if dpdk_conf.has('RTE_ARCH_X86')
+if dpdk_conf.has('RTE_RAW_IOAT')
 	deps += 'raw_ioat'
 	sources += files('ioat.c')
 endif
--
2.29.2

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

[Bruce Richardson]

On Thu, Nov 12, 2020 at 03:49:02PM +0000, Cheng Jiang wrote:
> Fix vhost-switch compiling issue when ioat dependency is missing.
> Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file.
> Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
> 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>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

[Maxime Coquelin]

On 11/12/20 4:49 PM, Cheng Jiang wrote:
> Fix vhost-switch compiling issue when ioat dependency is missing.
> Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file.
> Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
> 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>
> ---
> v5:
>  * Cleaned macro conditional compilation in ioat.c.
> 
> v4:
>  * Use macros in ioat.c for conditional compilation instead of changing Makefile.
> 
> 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    |  2 +-
>  examples/vhost/ioat.c      |  6 +++++-
>  examples/vhost/ioat.h      | 32 +++++++++++++++++++++++++-------
>  examples/vhost/main.c      | 22 +++++++++++-----------
>  examples/vhost/meson.build |  2 +-
>  5 files changed, 43 insertions(+), 21 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

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

[David Marchand]

On Thu, Nov 12, 2020 at 4:58 PM 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.
> Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
> 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>

Tested-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

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

[Maxime Coquelin]

On 11/12/20 4:49 PM, Cheng Jiang wrote:
> Fix vhost-switch compiling issue when ioat dependency is missing.
> Change 'RTE_x86' check into 'RTE_RAW_IOAT' check in meson build file.
> Use 'RTE_RAW_IOAT' to control conditional compiling in source file.
> 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>
> ---
> v5:
>  * Cleaned macro conditional compilation in ioat.c.
> 
> v4:
>  * Use macros in ioat.c for conditional compilation instead of changing Makefile.
> 
> 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    |  2 +-
>  examples/vhost/ioat.c      |  6 +++++-
>  examples/vhost/ioat.h      | 32 +++++++++++++++++++++++++-------
>  examples/vhost/main.c      | 22 +++++++++++-----------
>  examples/vhost/meson.build |  2 +-
>  5 files changed, 43 insertions(+), 21 deletions(-)

Applied to dpdk-next-virtio/main.

Thanks,
Maxime