[ovs-dev] [ovs-dev, v9, 03/10] Make flow table persistent in ovn controller

[Ben Pfaff]

On Fri, Mar 11, 2016 at 03:06:18PM -0600, Ryan Moats wrote:
> From: RYAN D. MOATS <rmoats at us.ibm.com>
> 
> This is a prerequisite for incremental processing.
> 
> Side effects:
> 
> 1. Table rows are now tracked so that removed rows are correctly
>    handled.
> 2. Hash by table id+priority+action added to help detect superseded
>    flows.
> 3. Hash by insert seqno added to help find deleted flows.
> 
> Signed-off-by: RYAN D. MOATS <rmoats at us.ibm.com>

This had a number of patch rejects so I'm reviewing it on the basis of
what I see, without trying to build or test it.  Therefore it's a pretty
superficial review; I'll look at it more carefully when it is rebased.

There's a number of instances of code similar to this, but I really
haven't got a clue what it really does or where the number 4 comes from:
> +        // this offset is to protect the hard coded rules in physical.c
> +        ins_seqno += 4;

It looks like ofpacts_hash() could just be a wrapper around
hash_bytes().

It seems like the main change here is to make lflow_run() iterate over
only the logical flows that have changed.  But I don't see how that can
work as-is, because OpenFlow flows are not a pure function of the
logical flows; rather, they have other inputs, such as the mapping from
logical ports to OpenFlow port numbers.  I don't see anything here that
makes sure that logical flows are revisited if this mapping changes.
Maybe this is in a later patch in the series, but we ordinarily require
each commit to be correct as a self-contained unit.