On Sun, Jul 24, 2016 at 2:33 PM, Ryan Moats <rmo...@us.ibm.com> wrote:
> The various fixes in handling physical and binding changes > have addressed the problems that the original attempt to > incrementally process the multicast group table ran into. > So, re-add incremental processing of the multicast group > table. > > Notes: > - This patch renders the short term changes in [1] taken > for multicast group processing unnecessary (and so it > conflicts with [1]). > - Running through the ovn unit tests 50 times with -j4 > didn't show increased test case failures over the > baseline code prior to adding this patch. > > [1] http://patchwork.ozlabs.org/patch/652117/ > > Signed-off-by: Ryan Moats <rmo...@us.ibm.com> > short of nitpicks below... Acked-by: Flavio Fernandes <fla...@flaviof.com> > --- > ovn/controller/physical.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c > index a104e33..b10baed 100644 > --- a/ovn/controller/physical.c > +++ b/ovn/controller/physical.c > @@ -61,11 +61,13 @@ static struct hmap tunnels = > HMAP_INITIALIZER(&tunnels); > /* UUID to identify OF flows not associated with ovsdb rows. */ > static struct uuid *hc_uuid = NULL; > static bool full_binding_processing = false; > +static bool full_mcgroup_processing = false; > > void > physical_reset_processing(void) > { > full_binding_processing = true; > + full_mcgroup_processing = true; > } > > /* Maps from a chassis to the OpenFlow port number of the tunnel that can > be > @@ -675,6 +677,7 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > tun->ofport = u16_to_ofp(ofport); > tun->type = tunnel_type; > nitpick... > full_binding_processing = true; > + full_mcgroup_processing = true; > how about replacing the new line and the line above with: physical_reset_processing(); > binding_reset_processing(); > > /* Reprocess logical flow table immediately. */ > @@ -715,6 +718,7 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > } > if (localvif_map_changed) { > > full_binding_processing = true; > + full_mcgroup_processing = true; > same as previous comment: just call physical_reset_processing(); > > /* Reprocess logical flow table immediately. */ > lflow_reset_processing(); > @@ -751,9 +755,25 @@ physical_run(struct controller_ctx *ctx, enum > mf_field_id mff_ovn_geneve, > const struct sbrec_multicast_group *mc; > struct ofpbuf remote_ofpacts; > ofpbuf_init(&remote_ofpacts, 0); > - SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->ovnsb_idl) { > - consider_mc_group(mff_ovn_geneve, ct_zones, > - local_datapaths, mc, &ofpacts, &remote_ofpacts); > + if (full_mcgroup_processing) { > + SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->ovnsb_idl) { > + consider_mc_group(mff_ovn_geneve, ct_zones, > + local_datapaths, mc, &ofpacts, > &remote_ofpacts); > + } > + full_mcgroup_processing = false; > + } else { > + SBREC_MULTICAST_GROUP_FOR_EACH_TRACKED (mc, ctx->ovnsb_idl) { > + if (sbrec_multicast_group_is_deleted(mc)) { > + ofctrl_remove_flows(&mc->header_.uuid); > + } else { > + if (!sbrec_multicast_group_is_new(mc)) { > + ofctrl_remove_flows(&mc->header_.uuid); > + } > + consider_mc_group(mff_ovn_geneve, ct_zones, > + local_datapaths, mc, &ofpacts, > + &remote_ofpacts); > + } > + } > } > > ofpbuf_uninit(&remote_ofpacts); > -- > 1.9.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
