[ovs-dev] [PATCH 04/11] ofproto-dpif.at: Fix some races in megaflow tests

[YAMAMOTO Takashi]

> On 7 April 2014 20:12, YAMAMOTO Takashi <yamamoto at valinux.co.jp> wrote:
> 
>> > I've been looking at some races in the megaflow testcases recently as
>> well,
>> > with a slightly different approach, could you a look at my patch below?
>> >
>> > http://openvswitch.org/pipermail/dev/2014-April/038536.html
>>
>> my patch inserts sleeps between two netdev-dummy/receive calls,
>> where it's expected that the former packet inserts flow and the
>> latter one hits it.  i don't think your patch helps this case.
>>
>> in general, your patch doesn't eliminate the need of "sleep 1"
>> because it still have a race with dispatcher and upcall handler
>> threads, does it?
> 
> 
> I see, that patch changes what we are testing. If the correct set of flows
> are installed at all, then the test passes. It does not check that one
> packet causes flow installation, then one packet hits it. I think that for
> the most part, the megaflow tests are checking that flows with the correct
> masks are installed, and different packets do not cause flows to be
> installed with incorrect masks. Correct me if I'm wrong, but I think that
> grepping logs for flow installation messages should cover this case, right?

grepping logs should be ok.  but i'm not sure if it's an improvement
over the current method unless it eliminates races.

let me explain my understanding.
netdev-dummy/receive merely puts the packet on the queue.
after involving some of threads, finally the upcall handler
thread installs the flow and thus output the logs which you grep.
(well, vlog_enable_async can make the situation even worse.)
so you still need some "sleep" between netdev-dummy/receive and grep.

YAMAMOTO Takashi

> 
> There are some tests that this does not assist, for instance as you say, if
> you want to check that one packet causes the flow to be installed, and
> checking that a subsequent packet hits the same flow.