diff mbox

[ovs-dev] tests: make ovn logical router test case more reliable

Message ID 1465236180-12352-1-git-send-email-lrichard@redhat.com
State Accepted
Headers show

Commit Message

Lance Richardson June 6, 2016, 6:03 p.m. UTC
The "ovn -- 1 HVs, 2 LSs, 1 lport/LS, 1 LR" test case creates a
configuration including a logical router, then:
    1) Sends a packet that is expected to be forwarded by the
       logical router.
    2) Disables the logical router.
    3) Sends another packet, identical to the one sent in (1), that
       should not be forwarded.

This test case fails intermittently, apparently because the disabling
of the logical router in (2) has not yet been propagated to the
forwarding plane at the time the second packet is sent. (When the
failure occurs, two packets are captured whereas only one is expected.)

Address this issue by adding a one second sleep between steps (2) and
(3). Adding a sleep does not actually fix anything, but it
does make this test case more likely to work correctly.

In one series of tests, this test case failed 11 times out of 20
without this fix and succeeded 20 times out of 20 attempts with
this fix.

Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
 tests/ovn.at | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Lance Richardson June 9, 2016, 1:44 p.m. UTC | #1
A Fixes: tag might be appropriate:

Fixes: f295c17bc704 ("ovn: Implement basic end-to-end full mesh test.")

Also, here are some recent travis-ci failures due to the issue addressed
by this patch (all of these occurred within the last 24 hours):

    https://travis-ci.org/openvswitch/ovs/jobs/136264257
    https://travis-ci.org/openvswitch/ovs/jobs/136234273
    https://travis-ci.org/openvswitch/ovs/jobs/136013834
    https://travis-ci.org/openvswitch/ovs/jobs/136012103
    https://travis-ci.org/openvswitch/ovs/jobs/135984940

Regards,

    Lance

----- Original Message -----
> From: "Lance Richardson" <lrichard@redhat.com>
> To: dev@openvswitch.org
> Sent: Monday, June 6, 2016 2:03:00 PM
> Subject: [ovs-dev] [PATCH] tests: make ovn logical router test case more	reliable
> 
> The "ovn -- 1 HVs, 2 LSs, 1 lport/LS, 1 LR" test case creates a
> configuration including a logical router, then:
>     1) Sends a packet that is expected to be forwarded by the
>        logical router.
>     2) Disables the logical router.
>     3) Sends another packet, identical to the one sent in (1), that
>        should not be forwarded.
> 
> This test case fails intermittently, apparently because the disabling
> of the logical router in (2) has not yet been propagated to the
> forwarding plane at the time the second packet is sent. (When the
> failure occurs, two packets are captured whereas only one is expected.)
> 
> Address this issue by adding a one second sleep between steps (2) and
> (3). Adding a sleep does not actually fix anything, but it
> does make this test case more likely to work correctly.
> 
> In one series of tests, this test case failed 11 times out of 20
> without this fix and succeeded 20 times out of 20 attempts with
> this fix.
> 
> Signed-off-by: Lance Richardson <lrichard@redhat.com>
> ---
>  tests/ovn.at | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 059c969..d353143 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -2334,6 +2334,10 @@ echo "---------------------"
>  echo "------ hv1 dump ----------"
>  as hv1 ovs-ofctl dump-flows br-int
>  
> +# Allow some time for the disabling of logical router R1 to propagate.
> +# XXX This should be more systematic.
> +sleep 1
> +
>  as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
>  
>  # Packet to Expect
> --
> 2.5.5
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
Lance Richardson June 9, 2016, 3:40 p.m. UTC | #2
Sorry, wrong commit ID for Fixes: tag, it should be:

Fixes: 5412db307420 ("ovn: Add column enabled to table Logical_Router")


----- Original Message -----
> From: "Lance Richardson" <lrichard@redhat.com>
> To: dev@openvswitch.org, "Ben Pfaff" <blp@ovn.org>
> Sent: Thursday, June 9, 2016 9:44:31 AM
> Subject: Re: [ovs-dev] [PATCH] tests: make ovn logical router test case more	reliable
> 
> A Fixes: tag might be appropriate:
> 
> Fixes: f295c17bc704 ("ovn: Implement basic end-to-end full mesh test.")
> 
> Also, here are some recent travis-ci failures due to the issue addressed
> by this patch (all of these occurred within the last 24 hours):
> 
>     https://travis-ci.org/openvswitch/ovs/jobs/136264257
>     https://travis-ci.org/openvswitch/ovs/jobs/136234273
>     https://travis-ci.org/openvswitch/ovs/jobs/136013834
>     https://travis-ci.org/openvswitch/ovs/jobs/136012103
>     https://travis-ci.org/openvswitch/ovs/jobs/135984940
> 
> Regards,
> 
>     Lance
> 
> ----- Original Message -----
> > From: "Lance Richardson" <lrichard@redhat.com>
> > To: dev@openvswitch.org
> > Sent: Monday, June 6, 2016 2:03:00 PM
> > Subject: [ovs-dev] [PATCH] tests: make ovn logical router test case more
> > 	reliable
> > 
> > The "ovn -- 1 HVs, 2 LSs, 1 lport/LS, 1 LR" test case creates a
> > configuration including a logical router, then:
> >     1) Sends a packet that is expected to be forwarded by the
> >        logical router.
> >     2) Disables the logical router.
> >     3) Sends another packet, identical to the one sent in (1), that
> >        should not be forwarded.
> > 
> > This test case fails intermittently, apparently because the disabling
> > of the logical router in (2) has not yet been propagated to the
> > forwarding plane at the time the second packet is sent. (When the
> > failure occurs, two packets are captured whereas only one is expected.)
> > 
> > Address this issue by adding a one second sleep between steps (2) and
> > (3). Adding a sleep does not actually fix anything, but it
> > does make this test case more likely to work correctly.
> > 
> > In one series of tests, this test case failed 11 times out of 20
> > without this fix and succeeded 20 times out of 20 attempts with
> > this fix.
> > 
> > Signed-off-by: Lance Richardson <lrichard@redhat.com>
> > ---
> >  tests/ovn.at | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 059c969..d353143 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -2334,6 +2334,10 @@ echo "---------------------"
> >  echo "------ hv1 dump ----------"
> >  as hv1 ovs-ofctl dump-flows br-int
> >  
> > +# Allow some time for the disabling of logical router R1 to propagate.
> > +# XXX This should be more systematic.
> > +sleep 1
> > +
> >  as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
> >  
> >  # Packet to Expect
> > --
> > 2.5.5
> > 
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> > 
>
Lance Richardson June 21, 2016, 5:50 p.m. UTC | #3
Any feedback? I see that there are still numerous occurrences of the failure
addressed by this patch.

