[ovs-dev] [PATCH] [ovn-controller] [RFC] Poor man's lflow incremental processing

[Ryan Moats]

Aha! that makes a lot more sense, but unfortunately, I don't think I'm
grokking the code yet:

1. When using the baseling of SBREC_LOGICAL_FLOW_FOR_EACH, at startup, each
lflow_run pass
handles 12 flows and 12 matches...

2. When I tried using ovsdb_idl_track_add_all and
SBREC_LOGICAL_FLOW_FOR_EACH_TRACKED,
then at startup each lflow_run pass handles 13 flows and spits out warnings
about unparsable match strings,
which worries me mightily, but honestly, the column I care about is
"match", which leads to...

3. Looking through the code I can't quite see how to translate from an
ovsdb_idl structure to a column that I can
use in ovsdb_idl_track_add_column - it looks like I have to reparse the ovs
schema file and look up the table
and column via that - is that really the case or am I just missing
something obvious (again)?

Thanks in advance,
Ryan

Ben Pfaff <blp at ovn.org> wrote on 02/08/2016 11:27:12 AM:

> From: Ben Pfaff <blp at ovn.org>
> To: Ryan Moats/Omaha/IBM at IBMUS
> Cc: dev at openvswitch.org
> Date: 02/08/2016 11:27 AM
> Subject: Re: [ovs-dev] [PATCH] [ovn-controller] [RFC] Poor man's
> lflow incremental processing
>
> On Mon, Feb 08, 2016 at 10:34:05AM -0600, Ryan Moats wrote:
> > > > I think this is because lflow_run() marks a logical datapath
"clean"
> > > > after processing any one logical flow in the datapath.  I don't
know
> > why
> > > > it does that, but on the face of it it seems wrong.
> > > >
> > >
> > > I'll definitely revisit that as that was *not* the intent...
> >
> > Yes, that is a definite mistake and the more I look at this, the more I
> > think
> > it just isn't going to work correctly the way I've tried to code it.
> >
> > Trying to mark logical flows as dirty either means changing how the IDL
> > code
> > is generated (which doesn't strike me as a good idea) or adding a
"dirty"
> > column to the Logical_Flows table, which then means I'm looking at
> > modifying
> > the ovsdb_idl_txn_write__ code to touch this column appropriately and
then
> > adding code into lflow_run() method to clear this column (which can be
> > done,
> > but still feels a little crufty).
> >
> > Ben, does this analysis look right to you or am I missing a third path?
>
> I'd consider making use of the ovsdb-idl support for change tracking.
> See this commit:
>
> From 932104f483ef4384d15dec1d26661da8da58de8d Mon Sep 17 00:00:00 2001
> From: Shad Ansari <shad.ansari at hp.com>
> Date: Tue, 27 Oct 2015 13:55:35 -0700
> Subject: [PATCH] ovsdb-idl: Add support for change tracking.
>
> Ovsdb-idl notifies a client that something changed; it does not track
> which table, row changed in what way (insert, modify or delete).
> As a result, a client has to scan or reconfigure the entire idl after
> ovsdb_idl_run(). This is presumably fine for typical ovs schemas where
> tables are relatively small. In use-cases where ovsdb is used with
> schemas that can have very large tables, the current ovsdb-idl
> notification mechanism does not appear to scale - clients need to do a
> lot of processing to determine the exact change delta.
>
> This change adds support for:
>  - Table and row based change sequence numbers to record the
>    most recent IDL change sequence numbers associated with insert,
>    modify or delete update on that table or row.
>  - Change tracking of specific columns. This ensures that changed
>    rows (inserted, modified, deleted) that have tracked columns, are
>    tracked by IDL. The client can directly access the changed rows
>    with get_first, get_next operations without the need to scan the
>    entire table.
>    The tracking functionality is not enabled by default and needs to
>    be turned on per-column by the client after ovsdb_idl_create()
>    and before ovsdb_idl_run().
>
>      /* Example Usage */
>
>      idl = ovsdb_idl_create(...);
>
>      /* Track specific columns */
>      ovsdb_idl_track_add_column(idl, column);
>      /* Or, track all columns */
>      ovsdb_idl_track_add_all(idl);
>
>      for (;;) {
>          ovsdb_idl_run(idl);
>          seqno = ovsdb_idl_get_seqno(idl);
>
>          /* Process only the changed rows in Table FOO */
>          FOO_FOR_EACH_TRACKED(row, idl) {
>              /* Determine the type of change from the row seqnos */
>              if (foo_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE)
>                     >= seqno)) {
>                  printf("row deleted\n");
>              } else if (foo_row_get_seqno(row, OVSDB_IDL_CHANGE_MODIFY)
>                            >= seqno))
>                  printf("row modified\n");
>              } else if (foo_row_get_seqno(row, OVSDB_IDL_CHANGE_INSERT)
>                            >= seqno))
>                  printf("row inserted\n");
>              }
>          }
>
>          /* All changes processed - clear the change track */
>          ovsdb_idl_track_clear(idl);
>     }
>
> Signed-off-by: Shad Ansari <shad.ansari at hp.com>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
>
> --
> 2.1.3
>