On Thu, Jul 28, 2016 at 05:12:04PM -0500, Ryan Moats wrote: > > Ben Pfaff <b...@ovn.org> wrote on 07/28/2016 04:23:57 PM: > > > From: Ben Pfaff <b...@ovn.org> > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: dev@openvswitch.org > > Date: 07/28/2016 04:24 PM > > Subject: Re: [ovs-dev] [PATCH] ovn-controller: Remove flows created > > for now deleted SB database rows. > > > > On Thu, Jul 28, 2016 at 07:54:30PM +0000, Ryan Moats wrote: > > > Ensure that rows created for deleted port binding and > > > multicast group rows are cleared when doing full processing. > > > > > > Signed-off-by: Ryan Moats <rmo...@us.ibm.com> > > > > I'm choosing to overlook storing UUIDs as strings in a set of strings. > > > > How about this simplification? > > > > --8<--------------------------cut here-------------------------->8-- > > > > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c > > index 6bc0095..a4a8fcf 100644 > > --- a/ovn/controller/physical.c > > +++ b/ovn/controller/physical.c > > @@ -598,25 +598,22 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, > > sset_destroy(&remote_chassis); > > } > > > > +/* Deletes the flows whose UUIDs are in 'old' but not 'new', and > > then replaces > > + * 'old' by 'new'. */ > > static void > > rationalize_ssets_and_delete_flows(struct sset *old, struct sset *new) > > { > > - const char *uuid_s, *next_uuid; > > - SSET_FOR_EACH_SAFE (uuid_s, next_uuid, old) { > > + const char *uuid_s; > > + SSET_FOR_EACH (uuid_s, old) { > > if (!sset_find(new, uuid_s)) { > > struct uuid uuid; > > if (uuid_from_string(&uuid, uuid_s)) { > > ofctrl_remove_flows(&uuid); > > } > > - sset_find_and_delete(old, uuid_s); > > } > > } > > - SSET_FOR_EACH_SAFE (uuid_s, next_uuid, new) { > > - if (!sset_find(old, uuid_s)) { > > - sset_add(old, uuid_s); > > - } > > - sset_find_and_delete(new, uuid_s); > > - } > > + sset_swap(old, new); > > + sset_clear(new); > > } > > > > void > > > > Works for me... Ryan
I noticed a memory leak in this patch, so I folded in the following and applied this to master. --8<--------------------------cut here-------------------------->8-- diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index b63aca2..c60611d 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -658,6 +658,14 @@ rationalize_ssets_and_delete_flows(struct sset *old, struct sset *new) sset_clear(new); } +static void +add_uuid_to_sset(struct sset *set, const struct uuid *uuid) +{ + char uuid_str[UUID_LEN + 1]; + snprintf(uuid_str, sizeof uuid_str, UUID_FMT, UUID_ARGS(uuid)); + sset_add(set, uuid_str); +} + void physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, const struct ovsrec_bridge *br_int, const char *this_chassis_id, @@ -822,8 +830,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, ofctrl_remove_flows(&binding->header_.uuid); consider_port_binding(mff_ovn_geneve, ct_zones, local_datapaths, patched_datapaths, binding, &ofpacts); - sset_add(&new_port_binding_uuids, - xasprintf(UUID_FMT, UUID_ARGS(&binding->header_.uuid))); + add_uuid_to_sset(&new_port_binding_uuids, &binding->header_.uuid); } rationalize_ssets_and_delete_flows(&port_binding_uuids, &new_port_binding_uuids); @@ -856,8 +863,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, ofctrl_remove_flows(&mc->header_.uuid); consider_mc_group(mff_ovn_geneve, ct_zones, local_datapaths, mc, &ofpacts, &remote_ofpacts); - sset_add(&new_multicast_group_uuids, - xasprintf(UUID_FMT, UUID_ARGS(&mc->header_.uuid))); + add_uuid_to_sset(&new_multicast_group_uuids, &mc->header_.uuid); } rationalize_ssets_and_delete_flows(&multicast_group_uuids, &new_multicast_group_uuids); _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev