[v10,0/4] add async data path in vhost sample

  This patch set makes vhost-vswitch be able to use vhost async APIs
for enqueue operations. Demonstrated how the application
leverage IOAT DMA channel with vhost async APIs.

We introduce two parameters to enable DMA acceleration for Tx
operations of queues:
-dma_type This parameter is used to specify DMA type for async
vhost-user net driver.
-dmas This parameter is used to specify the assigned DMA device of a
vhost device and enable async vhost data path.

---
v10:
 * Fixed compilation issue on specific environment

v9:
 * Optimized code structure to solve compilation issue on non-x86 platforms

v8:
 * Changed meson build file due to the change of ioat component name

v7:
 * Improved IOAT callbacks and added some comments

v6:
 * Cleand code and rebased for latest code

v5:
 * Improved meson build file and fixed dependency problem

v4:
 * Code rebased for latest IOAT driver

v3:
 * Fixed a coding style problem

v2:
 * Changed meson build file to fix dependency problem
 * Added parameter description in usage function
 * Optimized parameter settings and parsing function
 * Optimized abstraction, moved some code to ioat.c

Cheng Jiang (4):
  example/vhost: add async vhost args parsing function
  example/vhost: add support for vhost async data path
  doc: update vhost sample doc for vhost async data path
  doc: update release notes for vhost sample

 doc/guides/rel_notes/release_20_11.rst |   6 +
 doc/guides/sample_app_ug/vhost.rst     |  11 ++
 examples/vhost/ioat.c                  | 201 +++++++++++++++++++++++++
 examples/vhost/ioat.h                  |  45 ++++++
 examples/vhost/main.c                  |  93 +++++++++++-
 examples/vhost/main.h                  |   1 +
 examples/vhost/meson.build             |   5 +
 7 files changed, 360 insertions(+), 2 deletions(-)
 create mode 100644 examples/vhost/ioat.c
 create mode 100644 examples/vhost/ioat.h

--
2.27.0
  

On 10/22/20 10:59 AM, Cheng Jiang wrote:
> This patch set makes vhost-vswitch be able to use vhost async APIs
> for enqueue operations. Demonstrated how the application
> leverage IOAT DMA channel with vhost async APIs.
> 
> We introduce two parameters to enable DMA acceleration for Tx
> operations of queues:
> -dma_type This parameter is used to specify DMA type for async
> vhost-user net driver.
> -dmas This parameter is used to specify the assigned DMA device of a
> vhost device and enable async vhost data path.
> 
> ---
> v10:
>  * Fixed compilation issue on specific environment
> 
> v9:
>  * Optimized code structure to solve compilation issue on non-x86 platforms
> 
> v8:
>  * Changed meson build file due to the change of ioat component name
> 
> v7:
>  * Improved IOAT callbacks and added some comments
> 
> v6:
>  * Cleand code and rebased for latest code
> 
> v5:
>  * Improved meson build file and fixed dependency problem
> 
> v4:
>  * Code rebased for latest IOAT driver
> 
> v3:
>  * Fixed a coding style problem
> 
> v2:
>  * Changed meson build file to fix dependency problem
>  * Added parameter description in usage function
>  * Optimized parameter settings and parsing function
>  * Optimized abstraction, moved some code to ioat.c
> 
> Cheng Jiang (4):
>   example/vhost: add async vhost args parsing function
>   example/vhost: add support for vhost async data path
>   doc: update vhost sample doc for vhost async data path
>   doc: update release notes for vhost sample
> 
>  doc/guides/rel_notes/release_20_11.rst |   6 +
>  doc/guides/sample_app_ug/vhost.rst     |  11 ++
>  examples/vhost/ioat.c                  | 201 +++++++++++++++++++++++++
>  examples/vhost/ioat.h                  |  45 ++++++
>  examples/vhost/main.c                  |  93 +++++++++++-
>  examples/vhost/main.h                  |   1 +
>  examples/vhost/meson.build             |   5 +
>  7 files changed, 360 insertions(+), 2 deletions(-)
>  create mode 100644 examples/vhost/ioat.c
>  create mode 100644 examples/vhost/ioat.h
> 
> --
> 2.27.0
> 


Applied to dpdk-next-virtio/main.

Thanks,
Maxime
  

On 10/23/2020 12:23 PM, Maxime Coquelin wrote:
> 
> 
> On 10/22/20 10:59 AM, Cheng Jiang wrote:
>> This patch set makes vhost-vswitch be able to use vhost async APIs
>> for enqueue operations. Demonstrated how the application
>> leverage IOAT DMA channel with vhost async APIs.
>>
>> We introduce two parameters to enable DMA acceleration for Tx
>> operations of queues:
>> -dma_type This parameter is used to specify DMA type for async
>> vhost-user net driver.
>> -dmas This parameter is used to specify the assigned DMA device of a
>> vhost device and enable async vhost data path.
>>
>> ---
>> v10:
>>   * Fixed compilation issue on specific environment
>>
>> v9:
>>   * Optimized code structure to solve compilation issue on non-x86 platforms
>>
>> v8:
>>   * Changed meson build file due to the change of ioat component name
>>
>> v7:
>>   * Improved IOAT callbacks and added some comments
>>
>> v6:
>>   * Cleand code and rebased for latest code
>>
>> v5:
>>   * Improved meson build file and fixed dependency problem
>>
>> v4:
>>   * Code rebased for latest IOAT driver
>>
>> v3:
>>   * Fixed a coding style problem
>>
>> v2:
>>   * Changed meson build file to fix dependency problem
>>   * Added parameter description in usage function
>>   * Optimized parameter settings and parsing function
>>   * Optimized abstraction, moved some code to ioat.c
>>
>> Cheng Jiang (4):
>>    example/vhost: add async vhost args parsing function
>>    example/vhost: add support for vhost async data path
>>    doc: update vhost sample doc for vhost async data path
>>    doc: update release notes for vhost sample
>>
>>   doc/guides/rel_notes/release_20_11.rst |   6 +
>>   doc/guides/sample_app_ug/vhost.rst     |  11 ++
>>   examples/vhost/ioat.c                  | 201 +++++++++++++++++++++++++
>>   examples/vhost/ioat.h                  |  45 ++++++
>>   examples/vhost/main.c                  |  93 +++++++++++-
>>   examples/vhost/main.h                  |   1 +
>>   examples/vhost/meson.build             |   5 +
>>   7 files changed, 360 insertions(+), 2 deletions(-)
>>   create mode 100644 examples/vhost/ioat.c
>>   create mode 100644 examples/vhost/ioat.h
>>
>> --
>> 2.27.0
>>
> 
> 
> Applied to dpdk-next-virtio/main.
> 

Document patches squashed into the code patch in next-net.
  

On Fri, Oct 23, 2020 at 1:23 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 10/22/20 10:59 AM, Cheng Jiang wrote:
> > This patch set makes vhost-vswitch be able to use vhost async APIs
> > for enqueue operations. Demonstrated how the application
> > leverage IOAT DMA channel with vhost async APIs.
> >
> > We introduce two parameters to enable DMA acceleration for Tx
> > operations of queues:
> > -dma_type This parameter is used to specify DMA type for async
> > vhost-user net driver.
> > -dmas This parameter is used to specify the assigned DMA device of a
> > vhost device and enable async vhost data path.
> >
> >
> > Cheng Jiang (4):
> >   example/vhost: add async vhost args parsing function
> >   example/vhost: add support for vhost async data path

- This series breaks external compilation, as the external Makefile
was not updated.

/usr/bin/ld: /tmp/cce4w26j.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/cce4w26j.o: in function `main':
main.c:(.text.startup+0x25e): undefined reference to `open_ioat'
collect2: error: ld returned 1 exit status


- This series imposes a dependency on the raw/ioat driver for no reason.

$ meson configure build -Ddisable_drivers=raw/ioat
$ ninja-build -C build -j4
...
examples/meson.build:91:4: ERROR:  Problem encountered: Missing
dependency "raw_ioat" for example "vhost"

The check for the architecture == x86 is wrong.
The example must check for RTE_RAW_IOAT presence.


Please provide fixes before rc4 or I will revert this series.
Thanks.
  

