[ovs-dev] [PATCH 2/5] ovn-controller: Pass around pointers to individual tables.

Ben Pfaff blp at ovn.org
Mon Jun 11 22:23:45 UTC 2018


On Mon, Jun 11, 2018 at 02:59:01PM -0700, Han Zhou wrote:
> On Mon, Jun 11, 2018 at 2:01 PM, Ben Pfaff <blp at ovn.org> wrote:
> >
> > On Mon, Jun 11, 2018 at 11:23:58AM -0700, Han Zhou wrote:
> > > On Fri, Jun 8, 2018 at 2:59 PM, Ben Pfaff <blp at ovn.org> wrote:
> > > >
> > > > We're working to make ovn-controller compute more incrementally, to
> reduce
> > > > CPU usage.  To make it easier to keep track of dependencies, it makes
> > > sense
> > > > to pass around pointers to fine-grained resources instead of an entire
> > > > database at a time.  This commit introduces a way to pass individual
> > > tables
> > > > around and starts using that feature in ovn-controller.
> > > >
> > > > CC: Han Zhou <zhouhan at gmail.com>
> > > > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > > > ---
> > >
> > > In this patch although tables are passed individually, the ctx
> parameter is
> > > still kept for most functions. With ctx, it is still easy for the
> function
> > > to access any tables than the individual tables passed.
> > > I see two reasons why ctx is still kept in this patch:
> > > 1) index implementation requires passing the IDL, and I saw this is
> > > addressed in next patches by redesigning the index mechanism
> > > 2) OVSDB transaction requires passing the ..._idl_txn in the ctx. I
> think
> > > we can pass in ..._idl_txn directly instead of the ctx. This would avoid
> > > the misuse of tables.
> > >
> > > What do you think about point 2)?
> >
> > Eliminating ctx is a goal but this series doesn't accomplish it yet.  I
> > hope to do that in a followup.  Passing in idl_txn is an easy way to do
> > it; I was hoping to somehow distinguish read-only tables from read/write
> > ones (maybe via const?) but I haven't gotten that far.
> 
> Ok. I think distinguish read (ready-only and read/write) and write-only
> makes sense, because write-only tables are not regarded as inputs. However,
> I am not sure how to ensure this, i.e. verify that we are not reading the
> tables that are declared in the parameter as write-only.
> 
> Another point is, how to reflect the indirect dependency, e.g. bfd_run()
> doesn't has table "ovsrec_port" as input, but in fact it depends on this
> table by referencing from ovsrec_bridge->ports.
> 
> I agree that these don't need to be addressed in current patch. so
> 
> Acked-by: Han Zhou <hzhou8 at ebay.com>

Thanks.  I applied this to master.

I sent out v2 that eliminates controller_ctx in the last two patches,
which are new:
        https://patchwork.ozlabs.org/project/openvswitch/list/?series=49614&state=%2A&archive=both


More information about the dev mailing list