app/test: fix build with ring pmd but no bond pmd

Message ID 20200527145745.4060-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series app/test: fix build with ring pmd but no bond pmd |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-testing warning Testing issues
ci/Intel-compilation fail Compilation issues

Commit Message

Bruce Richardson May 27, 2020, 2:57 p.m. UTC
  If the bonding pmd is disabled, all autotest associated with it should be
disabled. However, some of those tests also depended upon the ring PMD so
were placed in a block depending on that driver - and unfortunately that
driver alone. This caused build failures if the ring PMD was enabled but
the bonding PMD disabled, due to missing header files and driver libs.

This error can be reproduced by configuring DPDK using e.g.

  meson configure -Ddisable_drivers=net/[!r]* build

(which will disable all drivers not starting with "r"), and then building
using ninja.

Fix this by moving all link bonding autotests to the one block and putting
a second conditional check within that block for those also requiring the
ring PMD.

Fixes: 7f6ef1664027 ("test/bonding: allow disabling driver")
Fixes: 207b1c813f39 ("test: fix build without ring PMD")
Cc: reshma.pattan@intel.com

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/meson.build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
  

Comments

David Marchand June 29, 2020, 10:09 a.m. UTC | #1
On Wed, May 27, 2020 at 4:58 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> If the bonding pmd is disabled, all autotest associated with it should be
> disabled. However, some of those tests also depended upon the ring PMD so
> were placed in a block depending on that driver - and unfortunately that
> driver alone. This caused build failures if the ring PMD was enabled but
> the bonding PMD disabled, due to missing header files and driver libs.
>
> This error can be reproduced by configuring DPDK using e.g.
>
>   meson configure -Ddisable_drivers=net/[!r]* build
>
> (which will disable all drivers not starting with "r"), and then building
> using ninja.
>
> Fix this by moving all link bonding autotests to the one block and putting
> a second conditional check within that block for those also requiring the
> ring PMD.
>
> Fixes: 7f6ef1664027 ("test/bonding: allow disabling driver")
> Fixes: 207b1c813f39 ("test: fix build without ring PMD")
> Cc: reshma.pattan@intel.com
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  app/test/meson.build | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 1715ddbcb..dc2474699 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -350,6 +350,10 @@ if dpdk_conf.has('RTE_LIBRTE_BOND_PMD')
>         test_deps += 'pmd_bond'
>         test_sources += ['test_link_bonding.c', 'test_link_bonding_rssconf.c']
>         driver_test_names += ['link_bonding_autotest', 'link_bonding_rssconf_autotest']
> +       if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> +               test_sources += 'test_link_bonding_mode4.c'
> +               driver_test_names += 'link_bonding_mode4_autotest'
> +       endif
>  endif
>  if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
>         test_deps += 'pmd_ring'
> @@ -358,7 +362,6 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
>         test_sources += 'test_event_eth_tx_adapter.c'
>         test_sources += 'test_bitratestats.c'
>         test_sources += 'test_latencystats.c'
> -       test_sources += 'test_link_bonding_mode4.c'
>         test_sources += 'sample_packet_forward.c'
>         test_sources += 'test_pdump.c'
>         fast_tests += [['ring_pmd_autotest', true]]
> @@ -366,7 +369,6 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
>         fast_tests += [['event_eth_tx_adapter_autotest', false]]
>         fast_tests += [['bitratestats_autotest', true]]
>         fast_tests += [['latencystats_autotest', true]]
> -       driver_test_names += 'link_bonding_mode4_autotest'
>         fast_tests += [['pdump_autotest', true]]
>  endif
>
> --
> 2.25.1
>

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

Just a note.
Looking at the makefile counterpart, I can see that if we disable the
null driver, we would have an issue too.
  