Hi,
The replies are inline.

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, November 9, 2020 8:40 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Jiang, Cheng1
> <cheng1.jiang@intel.com>
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; dev <dev@dpdk.org>; Fu, Patrick
> <patrick.fu@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v10 0/4] add async data path in vhost sample
> 
> On Fri, Oct 23, 2020 at 1:23 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
> > On 10/22/20 10:59 AM, Cheng Jiang wrote:
> > > This patch set makes vhost-vswitch be able to use vhost async APIs
> > > for enqueue operations. Demonstrated how the application leverage
> > > IOAT DMA channel with vhost async APIs.
> > >
> > > We introduce two parameters to enable DMA acceleration for Tx
> > > operations of queues:
> > > -dma_type This parameter is used to specify DMA type for async
> > > vhost-user net driver.
> > > -dmas This parameter is used to specify the assigned DMA device of a
> > > vhost device and enable async vhost data path.
> > >
> > >
> > > Cheng Jiang (4):
> > >   example/vhost: add async vhost args parsing function
> > >   example/vhost: add support for vhost async data path
> 
> - This series breaks external compilation, as the external Makefile was not
> updated.
> 

I'm not sure I understand what you mean by external Makefile, because as far as I know, makefile has been deprecated.
Do you mean I need to update the Makfile in examples/vhost?

> /usr/bin/ld: /tmp/cce4w26j.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/cce4w26j.o: in function `main':
> main.c:(.text.startup+0x25e): undefined reference to `open_ioat'
> collect2: error: ld returned 1 exit status
> 
> 
> - This series imposes a dependency on the raw/ioat driver for no reason.
> 
> $ meson configure build -Ddisable_drivers=raw/ioat $ ninja-build -C build -
> j4 ...
> examples/meson.build:91:4: ERROR:  Problem encountered: Missing
> dependency "raw_ioat" for example "vhost"
> 
> The check for the architecture == x86 is wrong.
> The example must check for RTE_RAW_IOAT presence.
> 

As for this issue, I agreed with you. I will fix it by changing the x86 check into RTE_RAW_IOAT check.

Thanks.
Cheng

> 
> Please provide fixes before rc4 or I will revert this series.
> Thanks.
> 
> 
> --
> David Marchand
  

On Tue, Nov 10, 2020 at 4:02 AM Jiang, Cheng1 <cheng1.jiang@intel.com> wrote:
> > - This series breaks external compilation, as the external Makefile was not
> > updated.
> >
>
> I'm not sure I understand what you mean by external Makefile, because as far as I know, makefile has been deprecated.

make support is dropped for dpdk compilation itself.

For the examples, the users will use make to compile them, as this is
the only way provided to users *out of* dpdk.
But the examples are compiled too via meson when compiling dpdk itself
if you pass -Dexamples= options.


Bruce,

I want to avoid more misses like this one.

If we want to keep the examples compilation in meson, we need a
consistent framework to compile in both cases.
Right now, we don't export meson for examples, and it makes no sense
in their current form.
It seems simpler to me to only maintain make support, and meson can
still call make for each example.

Another solution is to rely only on test-meson-builds.sh, but then it
ends up on the maintainer shoulders, so I prefer the solution above.

Other ideas?


Cheng,

For the problem on vhost, there are different ways to reproduce, but
the simpler for now is:
$ meson --default-library=shared builddir
$ ninja-build -C builddir -j4
$ export DESTDIR=$(pwd)/installdir
$ ninja-build -C builddir -j4 install
$ export PKG_CONFIG_PATH=$(dirname $(find $DESTDIR -name
libdpdk.pc)):$PKG_CONFIG_PATH
$ export LD_LIBRARY_PATH=$(dirname $(find $DESTDIR -name
librte_eal.so)):$LD_LIBRARY_PATH
$ export PKGCONF="pkg-config --define-prefix"
$ make -C $DESTDIR/usr/local/share/dpdk/examples/vhost clean shared static
...
rm -f build/vhost-switch build/vhost-switch-static build/vhost-switch-shared
test -d build && rmdir -p build || true
cc -O3 -I.../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.../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/ccXZVDJk.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/ccXZVDJk.o: in function `main':
main.c:(.text.startup+0x25e): undefined reference to `open_ioat'
collect2: error: ld returned 1 exit status
make: *** [Makefile:34: build/vhost-switch-shared] Error 1


