diff mbox

[ovs-dev,v2,00/11] ovn: get rid of most uses of patch ports

Message ID CAKFX60e9kKwXbQJvd=xOwUTONsn2SYWiJ3KN6_9LfoHKx0y1KA@mail.gmail.com
State Superseded
Headers show

Commit Message

Mickey Spiegel Dec. 17, 2016, 11:50 p.m. UTC
On Fri, Dec 16, 2016 at 2:51 PM, Guru Shetty <guru@ovn.org> wrote:

> On 16 December 2016 at 14:25, Ben Pfaff <blp@ovn.org> wrote:
>
> > At least a v3 will be forthcoming to incorporate Darrell and Liran's
> > "datapaths of interest" concept, but I said yesterday that I'd send a
> > revised version before v3 is ready.
> >
> > v1->v2:
> >    - Fixed some bugs reported by Mickey throughout the series.
> >    - Patches 1 and 2 are new.
> >    - Patch 3 is modified in various ways, most notably to avoid using the
> >    datapath "sample" action, to add a test, and to better conform to
> style
> >    in a few small ways.
> >
>
> I was hoping that Mickey's fixes would fix the gateway issues. But I still
> get gateway test failures when run with:
> make check-kernel TESTSUITEFLAGS="-k ovn"
>
> I will take a closer look, before v3.
>

I found the problem. The "l3gateway" ports were no longer being added
to mc groups.

The following incremental fixes the problem:


Mickey


>
>
> >
> > Ben Pfaff (10):
> >   ofp-actions: Use struct ext_action_header for appropriate actions.
> >   ofp-actions: Move function for struct ofpact_nest near struct
> >     definition.
> >   ovn-controller: Make indexes more broadly available.
> >   lport: Be a little more careful building lport index.
> >   lport: Tolerate null pointers in destroy functions.
> >   lport: Add index for logical datapaths.
> >   ovn-controller: Handle only relevant ports and flows.
> >   pnysical: Factor code out of consider_port_binding().
> >   ovn-controller: Avoid code duplication getting chassis record.
> >   ovn-controller: Drop most uses of OVS patch ports.
> >
> > William Tu (1):
> >   ofp-actions: Add clone action.
> >
> >  include/openvswitch/ofp-actions.h |  17 +-
> >  lib/ofp-actions.c                 | 187 ++++++++++++++-------
> >  ofproto/ofproto-dpif-xlate.c      |  14 ++
> >  ovn/controller/binding.c          | 118 ++++++++++---
> >  ovn/controller/binding.h          |  11 +-
> >  ovn/controller/chassis.c          |  12 +-
> >  ovn/controller/chassis.h          |   3 +-
> >  ovn/controller/lflow.c            |  46 +-----
> >  ovn/controller/lflow.h            |   1 -
> >  ovn/controller/lport.c            |  82 +++++++++
> >  ovn/controller/lport.h            |  33 +++-
> >  ovn/controller/ovn-controller.c   |  77 ++++-----
> >  ovn/controller/ovn-controller.h   |  33 ++--
> >  ovn/controller/patch.c            | 110 ++----------
> >  ovn/controller/patch.h            |   6 +-
> >  ovn/controller/physical.c         | 340 ++++++++++++++++++++++--------
> > --------
> >  ovn/controller/physical.h         |   9 +-
> >  ovn/controller/pinctrl.c          |  36 ++--
> >  ovn/controller/pinctrl.h          |   4 +-
> >  tests/ofp-actions.at              |   5 +
> >  tests/ofproto-dpif.at             |  19 +++
> >  tests/ovn-controller.at           |  50 +-----
> >  tests/system-traffic.at           |  29 ++++
> >  23 files changed, 724 insertions(+), 518 deletions(-)
> >
> > --
> > 2.10.2
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Comments

Gurucharan Shetty Dec. 19, 2016, 4:59 p.m. UTC | #1
On 17 December 2016 at 15:50, Mickey Spiegel <mickeys.dev@gmail.com> wrote:

