[ovs-dev] [PATCH v2] ovn-controller: Persist desired conntrack groups.

Ben Pfaff blp at ovn.org
Thu Jul 28 22:13:47 UTC 2016


On Thu, Jul 28, 2016 at 05:10:56PM -0500, Ryan Moats wrote:
> Ben Pfaff <blp at ovn.org> wrote on 07/28/2016 04:06:09 PM:
> 
> > From: Ben Pfaff <blp at ovn.org>
> > To: Ryan Moats/Omaha/IBM at IBMUS, Gurucharan Shetty <guru at ovn.org>
> > Cc: dev at openvswitch.org
> > Date: 07/28/2016 04:06 PM
> > Subject: Re: [ovs-dev] [PATCH v2] ovn-controller: Persist desired
> > conntrack groups.
> >
> > On Thu, Jul 28, 2016 at 02:18:45PM +0000, Ryan Moats wrote:
> > > With incremental processing of logical flows desired conntrack groups
> > > are not being persisted.  This patch adds this capability, with the
> > > side effect of adding a ds_clone method that this capability leverages.
> > >
> > > Signed-off-by: Ryan Moats <rmoats at us.ibm.com>
> > > Reported-by: Guru Shetty <guru at ovn.org>
> > > Reported-at: http://openvswitch.org/pipermail/dev/2016-July/076320.html
> > > Fixes: 70c7cfe ("ovn-controller: Add incremental processing to
> > lflow_run and physical_run")
> > > ---
> > >  v1->v2 addressed review comments
> > >    updated commit message
> > >    changed name of ds_copy to ds_clone
> > >    moved lflow uuid storage to action_params for cleaner code
> >
> > It seems odd that group_clone() doesn't copy the UUID.
> >
> > I'm not sure why group_info's 'group' member is a struct ds instead of
> > just a char *.  (This isn't anything that you introduced.)
> >
> > Coding style:
> > > +static struct group_info *
> > > +group_info_clone(struct group_info *source) {
> >
> > But I'll hold off on further review since Guru reports that this causes
> > test failures in the system tests.
> >
> 
> If you want to hit me with the rest of the style comments, I have the
> crash issue fixed and I can fold the style into v3...

That was the style comment.  The { should be on its own line.



More information about the dev mailing list