Thanks,

   Lance

----- Original Message -----
> From: "Lance Richardson" <lrichard@redhat.com>
> To: dev@openvswitch.org, "Ben Pfaff" <blp@ovn.org>, nazhu@cn.ibm.com
> Sent: Thursday, June 9, 2016 11:40:49 AM
> Subject: Re: [ovs-dev] [PATCH] tests: make ovn logical router test case	more	reliable
> 
> Sorry, wrong commit ID for Fixes: tag, it should be:
> 
> Fixes: 5412db307420 ("ovn: Add column enabled to table Logical_Router")
> 
> 
> ----- Original Message -----
> > From: "Lance Richardson" <lrichard@redhat.com>
> > To: dev@openvswitch.org, "Ben Pfaff" <blp@ovn.org>
> > Sent: Thursday, June 9, 2016 9:44:31 AM
> > Subject: Re: [ovs-dev] [PATCH] tests: make ovn logical router test case
> > more	reliable
> > 
> > A Fixes: tag might be appropriate:
> > 
> > Fixes: f295c17bc704 ("ovn: Implement basic end-to-end full mesh test.")
> > 
> > Also, here are some recent travis-ci failures due to the issue addressed
> > by this patch (all of these occurred within the last 24 hours):
> > 
> >     https://travis-ci.org/openvswitch/ovs/jobs/136264257
> >     https://travis-ci.org/openvswitch/ovs/jobs/136234273
> >     https://travis-ci.org/openvswitch/ovs/jobs/136013834
> >     https://travis-ci.org/openvswitch/ovs/jobs/136012103
> >     https://travis-ci.org/openvswitch/ovs/jobs/135984940
> > 
> > Regards,
> > 
> >     Lance
> > 
> > ----- Original Message -----
> > > From: "Lance Richardson" <lrichard@redhat.com>
> > > To: dev@openvswitch.org
> > > Sent: Monday, June 6, 2016 2:03:00 PM
> > > Subject: [ovs-dev] [PATCH] tests: make ovn logical router test case more
> > > 	reliable
> > > 
> > > The "ovn -- 1 HVs, 2 LSs, 1 lport/LS, 1 LR" test case creates a
> > > configuration including a logical router, then:
> > >     1) Sends a packet that is expected to be forwarded by the
> > >        logical router.
> > >     2) Disables the logical router.
> > >     3) Sends another packet, identical to the one sent in (1), that
> > >        should not be forwarded.
> > > 
> > > This test case fails intermittently, apparently because the disabling
> > > of the logical router in (2) has not yet been propagated to the
> > > forwarding plane at the time the second packet is sent. (When the
> > > failure occurs, two packets are captured whereas only one is expected.)
> > > 
> > > Address this issue by adding a one second sleep between steps (2) and
> > > (3). Adding a sleep does not actually fix anything, but it
> > > does make this test case more likely to work correctly.
> > > 
> > > In one series of tests, this test case failed 11 times out of 20
> > > without this fix and succeeded 20 times out of 20 attempts with
> > > this fix.
> > > 
> > > Signed-off-by: Lance Richardson <lrichard@redhat.com>
> > > ---
> > >  tests/ovn.at | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/tests/ovn.at b/tests/ovn.at
> > > index 059c969..d353143 100644
> > > --- a/tests/ovn.at
> > > +++ b/tests/ovn.at
> > > @@ -2334,6 +2334,10 @@ echo "---------------------"
> > >  echo "------ hv1 dump ----------"
> > >  as hv1 ovs-ofctl dump-flows br-int
> > >  
> > > +# Allow some time for the disabling of logical router R1 to propagate.
> > > +# XXX This should be more systematic.
> > > +sleep 1
> > > +
> > >  as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
> > >  
> > >  # Packet to Expect
> > > --
> > > 2.5.5
> > > 
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > http://openvswitch.org/mailman/listinfo/dev
> > > 
> > 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
Russell Bryant June 21, 2016, 6:06 p.m. UTC | #4
On Tue, Jun 21, 2016 at 1:50 PM, Lance Richardson <lrichard@redhat.com>
wrote:

> Any feedback? I see that there are still numerous occurrences of the
> failure
> addressed by this patch.
>

This seems like a harmless workaround that improves test failure rate.  I
applied it to master.
diff mbox

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index 059c969..d353143 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -2334,6 +2334,10 @@  echo "---------------------"
 echo "------ hv1 dump ----------"
 as hv1 ovs-ofctl dump-flows br-int
 
+# Allow some time for the disabling of logical router R1 to propagate.
+# XXX This should be more systematic.
+sleep 1
+
 as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
 
 # Packet to Expect