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

[Mickey Spiegel]

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.

Mickey


>                                              "dnat");
>              zone_ids.dnat = simap_get(ct_zones, dnat);
>              free(dnat);
>
> -            char *snat = alloc_nat_zone_key(&binding->
> datapath->header_.uuid,
> +            char *snat = alloc_nat_zone_key(&peer->
> datapath->header_.uuid,
>                                              "snat");
>              zone_ids.snat = simap_get(ct_zones, snat);
>              free(snat);
> --
> 2.10.2
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>