[ovs-dev] [PATCH monitor_cond V3 01/10] ovsdb: create column index mapping between ovsdb row to monitor row

Liran Schour LIRANS at il.ibm.com
Tue Feb 9 14:41:22 UTC 2016


Liran Schour/Haifa/IBM wrote on 09/02/2016 08:19:45 AM:

> From: Liran Schour/Haifa/IBM
> To: Andy Zhou <azhou at ovn.org>
> Cc: "<dev at openvswitch.org>"<dev at openvswitch.org>
> Date: 09/02/2016 08:19 AM
> Subject: Re: [ovs-dev] [PATCH monitor_cond V3 01/10] ovsdb: create 
> column index mapping between ovsdb row to monitor row
> 
> Andy Zhou <azhou at ovn.org> wrote on 09/02/2016 01:33:12 AM:
> 
> 
> > On Mon, Feb 8, 2016 at 12:43 PM, Liran Schour <LIRANS at il.ibm.com> 
wrote:
> > Andy Zhou <azhou at ovn.org> wrote on 08/02/2016 09:41:47 PM:
> > 
> > > On Sun, Feb 7, 2016 at 12:08 AM, Liran Schour <LIRANS at il.ibm.com> 
wrote:
> > > Andy Zhou <azhou at ovn.org> wrote on 04/02/2016 11:54:06 PM:
> > >
> > > > On Sun, Jan 31, 2016 at 11:03 PM, Liran Schour <lirans at il.ibm.com>
> > > wrote:
> > > > Columns indexing is different in ovsdb_row then in 
ovsdb_monitor_row.
> > > > We need mapping between the 2 for condition evaluation.
> > > >
> > > > signed-off-by: Liran Schour <lirans at il.ibm.com>
> > > >
> > > > It may be simpler to and more efficient to have  ovsdb_clause to
> > > > have pointer to ovsdb_monitor_column* than
> > > > to ovsdb_column directly.  We can add a field in
> > > > ovsdb_monitor_column to store the index for accessing row data.
> > > >
> > > > struct ovsdb_caluse {
> > > >          enum ovsdb_function function;
> > > >          const struct ovsdb_monitor_column *column;
> > > >          struct ovsdb_datum arg
> > > > };
> > > >
> > > > If this approach works, then we don't need to maintain the index 
map.
> > > >
> > >
> > > Ovsdb_clause and ovsdb_condition are being used originally to 
evaluate
> > > conditions on ovsdb_row. Because of that ovsdb_clause do needs the 
field
> > > ovsdb_column.
> > > I agree that the index map array does not looks too good but the 
option
> > to
> > > overcome it as I see it is to duplicate the code in condition.c for
> > > ovsdb_monitor_row that will have ovsdb_monitor_clause with
> > > ovsdb_monitor_column field. What do you think?
> > >
> > > Yes, I am concerned with idea of using index map array to bridge API
> > gaps.
> > >
> > > How about the following:
> > >
> > > struct ovsdb_clause {
> > >         enum ovsdb_function function;
> > >         const struct ovsdb_column * column;
> > >         unsigned int index;    /* column Index into ovsdb_datum for
> > > evaluation.*/
> > >         struct ovsdb_datum arg;
> > > }
> > >
> > > Without monitor_cond,  the new 'index' will be the same
> > > column->index.  monitor_cond
> > > can maintain the 'index' field with the same logic as index map. The
> > > condition API can
> > > mostly be reused, No?
> > >

> > Yes it can be reused. At default index will be initialize to 
column->index
> > and if the caller calls to ovsdb_condition_set_index(condition*,
> > index_map[]) after condition creation. Indexes will be set to the
> > index_map values. I will go with this option to simplify the condition
> > API. (but still needs to maintain index_map in monitor.c)

> > I wonder how difficult it would be to hide the index assignment 
> > within ovsdb_monitor_add_column? 
> > Thanks
> > 

> It is not straightforward, the caller has no correlation between the
> condition's clause to the added column.
> I will go with the option of adding the API: 
> ovsdb_condition_set_index(condition*, index_map[]) that is 
> optionally called after condition creation and set the index field 
> in the ovsdb_clause. 

On second thought, ovsdb_clause is a field in ovsdb_condition. We have at 
least 2 different conditions and each update operation we have a new 
condition. If we want to adjust the index inside the ovsdb_clause 
structure, we need to do it on each clause on each condition and that is 
more expansive then maintain index map array (that anyhow we do need to 
do) and pass it to ovsdb_condition_evaluate_or_datum() only. In this way 
all other code that calls the ordinary ovsdb_condition_evaluate() stays 
intact and we stay efficient.
Do you see a way to overcome it?




More information about the dev mailing list