On Fri, Aug 26, 2016 at 04:15:47PM -0500, Ryan Moats wrote: > > > Ben Pfaff <b...@ovn.org> wrote on 08/26/2016 03:27:56 PM: > > > From: Ben Pfaff <b...@ovn.org> > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: dev@openvswitch.org > > Date: 08/26/2016 03:28 PM > > Subject: Re: [ovs-dev,v5] ovn-controller: Back out incremental processing > > > > On Fri, Aug 26, 2016 at 01:30:59PM +0000, Ryan Moats wrote: > > > As [1] indicates, incremental processing hasn't resulted > > > in an improvement worth the complexity it has added. > > > This patch backs out all of the code specific to incremental > > > processing, along with the persisting of OF flows, > > > logical ports, multicast groups, all_lports, local and patched > > > datapaths > > > > > > [1] http://openvswitch.org/pipermail/dev/2016-August/078272.html > > > > > > Signed-off-by: Ryan Moats <rmo...@us.ibm.com> > > > > Thanks for working on this. I think that it is the best approach for > > now. I'm working on a few ideas about ways to build systematic and > > reliable incremental update, but they won't be ready for 2.6. > > > > There is still some incremental behavior in a few places. I am not sure > > whether this is intentional or if you just missed them. Can you comment > > on these? > > > > - Remaining _FOR_EACH_TRACKED usages in encaps_run(). Actually it's a > > little surprising that encaps still maintains static state, should > > it? > > I purposely left this file alone because of I haven't been 100% > sure how to correctly back out the changes from commit 9263ceaf > ("ovn-controller: Make encap processing more robust against changes."). > Coupling that with the fact that the problems that have been reported > have *not* been with this file since that patch set, I made the decision > to leave it alone for this patch set and address it in a future patch set. > > > - lflow.c has a persistent collection of address sets. > > I view persisting data as a necessary step towards running incrementally, > but I don't tie the two together (I can run full computations on persisted > data so long as the persistence is correct). The reason for backing out > the persistence that was backed out was the persistence isn't correct. > In the case of address sets, we've not see any errors in our testing, nor > have there been any reported on the mailing list, so I left it as is in > this patch set and revisit unpersisting it in a follow-on patch set. > > > - physical.c persists lots of state. > > This one was just a goof - I thought I had unpersisted state here in v4 > of the patch set but obviously that piece of the change got lost somewhere > along the line... > > I admit that I should have been more clear about what was going on in > binding.c and lflow.c in the commit message and I apologize for that > mistake. > > My question to you is: do you want a v6 of this patch that includes > unpersisting things in lflow.c and physical.c or can those show up > as a follow-on patch set (I still plan on leaving undoing binding.c > to a follow-on patch set).
I'd like a v6 with whatever was missing from physical.c, but we can take the others separately. I wanted to make sure that these were intentional and not overlooked. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
