diff mbox

[ovs-dev,11/24] datapath: backport: allow output of MPLS packets on tunnel vports

Message ID 1468387602-61955-2-git-send-email-pshelar@ovn.org
State Changes Requested
Headers show

Commit Message

Pravin Shelar July 13, 2016, 5:26 a.m. UTC
Upstream commit:
    commit fe3a5f6c795810edb1646a840fec3c8c350c2a4e
    Author: Simon Horman <simon.horman@netronome.com>

    openvswitch: allow output of MPLS packets on tunnel vports

    Currently output of MPLS packets on tunnel vports is not allowed by Open
    vSwitch. This is because historically encapsulation was done in such a way
    that the inner_protocol field of the skb needed to hold the inner protocol
    for both MPLS and tunnel encapsulation in order for GSO segmentation to be
    performed correctly.

    Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of
    vport") Open vSwitch makes use of lwt to output to tunnel netdevs which
    perform encapsulation. As no drivers expose support for MPLS offloads this
    means that GSO packets are segmented in software by validate_xmit_skb(),
    which is called from __dev_queue_xmit(), before tunnel encapsulation occurs.
    This means that the inner protocol of MPLS is no longer needed by the time
    encapsulation occurs and the contention on the inner_protocol field of the
    skb no longer occurs.

    Thus it is now safe to output MPLS to tunnel vports.

    Signed-off-by: Simon Horman <simon.horman@netronome.com>
    Reviewed-by: Jesse Gross <jesse@kernel.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
 datapath/flow_netlink.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Jesse Gross July 17, 2016, 12:05 a.m. UTC | #1
On Tue, Jul 12, 2016 at 10:26 PM, Pravin B Shelar <pshelar@ovn.org> wrote:
> Upstream commit:
>     commit fe3a5f6c795810edb1646a840fec3c8c350c2a4e
>     Author: Simon Horman <simon.horman@netronome.com>
>
>     openvswitch: allow output of MPLS packets on tunnel vports
>
>     Currently output of MPLS packets on tunnel vports is not allowed by Open
>     vSwitch. This is because historically encapsulation was done in such a way
>     that the inner_protocol field of the skb needed to hold the inner protocol
>     for both MPLS and tunnel encapsulation in order for GSO segmentation to be
>     performed correctly.
>
>     Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of
>     vport") Open vSwitch makes use of lwt to output to tunnel netdevs which
>     perform encapsulation. As no drivers expose support for MPLS offloads this
>     means that GSO packets are segmented in software by validate_xmit_skb(),
>     which is called from __dev_queue_xmit(), before tunnel encapsulation occurs.
>     This means that the inner protocol of MPLS is no longer needed by the time
>     encapsulation occurs and the contention on the inner_protocol field of the
>     skb no longer occurs.
>
>     Thus it is now safe to output MPLS to tunnel vports.
>
>     Signed-off-by: Simon Horman <simon.horman@netronome.com>
>     Reviewed-by: Jesse Gross <jesse@kernel.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>

I don't know if this is safe on our backported kernel module. The
premise of this commit was that packets coming out of OVS will go
through the device layer before they hit the tunnel code and that will
resolve any offloads and use of inner_protocol. However, with our
versions of lightweight tunnels, this doesn't happen.
Pravin Shelar July 17, 2016, 4:06 p.m. UTC | #2
On Sat, Jul 16, 2016 at 5:05 PM, Jesse Gross <jesse@kernel.org> wrote:
> On Tue, Jul 12, 2016 at 10:26 PM, Pravin B Shelar <pshelar@ovn.org> wrote:
>> Upstream commit:
>>     commit fe3a5f6c795810edb1646a840fec3c8c350c2a4e
>>     Author: Simon Horman <simon.horman@netronome.com>
>>
>>     openvswitch: allow output of MPLS packets on tunnel vports
>>
>>     Currently output of MPLS packets on tunnel vports is not allowed by Open
>>     vSwitch. This is because historically encapsulation was done in such a way
>>     that the inner_protocol field of the skb needed to hold the inner protocol
>>     for both MPLS and tunnel encapsulation in order for GSO segmentation to be
>>     performed correctly.
>>
>>     Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of
>>     vport") Open vSwitch makes use of lwt to output to tunnel netdevs which
>>     perform encapsulation. As no drivers expose support for MPLS offloads this
>>     means that GSO packets are segmented in software by validate_xmit_skb(),
>>     which is called from __dev_queue_xmit(), before tunnel encapsulation occurs.
>>     This means that the inner protocol of MPLS is no longer needed by the time
>>     encapsulation occurs and the contention on the inner_protocol field of the
>>     skb no longer occurs.
>>
>>     Thus it is now safe to output MPLS to tunnel vports.
>>
>>     Signed-off-by: Simon Horman <simon.horman@netronome.com>
>>     Reviewed-by: Jesse Gross <jesse@kernel.org>
>>     Signed-off-by: David S. Miller <davem@davemloft.net>
>>
>> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
>
> I don't know if this is safe on our backported kernel module. The
> premise of this commit was that packets coming out of OVS will go
> through the device layer before they hit the tunnel code and that will
> resolve any offloads and use of inner_protocol. However, with our
> versions of lightweight tunnels, this doesn't happen.

right, Lets skip this check if we are using upstream tunneling
modules. I will post update patch.
diff mbox

Patch

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 0d9daa5..b8e8799 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -2048,9 +2048,6 @@  static int validate_set(const struct nlattr *a,
 		break;
 
 	case OVS_KEY_ATTR_TUNNEL:
-		if (eth_p_mpls(eth_type))
-			return -EINVAL;
-
 		if (masked)
 			return -EINVAL; /* Masked tunnel set not supported. */