Ben Pfaff <b...@ovn.org> wrote on 08/12/2016 04:48:26 PM:

> From: Ben Pfaff <b...@ovn.org>
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 08/12/2016 04:48 PM
> Subject: Re: [ovs-dev] [PATCH] ovn-controller: Remove flows created
> for now deleted SB database rows.
>
> On Fri, Aug 12, 2016 at 02:47:09PM -0700, Ben Pfaff wrote:
> > 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.
>
> Oh, whoops, there was a newer version.  I've un-applied this and I'll
> look at the newer one.
>

Yeah, I decided to take you at your word in re using ssets for string
representation of UUIDs and replaced that hack with hmaps of the
actual uuids...

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to