Bruce Richardson June 29, 2020, 1:03 p.m. UTC | #2
On Mon, Jun 29, 2020 at 12:09:20PM +0200, David Marchand wrote:
> On Wed, May 27, 2020 at 4:58 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > If the bonding pmd is disabled, all autotest associated with it should be
> > disabled. However, some of those tests also depended upon the ring PMD so
> > were placed in a block depending on that driver - and unfortunately that
> > driver alone. This caused build failures if the ring PMD was enabled but
> > the bonding PMD disabled, due to missing header files and driver libs.
> >
> > This error can be reproduced by configuring DPDK using e.g.
> >
> >   meson configure -Ddisable_drivers=net/[!r]* build
> >
> > (which will disable all drivers not starting with "r"), and then building
> > using ninja.
> >
> > Fix this by moving all link bonding autotests to the one block and putting
> > a second conditional check within that block for those also requiring the
> > ring PMD.
> >
> > Fixes: 7f6ef1664027 ("test/bonding: allow disabling driver")
> > Fixes: 207b1c813f39 ("test: fix build without ring PMD")
> > Cc: reshma.pattan@intel.com
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  app/test/meson.build | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test/meson.build b/app/test/meson.build
> > index 1715ddbcb..dc2474699 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -350,6 +350,10 @@ if dpdk_conf.has('RTE_LIBRTE_BOND_PMD')
> >         test_deps += 'pmd_bond'
> >         test_sources += ['test_link_bonding.c', 'test_link_bonding_rssconf.c']
> >         driver_test_names += ['link_bonding_autotest', 'link_bonding_rssconf_autotest']
> > +       if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> > +               test_sources += 'test_link_bonding_mode4.c'
> > +               driver_test_names += 'link_bonding_mode4_autotest'
> > +       endif
> >  endif
> >  if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> >         test_deps += 'pmd_ring'
> > @@ -358,7 +362,6 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> >         test_sources += 'test_event_eth_tx_adapter.c'
> >         test_sources += 'test_bitratestats.c'
> >         test_sources += 'test_latencystats.c'
> > -       test_sources += 'test_link_bonding_mode4.c'
> >         test_sources += 'sample_packet_forward.c'
> >         test_sources += 'test_pdump.c'
> >         fast_tests += [['ring_pmd_autotest', true]]
> > @@ -366,7 +369,6 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> >         fast_tests += [['event_eth_tx_adapter_autotest', false]]
> >         fast_tests += [['bitratestats_autotest', true]]
> >         fast_tests += [['latencystats_autotest', true]]
> > -       driver_test_names += 'link_bonding_mode4_autotest'
> >         fast_tests += [['pdump_autotest', true]]
> >  endif
> >
> > --
> > 2.25.1
> >
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 
> Just a note.
> Looking at the makefile counterpart, I can see that if we disable the
> null driver, we would have an issue too.
> 
I'm not actually seeing an error with it in meson builds. Running a quick
test using build parameter "-Ddisable_drivers=net/null" shows no errors, so
I think we may be ok here. 

Regards,
/Bruce
  
David Marchand July 1, 2020, 7:50 a.m. UTC | #3
On Mon, Jun 29, 2020 at 3:05 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > Just a note.
> > Looking at the makefile counterpart, I can see that if we disable the
> > null driver, we would have an issue too.
> >
> I'm not actually seeing an error with it in meson builds. Running a quick
> test using build parameter "-Ddisable_drivers=net/null" shows no errors, so
> I think we may be ok here.

I would expect an issue at runtime, as the bonding driver test wants
to use null ports.
But yes, the build is fine.
  
David Marchand July 1, 2020, 9:11 a.m. UTC | #4
On Mon, Jun 29, 2020 at 12:09 PM David Marchand
<david.marchand@redhat.com> wrote:
> On Wed, May 27, 2020 at 4:58 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > If the bonding pmd is disabled, all autotest associated with it should be
> > disabled. However, some of those tests also depended upon the ring PMD so
> > were placed in a block depending on that driver - and unfortunately that
> > driver alone. This caused build failures if the ring PMD was enabled but
> > the bonding PMD disabled, due to missing header files and driver libs.
> >
> > This error can be reproduced by configuring DPDK using e.g.
> >
> >   meson configure -Ddisable_drivers=net/[!r]* build
> >
> > (which will disable all drivers not starting with "r"), and then building
> > using ninja.
> >
> > Fix this by moving all link bonding autotests to the one block and putting
> > a second conditional check within that block for those also requiring the
> > ring PMD.
> >
> > Fixes: 7f6ef1664027 ("test/bonding: allow disabling driver")
> > Fixes: 207b1c813f39 ("test: fix build without ring PMD")
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>

Applied, thanks.
  

Patch

diff --git a/app/test/meson.build b/app/test/meson.build
index 1715ddbcb..dc2474699 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -350,6 +350,10 @@  if dpdk_conf.has('RTE_LIBRTE_BOND_PMD')
 	test_deps += 'pmd_bond'
 	test_sources += ['test_link_bonding.c', 'test_link_bonding_rssconf.c']
 	driver_test_names += ['link_bonding_autotest', 'link_bonding_rssconf_autotest']
+	if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
+		test_sources += 'test_link_bonding_mode4.c'
+		driver_test_names += 'link_bonding_mode4_autotest'
+	endif
 endif
 if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
 	test_deps += 'pmd_ring'
@@ -358,7 +362,6 @@  if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
 	test_sources += 'test_event_eth_tx_adapter.c'
 	test_sources += 'test_bitratestats.c'
 	test_sources += 'test_latencystats.c'
-	test_sources += 'test_link_bonding_mode4.c'
 	test_sources += 'sample_packet_forward.c'
 	test_sources += 'test_pdump.c'
 	fast_tests += [['ring_pmd_autotest', true]]
@@ -366,7 +369,6 @@  if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
 	fast_tests += [['event_eth_tx_adapter_autotest', false]]
 	fast_tests += [['bitratestats_autotest', true]]
 	fast_tests += [['latencystats_autotest', true]]
-	driver_test_names += 'link_bonding_mode4_autotest'
 	fast_tests += [['pdump_autotest', true]]
 endif