[ovs-dev] [PATCH] ovn-controller: Remove flows created for now deleted SB database rows.

Ben Pfaff blp at ovn.org
Fri Aug 12 21:47:09 UTC 2016


On Thu, Jul 28, 2016 at 05:12:04PM -0500, Ryan Moats wrote:
> 
> Ben Pfaff <blp at ovn.org> wrote on 07/28/2016 04:23:57 PM:
> 
> > From: Ben Pfaff <blp at ovn.org>
> > To: Ryan Moats/Omaha/IBM at IBMUS
> > Cc: dev at 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 <rmoats at 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);



More information about the dev mailing list