Message-ID: <CAPWQB7EQhbcDEk==AmN58Qxndmd6oHpw8z78kj2Q4M4-mD7+Dw@mail.gmail.com>
Date:   Mon, 22 Aug 2016 14:47:42 -0700
From:   Joe Stringer <joe@....org>
To:     Simon Horman <simon.horman@...ronome.com>
Cc:     pravin shelar <pshelar@....org>, ovs dev <dev@...nvswitch.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jiri Benc <jbenc@...hat.com>
Subject: Re: [ovs-dev] [PATCH net-next v11 5/6] openvswitch: add layer 3
 flow/port support

On 22 August 2016 at 04:04, Simon Horman <simon.horman@...ronome.com> wrote:
> On Wed, Aug 10, 2016 at 10:17:30AM -0700, Joe Stringer wrote:
>> On 10 August 2016 at 03:20, Simon Horman <simon.horman@...ronome.com> wrote:
>> > On Tue, Aug 09, 2016 at 08:47:40AM -0700, pravin shelar wrote:
>> >> On Mon, Aug 8, 2016 at 8:17 AM, Simon Horman <simon.horman@...ronome.com> wrote:
>> >> > Light testing seems to indicate that it works for GSO skbs
>> >> > received over both L3 and L2 GRE tunnels by OvS with both
>> >> > IP-in-MPLS and IP (without MPLS) payloads.
>> >> >
>> >>
>> >> Thanks for testing it. Can you also add those tests to OVS kmod test suite?
>> >> ..
>> >
>> > Sure, I will look into doing that.
>> > Am I correct in thinking Joe Stringer is the best person to contact if
>> > I run into trouble there?
>>
>> Sure. The basics of running the tests is documented here:
>> https://github.com/openvswitch/ovs/blob/master/INSTALL.md#datapath-testing
>>
>> You should be able to get a good feel for how to add tests by perusing
>> the commits to tests/system-{traffic,kmod-macros}.at in the OVS source
>> tree.
>
> Thanks Joe,
>
> it took me a while but I think that I have something working
> against the head branch of the OVS tree. I'd value opinions
> on the direction I have taken.
>
> Subject: [PATCH] system-traffic: Exercise GSO
>
> Exercise GSO for: unencapsulated; MPLS; GRE; and MPLS in GRE.
>
> There is scope to extend this testing to other encapsulation formats
> if desired.
>
> This is motivated by a desire to test GRE and MPLS encapsulation in
> the context of L3/VPN (MPLS over non-TEB GRE work). That is not
> tested here but tests for those cases would ideally be based on those in
> this patch.

This makes sense to me. There's a few corners that could be improved,
primarily for reproducing sane results on a variety of systems, then a
couple of style comments. Please do run the tests via both "make
check-kernel" and "make check-system-userspace" before submitting,
ideally with at least two varieties of kernel: One where you would
expect the test to pass, and one where you would expect the tests to
be skipped.

* CHECK_MPLS is defined in system-kmod-macros.at, so a corresponding
version should be provided in system-userspace-macros.at. If the
criteria for running the test(s) with both userspace and kernel
datapaths is the same, then this could instead be moved into
system-common-macros.at.
* "datapath - ping over gre tunnel" adds a command to execute in
at_ns1, but that namespace doesn't exist.
* "datapath - http over gre tunnel" is missing MPLS_CHECK.
* Is there a way to clear the netstat statistics before running the
tests which rely on it? I'm getting a failure on one of my systems
(ubuntu trusty with a 4.7 kernel), but I'm not sure if the counter was
already high before I ran the test.
* "datapath - http over mpls between two ports"  (maybe others too?)
should shift all openflow rules into a single section using AT_DATA,
similar to the other tests. This makes it easier to reason about the
flow table and understand what's going on before reading through the
rest of the test.
* If there is a common set of configuration you do for local stack
within a namespace to route MPLS traffic, you could consider adding
another macro into system-common-macros.at.

I also see this error on "http over mpls over gre tunnel":
+sh: 1: cannot create /proc/sys/net/mpls/conf/ns_gre0/input: Directory
nonexistent

Maybe MPLS + GRE needs a separate check?

Powered by blists - more mailing lists