On Wed, Jul 27, 2016 at 8:34 AM, Ryan Moats <rmo...@us.ibm.com> wrote:
> "dev" <dev-boun...@openvswitch.org> wrote on 07/27/2016 12:44:38 AM: > > > From: Babu Shanmugam <bscha...@redhat.com> > > To: Russell Bryant <russ...@ovn.org> > > Cc: ovs dev <dev@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-boun...@openvswitch.org> > > > > > > > > On Wednesday 27 July 2016 06:43 AM, Russell Bryant wrote: > > > On Tue, Jul 26, 2016 at 6:46 AM, <bscha...@redhat.com > > > <mailto:bscha...@redhat.com <bscha...@redhat.com>>> wrote: > > > > > > From: Babu Shanmugam <bscha...@redhat.com > > > <mailto:bscha...@redhat.com <bscha...@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 <bscha...@redhat.com > > > <mailto:bscha...@redhat.com <bscha...@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@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
