[ovs-dev] [PATCH RFC] ofproto-dpif-upcall: Increment dump_seq before instead of after dump.

Joe Stringer joestringer at nicira.com
Wed Jul 8 03:13:58 UTC 2015


On 6 July 2015 at 22:20, Ben Pfaff <blp at nicira.com> wrote:
> When process_upcall() passes a miss upcall to upcall_xlate(), the new
> ukey's dump_seq member is initialized from the current dump_seq.  Later,
> when udpif_revalidator() calls revalidate(), any dumped flow for which
> ukey->dump_seq equals the current dump_seq is skipped.  However, until now
> the current dump_seq was only incremented *after* a revalidation run is
> completed.  That means that, if a ukey added is added between revalidation
> runs, it will be skipped on the subsequent revalidation run.  This commit
> fixes the problem by incrementing dump_seq just before a revalidation run
> instead of just after.

I think this change makes sense. In cases with few flows (or generally
low revalidator workload), it makes stats and forwarding behaviour
more accurate, then in cases with high revalidator workload there
should be no effect (the end of one round melds into the start of the
next).

The few areas that I can think of which are relevant are:
- ofproto-dpif's run() function uses udpif_dump_seq() to determine
when all the stats have just finished being updated, indicating it's a
good time to do periodic work like timing out openflow flows and
rebalancing bonds. It seems like this would be best performed between
revalidation rounds, but I don't have a good way of quantifying how
much this matters.
- In the testsuite we use 'ovs-appctl revalidator/wait' (which is
based on dump_seq) to determine when the flows have been dumped and
stats updated, which will be triggered at the end of revalidation
currently. This change would make it more likely that the testsuite
calls revalidator/wait while the revalidators are sleeping, and
returns before the flows are actually dumped. This may or may not be a
problem in practice, unfortunately the testuite only shows up this
kind of issue as intermittent failures on various tests. Anyway, this
could be trivially solved if it shows to be a problem.
- The race conditions mentioned in ukey_install() for the userspace
datapath are more likely, due to there being a smaller window between
dump_seq being incremented and revalidators reaching the GC stage. In
practice I doubt it's a problem.



More information about the dev mailing list