Ben Pfaff <b...@ovn.org> wrote on 06/03/2016 10:50:45 AM:
> From: Ben Pfaff <b...@ovn.org> > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: dev@openvswitch.org > Date: 06/03/2016 10:50 AM > Subject: Re: [ovs-dev,v17,4/5] Persist ovn flow tables. > > On Sun, May 22, 2016 at 04:36:21PM -0500, Ryan Moats wrote: > > Ensure that ovn flow tables are persisted so that changes to > > them chan be applied incrementally - this is a prereq for > > making lflow_run and physical_run incremental. > > > > Signed-off-by: Ryan Moats <rmo...@us.ibm.com> > > I don't really understand the design here. > > One way that I find it difficult to understand is that ofctrl_add_flow() > has a new 'is_new' parameter that appears to be important, but every > call I see to ofctrl_add_flow() passes 'true' as the argument. What's going on here is that is_new *will* be set to false in the _TRACKED branches of the next patch set. However, given the size of that patch set, I pulled the is_new modifications into the patch set. I'm open to suggestion on how to bundle all of this into more sane patch sets... > I don't understand the hashing scheme. Partly this is due to the > comment here: > > * Because it is possible for both actions and matches to change on a rule, > * and because the hmap struct only supports a single hash, this method > * uses two hash maps - one that uses table_id+priority+matches for its hash > * and the other that uses table_id+priority+actions. > > I see the hash table based on table_id+priority+matches, > 'match_flow_table'. I don't see a hash table that looks at > table_id+priority+actions, and I don't understand why that would be > useful. I do see a hash table based on a uuid; is that the one you > mean? Yes, that is my mistake. I was originally hashing on actions, but that is too coarse a hash mechanism and so I later switched to uuid - I will update the comment when I respin the patch. > A hash table based on a uuid will not be unique, because multiple > OpenFlow flows can be generated from a single logical flow and all of > those will have the same UUID (the logical flow's UUID). An hmap is not > a good hash table when duplicates are likely; instead, use an hindex. I will switch this to an hindex in the next respin, but I'll ask, does the logic otherwise make sense? Thanks, Ryan _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