>
> On Fri, Dec 16, 2016 at 2:51 PM, Guru Shetty <guru@ovn.org> wrote:
>
>> On 16 December 2016 at 14:25, Ben Pfaff <blp@ovn.org> wrote:
>>
>> > At least a v3 will be forthcoming to incorporate Darrell and Liran's
>> > "datapaths of interest" concept, but I said yesterday that I'd send a
>> > revised version before v3 is ready.
>> >
>> > v1->v2:
>> >    - Fixed some bugs reported by Mickey throughout the series.
>> >    - Patches 1 and 2 are new.
>> >    - Patch 3 is modified in various ways, most notably to avoid using
>> the
>> >    datapath "sample" action, to add a test, and to better conform to
>> style
>> >    in a few small ways.
>> >
>>
>> I was hoping that Mickey's fixes would fix the gateway issues. But I still
>> get gateway test failures when run with:
>> make check-kernel TESTSUITEFLAGS="-k ovn"
>>
>> I will take a closer look, before v3.
>>
>
> I found the problem. The "l3gateway" ports were no longer being added
> to mc groups.
>
> The following incremental fixes the problem:
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 8340d54..3c813b9 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -630,7 +630,8 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
>              put_resubmit(OFTABLE_CHECK_LOOPBACK, remote_ofpacts_p);
>          } else if (simap_contains(&localvif_to_ofport,
>                             (port->parent_port && *port->parent_port)
> -                           ? port->parent_port : port->logical_port)) {
> +                           ? port->parent_port : port->logical_port)
> +                   || !strcmp(port->type, "l3gateway")) {
>              put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, ofpacts_p);
>              put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
>          } else if (port->chassis && !get_localnet_port(local_datapaths,
>
> Mickey
>

Thanks. That looks to be it. The tests pass now.


>
>
>>
>>
>> >
>> > Ben Pfaff (10):
>> >   ofp-actions: Use struct ext_action_header for appropriate actions.
>> >   ofp-actions: Move function for struct ofpact_nest near struct
>> >     definition.
>> >   ovn-controller: Make indexes more broadly available.
>> >   lport: Be a little more careful building lport index.
>> >   lport: Tolerate null pointers in destroy functions.
>> >   lport: Add index for logical datapaths.
>> >   ovn-controller: Handle only relevant ports and flows.
>> >   pnysical: Factor code out of consider_port_binding().
>> >   ovn-controller: Avoid code duplication getting chassis record.
>> >   ovn-controller: Drop most uses of OVS patch ports.
>> >
>> > William Tu (1):
>> >   ofp-actions: Add clone action.
>> >
>> >  include/openvswitch/ofp-actions.h |  17 +-
>> >  lib/ofp-actions.c                 | 187 ++++++++++++++-------
>> >  ofproto/ofproto-dpif-xlate.c      |  14 ++
>> >  ovn/controller/binding.c          | 118 ++++++++++---
>> >  ovn/controller/binding.h          |  11 +-
>> >  ovn/controller/chassis.c          |  12 +-
>> >  ovn/controller/chassis.h          |   3 +-
>> >  ovn/controller/lflow.c            |  46 +-----
>> >  ovn/controller/lflow.h            |   1 -
>> >  ovn/controller/lport.c            |  82 +++++++++
>> >  ovn/controller/lport.h            |  33 +++-
>> >  ovn/controller/ovn-controller.c   |  77 ++++-----
>> >  ovn/controller/ovn-controller.h   |  33 ++--
>> >  ovn/controller/patch.c            | 110 ++----------
>> >  ovn/controller/patch.h            |   6 +-
>> >  ovn/controller/physical.c         | 340 ++++++++++++++++++++++--------
>> > --------
>> >  ovn/controller/physical.h         |   9 +-
>> >  ovn/controller/pinctrl.c          |  36 ++--
>> >  ovn/controller/pinctrl.h          |   4 +-
>> >  tests/ofp-actions.at              |   5 +
>> >  tests/ofproto-dpif.at             |  19 +++
>> >  tests/ovn-controller.at           |  50 +-----
>> >  tests/system-traffic.at           |  29 ++++
>> >  23 files changed, 724 insertions(+), 518 deletions(-)
>> >
>> > --
>> > 2.10.2
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev@openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> >
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
diff mbox

Patch

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 8340d54..3c813b9 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -630,7 +630,8 @@  consider_mc_group(enum mf_field_id mff_ovn_geneve,
             put_resubmit(OFTABLE_CHECK_LOOPBACK, remote_ofpacts_p);
         } else if (simap_contains(&localvif_to_ofport,
                            (port->parent_port && *port->parent_port)
-                           ? port->parent_port : port->logical_port)) {
+                           ? port->parent_port : port->logical_port)
+                   || !strcmp(port->type, "l3gateway")) {
             put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, ofpacts_p);
             put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
         } else if (port->chassis && !get_localnet_port(local_datapaths,