On Mon, Jul 25, 2016 at 04:28:52PM +0000, Ryan Moats wrote: > While commit ab39371d68842b7e4000cc5d8718e6fc04e92795 > (ovn-controller: Handle physical changes correctly) addressed > unit test failures, it did so at the cost of performance: [1] > notes that ovn-controller cpu usage is now pegged at 100%. > > Root cause of this is that while the storage for tunnels is > persisted, their creation is not (which the above changed > incorrectly assumed was the case). This patch persists > tunneled data across invocations of physical_run. A side > effect is that renaming of localfvif_map_changed variable > to physical_map_changed and extending its scope to include > tunnel changes. > > [1] http://openvswitch.org/pipermail/dev/2016-July/076058.html > > Signed-off-by: Ryan Moats <rmo...@us.ibm.com>
Thanks for the fix. I applied this to master, folding in the following incremental which is mostly style but I think that the extra check on tun->type is a bug fix albeit for a corner case. Please take a look. --8<--------------------------cut here-------------------------->8-- diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index e788fe5..e5df4f2 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -610,7 +610,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, struct simap new_localvif_to_ofport = SIMAP_INITIALIZER(&new_localvif_to_ofport); - struct simap new_tunnel_to_ofport = + struct simap new_tunnel_to_ofport = SIMAP_INITIALIZER(&new_tunnel_to_ofport); for (int i = 0; i < br_int->n_ports; i++) { const struct ovsrec_port *port_rec = br_int->ports[i]; @@ -674,11 +674,13 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, } simap_put(&new_tunnel_to_ofport, chassis_id, ofport); - struct chassis_tunnel *tun; - if ((tun = chassis_tunnel_find(chassis_id))) { + struct chassis_tunnel *tun = chassis_tunnel_find(chassis_id); + if (tun) { /* If the tunnel's ofport has changed, update. */ - if (tun->ofport != u16_to_ofp(ofport)) { + if (tun->ofport != u16_to_ofp(ofport) || + tun->type != tunnel_type) { tun->ofport = u16_to_ofp(ofport); + tun->type = tunnel_type; physical_map_changed = true; } } else { _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev