[ovs-dev] [PATCH monitor_cond V6 11/11] RFC OVN: Quick implementation of conditional monitoring

[Liran Schour]

Ryan Moats/Omaha/IBM wrote on 23/05/2016 04:53:00 PM:

> "dev" <dev-bounces at openvswitch.org> wrote on 05/23/2016 03:48:39 AM:
> 
> > From: "Liran Schour" <LIRANS at il.ibm.com>
> > To: "Ben Pfaff" <blp at ovn.org>
> > Cc: ovs-dev <dev at openvswitch.org>
> > Date: 05/23/2016 03:49 AM
> > Subject: Re: [ovs-dev] [PATCH monitor_cond V6 11/11] RFC OVN: Quick 
> > implementation of conditional monitoring
> > Sent by: "dev" <dev-bounces at openvswitch.org>
> > 
> > Liran Schour/Haifa/IBM at IBMIL wrote on 17/05/2016 05:22:33 PM:
> > 
> > > Conditional monitor of: Port_Binding, Logical_Flow, Multicast_Group
> > > MAC_Binding tables. As a result ovn-controller will be notified only 

> > about
> > > records belongs to a datapath that is being served by this 
hypervisor.
> > > 
> > > Hack: Ideally, logical datapath ID should be retrieved from 
Port_Binding 
> > table
> > > and by that conditions should be composed only from logical datapath 

> > IDs.
> > > In the meantime we first add the logical port to the conditions and 
> > > on retrieval
> > > of port binding record, we add the logical datapath to the 
conditions.
> > > 
> > > ovn-bridge-mappings unit test fails due to the fact that patch ports 

> > will not
> > > be created on local bridge if there is no port binded to the logical
> > > datapath on
> > > the local host.
> > > 
> > > Signed-off-by: Liran Schour <lirans at il.ibm.com>
> > 
> > Pushed a tiny fix to this patch to pass ovn-bridge-mappings unit test.
> > Updated code is pushed to: https://github.com/liranschour/ovs.git 
branch: 
> > monitor_cond_ovn.
> 
> That patch *almost* gets us the rest of the way there - that test case
> switches between the current database and an empty database in lines 
134-136
> as a test of the database switching code - unfortunately, it looks like
> the patch ports aren't being recreated.  That has the potential to cause
> headaches later on...

At my environment it passes this unit test.
Look at the following lines at ovn-controller.c:

@@ -337,6 +354,12 @@ main(int argc, char *argv[])
             free(ovnsb_remote);
             ovnsb_remote = new_ovnsb_remote;
             ovsdb_idl_set_remote(ovnsb_idl_loop.idl, ovnsb_remote, true);
+
+            /* Update existing conditions */
+            ovsdb_idl_cond_update(ovnsb_idl_loop.idl, &binding_cond);
+            ovsdb_idl_cond_update(ovnsb_idl_loop.idl, &lflow_cond);
+            ovsdb_idl_cond_update(ovnsb_idl_loop.idl, &mgroup_cond);
+            ovsdb_idl_cond_update(ovnsb_idl_loop.idl, &mac_binding_cond);
         } else {
             free(new_ovnsb_remote);
         }

These lines re-update the existing conditions on the new monitor session 
with the new database and ensure that we will get the right updates.

However, on a second look it seems that it is a bug in ovsdb-idl.c in 
ovsdb_idl_cond_update() and the above lines are just a work a round.

I pushed the proper change to the next patch series. It is available here: 
https://github.com/liranschour/ovs.git Branch: monitor_cond_v7_ovn