> Do you mean I need to update the Makfile in examples/vhost?

Yes.


> > - This series imposes a dependency on the raw/ioat driver for no reason.
> >
> > $ meson configure build -Ddisable_drivers=raw/ioat $ ninja-build -C build -
> > j4 ...
> > examples/meson.build:91:4: ERROR:  Problem encountered: Missing
> > dependency "raw_ioat" for example "vhost"
> >
> > The check for the architecture == x86 is wrong.
> > The example must check for RTE_RAW_IOAT presence.
> >
>
> As for this issue, I agreed with you. I will fix it by changing the x86 check into RTE_RAW_IOAT check.

Reading the thread again, Maxime had already asked for this change.
  

On Tue, Nov 10, 2020 at 09:17:36AM +0100, David Marchand wrote:
> On Tue, Nov 10, 2020 at 4:02 AM Jiang, Cheng1 <cheng1.jiang@intel.com> wrote:
> > > - This series breaks external compilation, as the external Makefile was not
> > > updated.
> > >
> >
> > I'm not sure I understand what you mean by external Makefile, because as far as I know, makefile has been deprecated.
> 
> make support is dropped for dpdk compilation itself.
> 
> For the examples, the users will use make to compile them, as this is
> the only way provided to users *out of* dpdk.
> But the examples are compiled too via meson when compiling dpdk itself
> if you pass -Dexamples= options.
> 
> 
> Bruce,
> 
> I want to avoid more misses like this one.
> 
> If we want to keep the examples compilation in meson, we need a
> consistent framework to compile in both cases.
> Right now, we don't export meson for examples, and it makes no sense
> in their current form.
> It seems simpler to me to only maintain make support, and meson can
> still call make for each example.
> 
> Another solution is to rely only on test-meson-builds.sh, but then it
> ends up on the maintainer shoulders, so I prefer the solution above.
> 
> Other ideas?
> 

Hi David,

I've been thinking a bit about this since I got your email, but inspiration
for new ideas has yet to strike me.

While I can see the downside of having both make and meson build files for
the examples, I'm loath to see one of them dropped. Here is my current
thinking on it:

* We obviously need to keep the basic makefiles so as to make it easy for
  end-users to compile up and use the examples separate from DPDK itself.
  The makefiles serve as good examples as to how to pull the DPDK info from
  pkg-config.

* On the other hand, being able to build the examples as part of a regular
  meson build is a big advantage over having to build them separately, and
  allows errors with examples to be caught quicker. It's also useful for
  those of us working on specific components with associated sample apps to
  have just that one app built constantly as we work.

* Therefore, while it looks like the more logical part to drop is indeed
  the meson support for the examples, we may struggle to implement clean
  building of the examples from meson using make, at least until meson 0.54
  becomes our standard. Before that version, we don't have a
  "meson-uninstalled" folder with build-time package-config file to use as
  source of build flags for each example.

* One final idea I had and investigated in the past was whether we could at
  build or install time auto-generate the Makefile for each example from
  the meson.build file. Unfortunately, nothing came of that idea the first
  time I tried it, but it might still be worth looking at. Even if it works
  for 80-90% of cases, it means that we have a much smaller subset of
  examples where we need to test independently the make and meson builds.

So overall my assessment is that it needs a bit of investigation and
prototyping to see what we can come up with.

On a semi-related note, it's perhaps a bigger problem that we cannot rely
on test-meson-builds and CI infrastructure to prevent issues like this.
Surely this is what CI is there for - to help reduce the workload for
maintainers. The fact that we are considering removing the meson build of
examples because we cannot rely on CI is surely more worthy of a solution
than trying to find a way to build examples with make from within meson?

Perhaps we need to see about stricter measures from CI failure, e.g.
anything failing travis build automatically gets marked as changes
requested in patchwork, and the author gets an email informing them of
such?

Regards,
/Bruce
  

