[v1] app/test-pmd: fix meson build failed when enabled pmd_bonded

Message ID 20200910015525.59124-1-stevex.yang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v1] app/test-pmd: fix meson build failed when enabled pmd_bonded |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Steve Yang Sept. 10, 2020, 1:55 a.m. UTC
  meson build cannot find the header rte_eth_bond.h when build DPDK first
time or never installed DPDK lib after build via meson/ninja.

Because the corresponding header directory isn't included after enabled
RTE_LIBRTE_PMD_BOND macro.

Add the header file location and link library to meson.build of test-pmd

Signed-off-by: SteveX Yang <stevex.yang@intel.com>
---
 app/test-pmd/meson.build | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Bruce Richardson Sept. 10, 2020, 9:16 a.m. UTC | #1
On Thu, Sep 10, 2020 at 01:55:25AM +0000, SteveX Yang wrote:
> meson build cannot find the header rte_eth_bond.h when build DPDK first
> time or never installed DPDK lib after build via meson/ninja.
> 
> Because the corresponding header directory isn't included after enabled
> RTE_LIBRTE_PMD_BOND macro.
> 
> Add the header file location and link library to meson.build of test-pmd
> 
> Signed-off-by: SteveX Yang <stevex.yang@intel.com>
> ---

While there is a problem here, I think the solution may itself be
incomplete. Since the bonding PMD is always enabled by default, it's
support should always be enabled in testpmd. Therefore, testpmd should be
using the RTE_LIBRTE_BOND_PMD macro internally, not RTE_LIBRTE_PMD_BOND
one.  Once that macro is changed, the issue you report becomes generally
visible.

>  app/test-pmd/meson.build | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build
> index ea56e547b..db0ff02eb 100644
> --- a/app/test-pmd/meson.build
> +++ b/app/test-pmd/meson.build
> @@ -4,6 +4,11 @@
>  # override default name to drop the hyphen
>  name = 'testpmd'
>  cflags += '-Wno-deprecated-declarations'
> +
> +if dpdk_conf.has('RTE_LIBRTE_BOND_PMD')
> +	cflags += '-I' + meson.source_root() + '/drivers/net/bonding'
> +endif
> +

Don't need this block. Adding the dependency automatically pulls in header
paths.

>  sources = files('5tswap.c',
>  	'cmdline.c',
>  	'cmdline_flow.c',
> @@ -25,6 +30,11 @@ sources = files('5tswap.c',
>  	'util.c')
>  
>  deps += ['ethdev', 'gro', 'gso', 'cmdline', 'metrics', 'meter', 'bus_pci']
> +
> +if dpdk_conf.has('RTE_LIBRTE_BOND_PMD')
> +	deps += 'pmd_bond'
> +endif
> +

The code is right, but just note that the other blocks like this aren't
separated by whitespace. Since they are quite readable as they are, I'd
avoid adding in the unnecessary whitespace and just keep it consistent.

>  if dpdk_conf.has('RTE_LIBRTE_PDUMP')
>  	deps += 'pdump'
>  endif
> -- 
> 2.17.1
>
  
Bruce Richardson Sept. 10, 2020, 11:12 a.m. UTC | #2
On Thu, Sep 10, 2020 at 10:16:57AM +0100, Bruce Richardson wrote:
> On Thu, Sep 10, 2020 at 01:55:25AM +0000, SteveX Yang wrote:
> > meson build cannot find the header rte_eth_bond.h when build DPDK first
> > time or never installed DPDK lib after build via meson/ninja.
> > 
> > Because the corresponding header directory isn't included after enabled
> > RTE_LIBRTE_PMD_BOND macro.
> > 
> > Add the header file location and link library to meson.build of test-pmd
> > 
> > Signed-off-by: SteveX Yang <stevex.yang@intel.com>
> > ---
> 
> While there is a problem here, I think the solution may itself be
> incomplete. Since the bonding PMD is always enabled by default, it's
> support should always be enabled in testpmd. Therefore, testpmd should be
> using the RTE_LIBRTE_BOND_PMD macro internally, not RTE_LIBRTE_PMD_BOND
> one.  Once that macro is changed, the issue you report becomes generally
> visible.
> 
I also think a release note entry about the macro change would be good.
Similarly for the latency stats one, depending on the fix chosen.
  

Patch

diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build
index ea56e547b..db0ff02eb 100644
--- a/app/test-pmd/meson.build
+++ b/app/test-pmd/meson.build
@@ -4,6 +4,11 @@ 
 # override default name to drop the hyphen
 name = 'testpmd'
 cflags += '-Wno-deprecated-declarations'
+
+if dpdk_conf.has('RTE_LIBRTE_BOND_PMD')
+	cflags += '-I' + meson.source_root() + '/drivers/net/bonding'
+endif
+
 sources = files('5tswap.c',
 	'cmdline.c',
 	'cmdline_flow.c',
@@ -25,6 +30,11 @@  sources = files('5tswap.c',
 	'util.c')
 
 deps += ['ethdev', 'gro', 'gso', 'cmdline', 'metrics', 'meter', 'bus_pci']
+
+if dpdk_conf.has('RTE_LIBRTE_BOND_PMD')
+	deps += 'pmd_bond'
+endif
+
 if dpdk_conf.has('RTE_LIBRTE_PDUMP')
 	deps += 'pdump'
 endif