Re: [dpdk-dev] [PATCH v11 12/19] net/ipn3ke: remove configuration for i40e port bonding

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

On Mon, Oct 21, 2019 at 03:23:09PM +0800, Ye Xiaolong wrote:
> +Bruce for the meson change.
> 
> Thanks,
> Xiaolong
> 
> On 10/21, Andy Pei wrote:
> >From: Rosen Xu <rosen.xu@intel.com>
> >
> >The ipn3ke board FPGA and i40e BDF scan has added in ifpga_rawdev,
> >so it doesn't need to provide configuration for i40e port bonding.
> >
> >Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> >Signed-off-by: Andy Pei <andy.pei@intel.com>
> >---
> > drivers/meson.build                            |   7 +-
> > drivers/net/ipn3ke/Makefile                    |   2 +
> > drivers/net/ipn3ke/ipn3ke_ethdev.c             | 289 +++----------------------
> > drivers/net/ipn3ke/ipn3ke_representor.c        |   8 +-
> > drivers/net/ipn3ke/meson.build                 |   2 +-
> > drivers/raw/ifpga/meson.build                  |   6 +
> > drivers/raw/ifpga/rte_rawdev_ifpga_version.map |   6 +
> > 7 files changed, 62 insertions(+), 258 deletions(-)
> >
> >diff --git a/drivers/meson.build b/drivers/meson.build
> >index 2ed2e95..afdbb3b 100644
> >--- a/drivers/meson.build
> >+++ b/drivers/meson.build
> >@@ -9,12 +9,12 @@ endif
> > dpdk_driver_classes = ['common',
> > 	       'bus',
> > 	       'mempool', # depends on common and bus.
> >+               'raw',
> > 	       'net',     # depends on common, bus and mempool.
> > 	       'crypto',  # depends on common, bus and mempool (net in future).
> > 	       'compress', # depends on common, bus, mempool.
> > 	       'event',   # depends on common, bus, mempool and net.
> >-	       'baseband', # depends on common and bus.
> >-	       'raw']     # depends on common, bus, mempool, net and event.
> >+	       'baseband']
> > 

This needs an explanation in the commit log, and possibly should be a
separate patch. The comment on "net" need to be updated explaining why it
comes after raw.

> > default_cflags = machine_args
> > if cc.has_argument('-Wno-format-truncation')
> >@@ -157,6 +157,9 @@ foreach class:dpdk_driver_classes
> > 
> > 			set_variable('shared_@0@'.format(lib_name), shared_dep)
> > 			set_variable('static_@0@'.format(lib_name), static_dep)
> >+			dependency_name = ''.join(lib_name.split('rte_'))
> >+			message('drivers/@0@: Defining dependency "@1@"'.format(
> >+					drv_path, dependency_name))
> > 		endif # build
> > 	endforeach
> > 

This doesn't belong in this patch. It's covered by:
http://patches.dpdk.org/patch/59470/, so feel free to ack that patch
instead.

> >diff --git a/drivers/net/ipn3ke/Makefile b/drivers/net/ipn3ke/Makefile
> >index 8c3ae37..2c65e49 100644

<snip>

> >diff --git a/drivers/net/ipn3ke/meson.build b/drivers/net/ipn3ke/meson.build
> >index 74b4d7c..4ea57b9 100644
> >--- a/drivers/net/ipn3ke/meson.build
> >+++ b/drivers/net/ipn3ke/meson.build
> >@@ -14,4 +14,4 @@ sources += files('ipn3ke_ethdev.c',
> > 	'ipn3ke_representor.c',
> > 	'ipn3ke_tm.c',
> > 	'ipn3ke_flow.c')
> >-deps += ['bus_ifpga', 'sched']
> >+deps += ['bus_ifpga', 'sched', 'pmd_i40e', 'rawdev', 'rawdev_ifpga']

Minor nit: rawdev_ifpga already depends on rawdev and buf_ifpga, so you can
drop them to shorten the list if you like.
	deps += ['sched', 'pmd_i40e', 'rawdev_ifpga']

> >diff --git a/drivers/raw/ifpga/meson.build b/drivers/raw/ifpga/meson.build
> >index 0ab6fd7..dbd74b9 100644
> >--- a/drivers/raw/ifpga/meson.build
> >+++ b/drivers/raw/ifpga/meson.build
> >@@ -8,13 +8,19 @@ objs = [base_objs]
> > 
> > dep = dependency('libfdt', required: false)
> > if not dep.found()
> >+	dep = cc.find_library('libfdt', required: false)
> >+endif
> >+if not dep.found()
> > 	build = false
> > 	reason = 'missing dependency, "libfdt"'
> > endif
> > deps += ['rawdev', 'pci', 'bus_pci', 'kvargs',
> > 	'bus_vdev', 'bus_ifpga', 'net']

Similarly, if you want to cut this list down: 'kvargs' is always depended
upon by everything since its a dependency of eal, every rawdev already
depends upon the rawdev library and bus_pci depends upon pci.

	deps += ['bus_pci', 'bus_vdev', 'bus_ifpga', 'net']

should work on it's own.

Cutting the dependency list can help speed up configuration a little as fewer
dependency chains need to be iterated.