10/11/2020 12:19, Bruce Richardson:
> On Tue, Nov 10, 2020 at 09:17:36AM +0100, David Marchand wrote:
> > On Tue, Nov 10, 2020 at 4:02 AM Jiang, Cheng1 <cheng1.jiang@intel.com> wrote:
> > > > - This series breaks external compilation, as the external Makefile was not
> > > > updated.
> > > >
> > >
> > > I'm not sure I understand what you mean by external Makefile,
> > > because as far as I know, makefile has been deprecated.
> > 
> > make support is dropped for dpdk compilation itself.
> > 
> > For the examples, the users will use make to compile them, as this is
> > the only way provided to users *out of* dpdk.
> > But the examples are compiled too via meson when compiling dpdk itself
> > if you pass -Dexamples= options.
> > 
> > 
> > Bruce,
> > 
> > I want to avoid more misses like this one.
> > 
> > If we want to keep the examples compilation in meson, we need a
> > consistent framework to compile in both cases.
> > Right now, we don't export meson for examples, and it makes no sense
> > in their current form.
> > It seems simpler to me to only maintain make support, and meson can
> > still call make for each example.
> > 
> > Another solution is to rely only on test-meson-builds.sh, but then it
> > ends up on the maintainer shoulders, so I prefer the solution above.
> > 
> > Other ideas?
> > 
> 
> Hi David,
> 
> I've been thinking a bit about this since I got your email, but inspiration
> for new ideas has yet to strike me.
> 
> While I can see the downside of having both make and meson build files for
> the examples, I'm loath to see one of them dropped. Here is my current
> thinking on it:
> 
> * We obviously need to keep the basic makefiles so as to make it easy for
>   end-users to compile up and use the examples separate from DPDK itself.
>   The makefiles serve as good examples as to how to pull the DPDK info from
>   pkg-config.

The external compilation is part of the example, yes.

> * On the other hand, being able to build the examples as part of a regular
>   meson build is a big advantage over having to build them separately, and
>   allows errors with examples to be caught quicker.

In the past we had a Makefile which builds all examples.
If you want, it can even been called at the end of meson DPDK compilation.

>   It's also useful for those of us working on specific components
>   with associated sample apps to have just that one app built constantly
>   as we work.

I don't understand this point:
	ninja -C build && make -C examples/myexample

> * Therefore, while it looks like the more logical part to drop is indeed
>   the meson support for the examples,

Yes, building the examples from the inside build system is strange,
and hide issues.

>   we may struggle to implement clean
>   building of the examples from meson using make, at least until meson 0.54
>   becomes our standard. Before that version, we don't have a
>   "meson-uninstalled" folder with build-time package-config file to use as
>   source of build flags for each example.

We don't have to use meson at all.

> * One final idea I had and investigated in the past was whether we could at
>   build or install time auto-generate the Makefile for each example from
>   the meson.build file. Unfortunately, nothing came of that idea the first
>   time I tried it, but it might still be worth looking at. Even if it works
>   for 80-90% of cases, it means that we have a much smaller subset of
>   examples where we need to test independently the make and meson builds.

Hand-crafted Makefile is enough. They may be improved.
If we feel it is too hard, we can use another build system
in examples, like cmake.

> So overall my assessment is that it needs a bit of investigation and
> prototyping to see what we can come up with.

I think testing external build + removing build from internal meson
would be a good start to ensure quality of examples maintenance.

> On a semi-related note, it's perhaps a bigger problem that we cannot rely
> on test-meson-builds and CI infrastructure to prevent issues like this.

We can. We just have to add all examples in test-meson-builds.sh.

> Surely this is what CI is there for - to help reduce the workload for
> maintainers. The fact that we are considering removing the meson build of
> examples because we cannot rely on CI is surely more worthy of a solution
> than trying to find a way to build examples with make from within meson?

No the concern is to have all contributors work on the same
single build path.

> Perhaps we need to see about stricter measures from CI failure, e.g.
> anything failing travis build automatically gets marked as changes
> requested in patchwork, and the author gets an email informing them of
> such?

When there is a failure, authors receive an email,
and maintainers can see a red flag. I think it's OK.

The only issue was that this build path was not tested.
I think David is going to fix it by compiling all possible examples
with external make build from test-meson-builds.sh.
  

