On Fri, Mar 11, 2016 at 03:06:18PM -0600, Ryan Moats wrote: > From: RYAN D. MOATS <rmo...@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 <rmo...@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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
