[ovs-dev] [PATCH 2/2] ovn-controller: Change strategy for gateway conntrack zone allocation.

[Ryan Moats]

Guru Shetty <guru at ovn.org> wrote on 07/08/2016 12:55:06 PM:

> From: Guru Shetty <guru at ovn.org>
> To: Ryan Moats/Omaha/IBM at IBMUS
> Cc: ovs dev <dev at openvswitch.org>
> Date: 07/08/2016 12:55 PM
> Subject: Re: [ovs-dev] [PATCH 2/2] ovn-controller: Change strategy
> for gateway conntrack zone allocation.
>
> On 8 July 2016 at 10:51, Ryan Moats <rmoats at us.ibm.com> wrote:
> "dev" <dev-bounces at openvswitch.org> wrote on 07/08/2016 02:38:06 AM:
>
> > From: Gurucharan Shetty <guru at ovn.org>
> > To: dev at openvswitch.org
> > Date: 07/08/2016 12:36 PM
> > Subject: [ovs-dev] [PATCH 2/2] ovn-controller: Change strategy for
> > gateway conntrack zone allocation.
> > Sent by: "dev" <dev-bounces at openvswitch.org>
> >
> > Commit 263064aeaa31e7 (Convert binding_run to incremental processing.)
> > changed the way patched_datapaths were handled. Previously we would
> > destroy the datastructure in every run and re-create it fresh. The new
> > way causes problems with the way conntrack zones are allocated as now
> > we can have stale port_binding entries causing segmentation faults.
> >
> > With this commit, we simply don't depend on port_binding records in
> > conntrack zone allocation and instead store the UUID as a string in
> > the patch_datapath datastructure.
> >
> > Fixes: 263064aeaa31e7 ("Convert binding_run to incremental
processing.")
> > Signed-off-by: Gurucharan Shetty <guru at ovn.org>
> > ---
>
> I like what this is doing, but I'd like it more if it had a test case
> that would fail without this patch, but pass with it, so that we don't
> regress...
> Ryan,
>  With the above referenced patch that this issue fixes, stale
> patched datapaths were simply not removed. And none of the OVN tests
> caught it. We can't catch this unless we start adding negative tests
> (for e.g: delete records and look to see if ovn-controller deleted
> patch ports in br-int). Or do you have something else in mind?

If this is going to cause a core dump, how about a test that runs
through the steps that cause a core dump and then check to see if
ovn-controller is still running?  That's what was done for the patch
that fixed the double destroy bug from the original address set patch
series.

Ryan