[ovs-dev] [PATCH 9/9] ovn-controller: Drop most uses of OVS patch ports.

Ben Pfaff blp at ovn.org
Thu Dec 15 23:38:47 UTC 2016


On Thu, Dec 15, 2016 at 11:17:57AM -0800, Mickey Spiegel wrote:
> On Thu, Dec 15, 2016 at 10:15 AM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > On Wed, Dec 07, 2016 at 10:46:22AM -0800, Ben Pfaff wrote:
> > > On Wed, Dec 07, 2016 at 10:08:25AM -0800, Guru Shetty wrote:
> > > > On 4 December 2016 at 23:17, Ben Pfaff <blp at ovn.org> wrote:
> > > >
> > > > > Until now, ovn-controller has implemented OVN logical patch ports and
> > > > > l3gateway ports in terms of OVS patch ports.  It is a hassle to
> > create and
> > > > > destroy ports, and it is also wasteful compared to what the patch
> > ports
> > > > > actually buy us: the ability to "save and restore" a packet around a
> > > > > recursive trip through the flow table.  The "clone" action can do
> > that too,
> > > > > without the need to create a port.  This commit takes advantage of
> > the
> > > > > clone action for that purpose, getting rid of most of the patch ports
> > > > > previously created by ovn-controller.
> > > > >
> > > > > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > > > >
> > > >
> > > > Though all the unit tests pass, all the system tests related to the
> > gateway
> > > > fail.
> > > > make check-kernel TESTSUITEFLAGS="-k ovn"
> > >
> > > Uh-oh.  Is there something obviously wrong?
> >
> > Does the following incremental fix the problem?
> >
> > (I'll repost the whole series in a while though.)
> >
> > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> > index 678b62d..f33750e 100644
> > --- a/ovn/controller/physical.c
> > +++ b/ovn/controller/physical.c
> > @@ -321,12 +321,12 @@ consider_port_binding(enum mf_field_id
> > mff_ovn_geneve,
> >
> >          struct zone_ids zone_ids = { .ct = 0 };
> >          if (!strcmp(binding->type, "l3gateway")) {
> > -            char *dnat = alloc_nat_zone_key(&binding->
> > datapath->header_.uuid,
> > +            char *dnat = alloc_nat_zone_key(&peer->
> > datapath->header_.uuid,
> >
> 
> This needs to be &binding->datapath->header_.uuid for the call to
> put_local_common_flows below.
> The zone_ids need to be recalculated somewhere between
> put_local_common_flows and load_logical_ingress_metadata,
> based on &peer->datapath->header_.uuid.

Oh, I see what you mean.

put_local_common_flows() needs the zones for 'binding'.
load_logical_ingress_metadata() needs the zones for 'peer'.

I'll fix that.  Thank you for the review!


More information about the dev mailing list