On Tue, Nov 10, 2020 at 02:37:02PM +0100, Thomas Monjalon wrote:
> 10/11/2020 12:19, Bruce Richardson:
> > On Tue, Nov 10, 2020 at 09:17:36AM +0100, David Marchand wrote:
> > > On Tue, Nov 10, 2020 at 4:02 AM Jiang, Cheng1 <cheng1.jiang@intel.com> wrote:
> > > > > - This series breaks external compilation, as the external Makefile was not
> > > > > updated.
> > > > >
> > > >
> > > > I'm not sure I understand what you mean by external Makefile,
> > > > because as far as I know, makefile has been deprecated.
> > > 
> > > make support is dropped for dpdk compilation itself.
> > > 
> > > For the examples, the users will use make to compile them, as this is
> > > the only way provided to users *out of* dpdk.
> > > But the examples are compiled too via meson when compiling dpdk itself
> > > if you pass -Dexamples= options.
> > > 
> > > 
> > > Bruce,
> > > 
> > > I want to avoid more misses like this one.
> > > 
> > > If we want to keep the examples compilation in meson, we need a
> > > consistent framework to compile in both cases.
> > > Right now, we don't export meson for examples, and it makes no sense
> > > in their current form.
> > > It seems simpler to me to only maintain make support, and meson can
> > > still call make for each example.
> > > 
> > > Another solution is to rely only on test-meson-builds.sh, but then it
> > > ends up on the maintainer shoulders, so I prefer the solution above.
> > > 
> > > Other ideas?
> > > 
> > 
> > Hi David,
> > 
> > I've been thinking a bit about this since I got your email, but inspiration
> > for new ideas has yet to strike me.
> > 
> > While I can see the downside of having both make and meson build files for
> > the examples, I'm loath to see one of them dropped. Here is my current
> > thinking on it:
> > 
> > * We obviously need to keep the basic makefiles so as to make it easy for
> >   end-users to compile up and use the examples separate from DPDK itself.
> >   The makefiles serve as good examples as to how to pull the DPDK info from
> >   pkg-config.
> 
> The external compilation is part of the example, yes.
> 
> > * On the other hand, being able to build the examples as part of a regular
> >   meson build is a big advantage over having to build them separately, and
> >   allows errors with examples to be caught quicker.
> 
> In the past we had a Makefile which builds all examples.
> If you want, it can even been called at the end of meson DPDK compilation.
> 

Yes, we can do that, but my concern is not so much about having them built
as part of build tests, but rather having them part of a build workflow for
developers during development.  Thinking here in particular of those coding
with IDEs such as eclipse or VScode, which I know a lot of people use -
including myself from time to time, depending on what I am working on.

> >   It's also useful for those of us working on specific components
> >   with associated sample apps to have just that one app built constantly
> >   as we work.
> 
> I don't understand this point:
> 	ninja -C build && make -C examples/myexample
> 

That works fine for building on the commandline, but does not work well for
building as part of "build-on-save" inside an IDE, which is the biggest
reason I want to keep support for building examples using meson. For doing
development with a feature and associated sample app, being able to
configure your build folder to rebuild a particular sample app as part of a
main infrastructure rebuild is really useful - and works really quickly
too, since incremental builds of C file changes happen really fast with
meson.

> > * Therefore, while it looks like the more logical part to drop is indeed
> >   the meson support for the examples,
> 
> Yes, building the examples from the inside build system is strange,
> and hide issues.
> 

Sorry, while it may hide issues with the makefile because people weren't
really aware that they were sticking around, it definitely helps pick up
issues with C code changes as you are developing.

> >   we may struggle to implement clean
> >   building of the examples from meson using make, at least until meson 0.54
> >   becomes our standard. Before that version, we don't have a
> >   "meson-uninstalled" folder with build-time package-config file to use as
> >   source of build flags for each example.
> 
> We don't have to use meson at all.
> 

No, we don't, so long as we don't miss out on the benefits we currently get
from having it integrated.

> > * One final idea I had and investigated in the past was whether we could at
> >   build or install time auto-generate the Makefile for each example from
> >   the meson.build file. Unfortunately, nothing came of that idea the first
> >   time I tried it, but it might still be worth looking at. Even if it works
> >   for 80-90% of cases, it means that we have a much smaller subset of
> >   examples where we need to test independently the make and meson builds.
> 
> Hand-crafted Makefile is enough. They may be improved.
> If we feel it is too hard, we can use another build system
> in examples, like cmake.
> 
> > So overall my assessment is that it needs a bit of investigation and
> > prototyping to see what we can come up with.
> 
> I think testing external build + removing build from internal meson
> would be a good start to ensure quality of examples maintenance.
> 
If we remove the meson build of the examples, I think we need equivalent
functionality provided some other way. Calling make for examples from
within meson may be good enough, so long as it's not too slow.

