"dev" <dev-boun...@openvswitch.org> wrote on 26/07/2016 12:02:37 AM:
> From: Flavio Fernandes <fla...@flaviof.com> > To: Ryan Moats <rmo...@us.ibm.com> > Cc: dev@openvswitch.org > Date: 26/07/2016 12:06 AM > Subject: Re: [ovs-dev] [ovs-dev,ovn-controller] Physical: persist tunnels > Sent by: "dev" <dev-boun...@openvswitch.org> > > > > On Jul 25, 2016, at 12:28 PM, Ryan Moats <rmo...@us.ibm.com> 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. > > > > I tested this fix by using a Vagrant file that stamps out vms as > compute nodes. > To deploy ovn, call the script ?/vagrant/scripts/setup-ovn- > cluster.sh? and that > would render ovn-controller in compute nodes to peg the cpu at 100% before the > changes. > > More info on _easily_ deploying this cluster is available here: > https://github.com/flavio-fernandes/just-ovn-nodes/blob/master/README.md > > > > [1] http://openvswitch.org/pipermail/dev/2016-July/076058.html > > > > Signed-off-by: Ryan Moats <rmo...@us.ibm.com> > > Acked-by: Flavio Fernandes <fla...@flaviof.com> > Tested-by: Flavio Fernandes <fla...@flaviof.com> > Tested this fix with a cluster of 50 hosts. Acked-by: Liran Schour <lir...@il.ibm.com> Tested-by: Liran Schour <lir...@il.ibm.com> > > > --- > > ovn/controller/physical.c | 59 ++++++++++++++++++++++++++++ > +------------------ > > 1 file changed, 37 insertions(+), 22 deletions(-) > > > > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c > > index a104e33..e788fe5 100644 > > --- a/ovn/controller/physical.c > > +++ b/ovn/controller/physical.c > > @@ -605,8 +605,13 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > > uuid_generate(hc_uuid); > > } > > > > + /* This bool tracks physical mapping changes. */ > > + bool physical_map_changed = false; > > + > > struct simap new_localvif_to_ofport = > > SIMAP_INITIALIZER(&new_localvif_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]; > > if (!strcmp(port_rec->name, br_int->name)) { > > @@ -668,18 +673,23 @@ physical_run(struct controller_ctx *ctx, > enum mf_field_id mff_ovn_geneve, > > continue; > > } > > > > - struct chassis_tunnel *tun = xmalloc(sizeof *tun); > > - hmap_insert(&tunnels, &tun->hmap_node, > > - hash_string(chassis_id, 0)); > > - tun->chassis_id = chassis_id; > > - tun->ofport = u16_to_ofp(ofport); > > - tun->type = tunnel_type; > > - full_binding_processing = true; > > - binding_reset_processing(); > > - > > - /* Reprocess logical flow table immediately. */ > > - lflow_reset_processing(); > > - poll_immediate_wake(); > > + simap_put(&new_tunnel_to_ofport, chassis_id, ofport); > > + struct chassis_tunnel *tun; > > + if ((tun = chassis_tunnel_find(chassis_id))) { > > + /* If the tunnel's ofport has changed, update. */ > > + if (tun->ofport != u16_to_ofp(ofport)) { > > + tun->ofport = u16_to_ofp(ofport); > > + physical_map_changed = true; > > + } > > + } else { > > + tun = xmalloc(sizeof *tun); > > + hmap_insert(&tunnels, &tun->hmap_node, > > + hash_string(chassis_id, 0)); > > + tun->chassis_id = chassis_id; > > + tun->ofport = u16_to_ofp(ofport); > > + tun->type = tunnel_type; > > + physical_map_changed = true; > > + } > > break; > > } else { > > const char *iface_id = smap_get(&iface_rec->external_ids, > > @@ -691,29 +701,38 @@ physical_run(struct controller_ctx *ctx, > enum mf_field_id mff_ovn_geneve, > > } > > } > > > > + /* Remove tunnels that are no longer here. */ > > + struct chassis_tunnel *tun, *tun_next; > > + HMAP_FOR_EACH_SAFE (tun, tun_next, hmap_node, &tunnels) { > > + if (!simap_find(&new_tunnel_to_ofport, tun->chassis_id)) { > > + hmap_remove(&tunnels, &tun->hmap_node); > > + physical_map_changed = true; > > + free(tun); > > + } > > + } > > + > > /* Capture changed or removed openflow ports. */ > > - bool localvif_map_changed = false; > > struct simap_node *vif_name, *vif_name_next; > > SIMAP_FOR_EACH_SAFE (vif_name, vif_name_next, &localvif_to_ofport) { > > int newport; > > if ((newport = simap_get(&new_localvif_to_ofport, > vif_name->name))) { > > if (newport != simap_get(&localvif_to_ofport, vif_name->name)) { > > simap_put(&localvif_to_ofport, vif_name->name, newport); > > - localvif_map_changed = true; > > + physical_map_changed = true; > > } > > } else { > > simap_find_and_delete(&localvif_to_ofport, vif_name->name); > > - localvif_map_changed = true; > > + physical_map_changed = true; > > } > > } > > SIMAP_FOR_EACH (vif_name, &new_localvif_to_ofport) { > > if (!simap_get(&localvif_to_ofport, vif_name->name)) { > > simap_put(&localvif_to_ofport, vif_name->name, > > simap_get(&new_localvif_to_ofport, vif_name->name)); > > - localvif_map_changed = true; > > + physical_map_changed = true; > > } > > } > > - if (localvif_map_changed) { > > + if (physical_map_changed) { > > full_binding_processing = true; > > > > /* Reprocess logical flow table immediately. */ > > @@ -769,7 +788,6 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > > * ports. We set MFF_LOG_DATAPATH, MFF_LOG_INPORT, and > > * MFF_LOG_OUTPORT from the tunnel key data, then resubmit to table > > * 33 to handle packets to the local hypervisor. */ > > - struct chassis_tunnel *tun; > > HMAP_FOR_EACH (tun, hmap_node, &tunnels) { > > struct match match = MATCH_CATCHALL_INITIALIZER; > > match_set_in_port(&match, tun->ofport); > > @@ -858,10 +876,7 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > > ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts, hc_uuid); > > > > ofpbuf_uninit(&ofpacts); > > - HMAP_FOR_EACH_POP (tun, hmap_node, &tunnels) { > > - free(tun); > > - } > > - hmap_clear(&tunnels); > > > > simap_destroy(&new_localvif_to_ofport); > > + simap_destroy(&new_tunnel_to_ofport); > > } > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
