[ovs-dev] [PATCH 2/2] tests: Fix threading race in "ofproto-dpif megaflow - learning" test.

[Ben Pfaff]

Good idea, I added:
# We send each packet twice because the first packet in each flow causes the
# flow table to change and thus revalidations, which (depending on timing)
# can keep a megaflow from being installed.  The revalidations are done by
# the second iteration, allowing the flows to be installed.
and I will shortly push these.

Thanks,

Ben.

On Tue, Aug 13, 2013 at 12:21:49PM -0700, Justin Pettit wrote:
> Looks good.  Do you think it's worth adding a comment explaining this?
> 
> --Justin
> 
> 
> On Aug 13, 2013, at 11:28 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > Threaded ofproto-dpif uses a queue to pass packets from the forwarding
> > threads to the main thread for (mega)flow setup and for learning.  When
> > learning occurs, causing revalidations, this races against flow setup, so
> > that sometimes a datapath (mega)flow does get set up for a packet that
> > causes learning and sometimes it doesn't.  This caused this test to
> > sometimes fail because one megaflow or the other that was expected to be
> > set up was not.
> > 
> > This commit fixes the problem by sending a second packet in each flow.
> > These additional packets don't cause any additional changes to the flow
> > table but they do cause flows to be set up, fixing the problem.
> > 
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > tests/ofproto-dpif.at |    7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> > index 46e1dea..2e3e8f6 100644
> > --- a/tests/ofproto-dpif.at
> > +++ b/tests/ofproto-dpif.at
> > @@ -2581,8 +2581,11 @@ AT_DATA([flows.txt], [dnl
> > table=0 in_port=1 actions=load:2->NXM_NX_REG0[[0..15]],learn(table=1,priority=65535,NXM_OF_ETH_SRC[[]],NXM_OF_VLAN_TCI[[0..11]],output:NXM_NX_REG0[[0..15]]),output:2
> > ])
> > AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> > -AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> > -AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> > +for i in 1 2; do
> > +    AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> > +    AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> > +    ovs-appctl time/warp 100
> > +done
> > dnl The original flow is missing due to a revalidation.
> > AT_CHECK([ovs-appctl dpif/dump-megaflows br0 | STRIP_XOUT], [0], [dnl
> > skb_priority=0,ip,in_port=1,vlan_tci=0x0000/0x0fff,dl_src=50:54:00:00:00:09,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> > -- 
> > 1.7.10.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>