> > On a semi-related note, it's perhaps a bigger problem that we cannot rely
> > on test-meson-builds and CI infrastructure to prevent issues like this.
> 
> We can. We just have to add all examples in test-meson-builds.sh.
> 

Yep, definite +1 here.
Once that is done, I'd then like to wait and see what future issues crop up
before we start ripping out other bits. This is the first release where we
have removed the make build system, so there are lots of teething issues
all over the place, of which this is but one example!

> > Surely this is what CI is there for - to help reduce the workload for
> > maintainers. The fact that we are considering removing the meson build of
> > examples because we cannot rely on CI is surely more worthy of a solution
> > than trying to find a way to build examples with make from within meson?
> 
> No the concern is to have all contributors work on the same
> single build path.
> 
Building all examples from test-meson-build should probably be sufficient
here, I think.

> > Perhaps we need to see about stricter measures from CI failure, e.g.
> > anything failing travis build automatically gets marked as changes
> > requested in patchwork, and the author gets an email informing them of
> > such?
> 
> When there is a failure, authors receive an email,
> and maintainers can see a red flag. I think it's OK.
> 
> The only issue was that this build path was not tested.
> I think David is going to fix it by compiling all possible examples
> with external make build from test-meson-builds.sh.
> 

Seems like we are all aligned on the first step anyway. Let's see beyond
that what needs to be done in 21.02.

/Bruce
  

