Liran Schour/Haifa/IBM wrote on 05/24/2016 03:34:36 AM: > From: Liran Schour/Haifa/IBM > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: "Ben Pfaff" <b...@ovn.org>, ovs-dev <dev@openvswitch.org> > Date: 05/24/2016 03:34 AM > Subject: Re: [ovs-dev] [PATCH monitor_cond V6 11/11] RFC OVN: Quick > implementation of conditional monitoring > > Ryan Moats/Omaha/IBM wrote on 23/05/2016 04:53:00 PM: > > > "dev" <dev-boun...@openvswitch.org> wrote on 05/23/2016 03:48:39 AM: > > > > > From: "Liran Schour" <lir...@il.ibm.com> > > > To: "Ben Pfaff" <b...@ovn.org> > > > Cc: ovs-dev <dev@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-boun...@openvswitch.org> > > > > > > Liran Schour/Haifa/IBM@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 <lir...@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.gitbranch: > > > 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); > } Ok, now I see - somehow, the version of code I have is missing those four lines. Once I put these in then the test case passes (and I no longer feel like I'm going crazy... or at least concerning this) > > 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 I'll let Ben weigh in on this, but I think I'd prefer to get the current patch set applied and address this as a followon optimization. Ryan _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
