[ovs-dev] [PATCH] ovn-controller: update_ct_zone operates always on empty set
Russell Bryant
russell at ovn.org
Wed Jul 27 21:09:12 UTC 2016
On Wed, Jul 27, 2016 at 8:34 AM, Ryan Moats <rmoats at us.ibm.com> wrote:
> "dev" <dev-bounces at openvswitch.org> wrote on 07/27/2016 12:44:38 AM:
>
> > From: Babu Shanmugam <bschanmu at redhat.com>
> > To: Russell Bryant <russell at ovn.org>
> > Cc: ovs dev <dev at openvswitch.org>
> > Date: 07/27/2016 12:45 AM
> > Subject: Re: [ovs-dev] [PATCH] ovn-controller: update_ct_zone
> > operates always on empty set
> > Sent by: "dev" <dev-bounces at openvswitch.org>
> >
> >
> >
> > On Wednesday 27 July 2016 06:43 AM, Russell Bryant wrote:
> > > On Tue, Jul 26, 2016 at 6:46 AM, <bschanmu at redhat.com
> > > <mailto:bschanmu at redhat.com <bschanmu at redhat.com>>> wrote:
> > >
> > > From: Babu Shanmugam <bschanmu at redhat.com
> > > <mailto:bschanmu at redhat.com <bschanmu at redhat.com>>>
> > >
> > > Commit 263064a (Convert binding_run to incremental processing.)
> > > removed the usage
> > > of all_lports from binding_run, but it is infact used in the
> > > context of the caller,
> > > especially by update_ct_zones().
> > >
> > > Without this change, update_ct_zones operates on an empty set
> always.
> > >
> > > Signed-off-by: Babu Shanmugam <bschanmu at redhat.com
> > > <mailto:bschanmu at redhat.com <bschanmu at redhat.com>>>
> > >
> > >
> > > Ouch. This is a really bad regression. If I understand correctly,
> > > we're not setting a ct zone ID for any logical ports. All are just
> > > using the default zone of 0.
> > >
> > Yes Russell, your understanding is correct.
> > > We should think about a good way to test OVN's use of conntrack zones
> > > to ensure that entries end up in separate zones for separate ports. A
> > > good test for that may require userspace conntrack support, though.
> > > Another test we could do now would be looking at the flows in table 0
> > > and ensuring that the input flow for each port has a different
> > > conntrack zone ID assigned. That feels like kind of a hack, though.
> > I agree that we need more test cases. I could not spend much time to
> > figure out a proper approach for a test case. I will have a look at it.
> >
> > Thank you,
> > Babu
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> <http://patchwork.ozlabs.org/patch/653288/>
> http://patchwork.ozlabs.org/patch/653288/ replicates the
> all_lports code from binding.c prior to commit 263064a (I literally
> rolled a repo back to the commit before that one and then did a
> code inspection and copy/paste). Now, I still don't have a test case
> that shows the revert is fixed (because I frankly don't know how to
> write this one), but I believe that with the above patch set we are
> no longer using only the default zone.
>
Thanks. Re-adding a full loop over all port bindings seems pretty
undesirable. I think we can iterate on this original patch and get a full
solution without a ton of extra effort.
--
Russell Bryant
More information about the dev
mailing list