10/11/2020 15:34, Bruce Richardson:
> On Tue, Nov 10, 2020 at 02:37:02PM +0100, Thomas Monjalon wrote:
> > 10/11/2020 12:19, Bruce Richardson:
> > > On Tue, Nov 10, 2020 at 09:17:36AM +0100, David Marchand wrote:
> > > > On Tue, Nov 10, 2020 at 4:02 AM Jiang, Cheng1 <cheng1.jiang@intel.com> wrote:
> > > > > > - This series breaks external compilation, as the external Makefile was not
> > > > > > updated.
> > > > > >
> > > > >
> > > > > I'm not sure I understand what you mean by external Makefile,
> > > > > because as far as I know, makefile has been deprecated.
> > > > 
> > > > make support is dropped for dpdk compilation itself.
> > > > 
> > > > For the examples, the users will use make to compile them, as this is
> > > > the only way provided to users *out of* dpdk.
> > > > But the examples are compiled too via meson when compiling dpdk itself
> > > > if you pass -Dexamples= options.
> > > > 
> > > > 
> > > > Bruce,
> > > > 
> > > > I want to avoid more misses like this one.
> > > > 
> > > > If we want to keep the examples compilation in meson, we need a
> > > > consistent framework to compile in both cases.
> > > > Right now, we don't export meson for examples, and it makes no sense
> > > > in their current form.
> > > > It seems simpler to me to only maintain make support, and meson can
> > > > still call make for each example.
> > > > 
> > > > Another solution is to rely only on test-meson-builds.sh, but then it
> > > > ends up on the maintainer shoulders, so I prefer the solution above.
> > > > 
> > > > Other ideas?
> > > > 
> > > 
> > > Hi David,
> > > 
> > > I've been thinking a bit about this since I got your email, but inspiration
> > > for new ideas has yet to strike me.
> > > 
> > > While I can see the downside of having both make and meson build files for
> > > the examples, I'm loath to see one of them dropped. Here is my current
> > > thinking on it:
> > > 
> > > * We obviously need to keep the basic makefiles so as to make it easy for
> > >   end-users to compile up and use the examples separate from DPDK itself.
> > >   The makefiles serve as good examples as to how to pull the DPDK info from
> > >   pkg-config.
> > 
> > The external compilation is part of the example, yes.
> > 
> > > * On the other hand, being able to build the examples as part of a regular
> > >   meson build is a big advantage over having to build them separately, and
> > >   allows errors with examples to be caught quicker.
> > 
> > In the past we had a Makefile which builds all examples.
> > If you want, it can even been called at the end of meson DPDK compilation.
> > 
> 
> Yes, we can do that, but my concern is not so much about having them built
> as part of build tests, but rather having them part of a build workflow for
> developers during development.  Thinking here in particular of those coding
> with IDEs such as eclipse or VScode, which I know a lot of people use -
> including myself from time to time, depending on what I am working on.
> 
> > >   It's also useful for those of us working on specific components
> > >   with associated sample apps to have just that one app built constantly
> > >   as we work.
> > 
> > I don't understand this point:
> > 	ninja -C build && make -C examples/myexample
> > 
> 
> That works fine for building on the commandline, but does not work well for
> building as part of "build-on-save" inside an IDE, which is the biggest
> reason I want to keep support for building examples using meson. For doing
> development with a feature and associated sample app, being able to
> configure your build folder to rebuild a particular sample app as part of a
> main infrastructure rebuild is really useful - and works really quickly
> too, since incremental builds of C file changes happen really fast with
> meson.
> 
> > > * Therefore, while it looks like the more logical part to drop is indeed
> > >   the meson support for the examples,
> > 
> > Yes, building the examples from the inside build system is strange,
> > and hide issues.
> > 
> 
> Sorry, while it may hide issues with the makefile because people weren't
> really aware that they were sticking around, it definitely helps pick up
> issues with C code changes as you are developing.
> 
> > >   we may struggle to implement clean
> > >   building of the examples from meson using make, at least until meson 0.54
> > >   becomes our standard. Before that version, we don't have a
> > >   "meson-uninstalled" folder with build-time package-config file to use as
> > >   source of build flags for each example.
> > 
> > We don't have to use meson at all.
> > 
> 
> No, we don't, so long as we don't miss out on the benefits we currently get
> from having it integrated.
> 
> > > * One final idea I had and investigated in the past was whether we could at
> > >   build or install time auto-generate the Makefile for each example from
> > >   the meson.build file. Unfortunately, nothing came of that idea the first
> > >   time I tried it, but it might still be worth looking at. Even if it works
> > >   for 80-90% of cases, it means that we have a much smaller subset of
> > >   examples where we need to test independently the make and meson builds.
> > 
> > Hand-crafted Makefile is enough. They may be improved.
> > If we feel it is too hard, we can use another build system
> > in examples, like cmake.
> > 
> > > So overall my assessment is that it needs a bit of investigation and
> > > prototyping to see what we can come up with.
> > 
> > I think testing external build + removing build from internal meson
> > would be a good start to ensure quality of examples maintenance.
> > 
> If we remove the meson build of the examples, I think we need equivalent
> functionality provided some other way. Calling make for examples from
> within meson may be good enough, so long as it's not too slow.
> 
> > > On a semi-related note, it's perhaps a bigger problem that we cannot rely
> > > on test-meson-builds and CI infrastructure to prevent issues like this.
> > 
> > We can. We just have to add all examples in test-meson-builds.sh.
> > 
> 
> Yep, definite +1 here.
> Once that is done, I'd then like to wait and see what future issues crop up
> before we start ripping out other bits. This is the first release where we
> have removed the make build system, so there are lots of teething issues
> all over the place, of which this is but one example!
> 
> > > Surely this is what CI is there for - to help reduce the workload for
> > > maintainers. The fact that we are considering removing the meson build of
> > > examples because we cannot rely on CI is surely more worthy of a solution
> > > than trying to find a way to build examples with make from within meson?
> > 
> > No the concern is to have all contributors work on the same
> > single build path.
> > 
> Building all examples from test-meson-build should probably be sufficient
> here, I think.
> 
> > > Perhaps we need to see about stricter measures from CI failure, e.g.
> > > anything failing travis build automatically gets marked as changes
> > > requested in patchwork, and the author gets an email informing them of
> > > such?
> > 
> > When there is a failure, authors receive an email,
> > and maintainers can see a red flag. I think it's OK.
> > 
> > The only issue was that this build path was not tested.
> > I think David is going to fix it by compiling all possible examples
> > with external make build from test-meson-builds.sh.
> > 
> 
> Seems like we are all aligned on the first step anyway. Let's see beyond
> that what needs to be done in 21.02.

OK