[ovs-dev] [ovs-dev,v17,4/5] Persist ovn flow tables.

Ryan Moats rmoats at us.ibm.com
Mon Jun 6 15:25:32 UTC 2016



Ben Pfaff <blp at ovn.org> wrote on 06/03/2016 10:50:45 AM:

> From: Ben Pfaff <blp at ovn.org>
> To: Ryan Moats/Omaha/IBM at IBMUS
> Cc: dev at 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 <rmoats at 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



More information about the dev mailing list