[ovs-dev] [PATCH] testsuite: ofproto-dpif: fix sockets dependencies

[Flavio Leitner]

On Thu, 2 Apr 2015 13:47:56 -0700
Ben Pfaff <blp at nicira.com> wrote:

> On Thu, Apr 02, 2015 at 05:45:34PM -0300, Flavio Leitner wrote:
> > On Thu, 2 Apr 2015 13:37:06 -0700
> > Ben Pfaff <blp at nicira.com> wrote:
> > 
> > > On Thu, Apr 02, 2015 at 05:33:25PM -0300, Flavio Leitner wrote:
> > > > On Thu, 2 Apr 2015 12:58:37 -0700
> > > > Ben Pfaff <blp at nicira.com> wrote:
> > > > 
> > > > > On Thu, Apr 02, 2015 at 03:57:20PM -0300, Flavio Leitner
> > > > > wrote:
> > > > > > The ofproto-dpif creates dummies backed by sockets so
> > > > > > depending on the order of execution when bridge is
> > > > > > reconfiguring, an active socket may run first and not find
> > > > > > the file.  That is usually not a problem because it will
> > > > > > try to reconnect one second later. However, it breaks the
> > > > > > testsuite.
> > > > > > 
> > > > > > This patch fixes the issue splitting active and passive
> > > > > > sockets in different vsctl-ctl commands that guarantees the
> > > > > > proper ordering between them.
> > > > > 
> > > > > WAIT_FOR_DUMMY_PORTS is supposed to avoid this problem, by
> > > > > waiting until the ports have connected.  Is it busted?
> > > > 
> > > > No, but it takes at least one extra second for the port to
> > > > reconnect. If we consider the four tests fixed by the patch, it
> > > > can be 4 extra seconds to complete the same tests, so I'd just
> > > > go with the proposed patch.
> > > 
> > > OK, I buy that, but in that case the commit message goes overboard
> > > when it says that the current form "breaks the testsuite".  Can
> > > you rephrase this as an optimization rather than a bug fix, then?
> > 
> > Not really because it is a bug fix.  Those tests break like 7 out
> > of 10 times on a s390x.
> 
> OK, so WAIT_FOR_DUMMY_PORTS is buggy then, can we figure out why and
> fix it?  Optimizations are fine too but I'd like to get to the root of
> the problem.

Ok, I missed that WAIT_FOR_DUMMY_PORTS is not available on branch-2.3
where it breaks most of the times.  So, I think the correct would be
to backport these patches to branch-2.3 first:
 
93fa0de tests: Fix race in 'balance-tcp bonding' test.
60187ac test: Remove explicit sleeps from ofproto-dpif bond tests
d611e23 test: add WAIT_FOR_DUMMY_PORTS helper macro for writing tests

and then hopefully my patch becomes just an optimization.  Anyway,
I can do that next week if you or someone else didn't do it before me.

Thanks Ben!
fbl