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). Ryan _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
