netdev - Re: [PATCH net-next 2/3] net: mpls: Fixups for GSO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <18023723-58cc-cab5-21ad-14785e18c072@cumulusnetworks.com>
Date:	Wed, 17 Aug 2016 20:59:44 -0600
From:	David Ahern <dsa@...ulusnetworks.com>
To:	Alexander Duyck <alexander.duyck@...il.com>
Cc:	Netdev <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Lennert Buytenhek <buytenh@...tstofly.org>,
	Simon Horman <simon.horman@...ronome.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>, rshearma@...cade.com,
	Tom Herbert <tom@...bertland.com>, Thomas Graf <tgraf@...g.ch>,
	olivier.dugeon@...nge.com, roopa <roopa@...ulusnetworks.com>
Subject: Re: [PATCH net-next 2/3] net: mpls: Fixups for GSO

On 8/17/16 7:06 PM, Alexander Duyck wrote:
> On Wed, Aug 17, 2016 at 4:23 PM, David Ahern <dsa@...ulusnetworks.com> wrote:
>> On 8/17/16 5:16 PM, Alexander Duyck wrote:
>>>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>>>> index 1ecbd7715f6d..6d78f162a88b 100644
>>>> --- a/net/openvswitch/actions.c
>>>> +++ b/net/openvswitch/actions.c
>>>> @@ -167,6 +167,12 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
>>>>                 skb->mac_len);
>>>>         skb_reset_mac_header(skb);
>>>>
>>>> +       /* for GSO: set MPLS as network header and encapsulated protocol
>>>> +        * header as inner network header
>>>> +        */
>>>> +       skb_set_network_header(skb, skb->mac_len);
>>>> +       skb_set_inner_network_header(skb, skb->mac_len + MPLS_HLEN);
>>>> +
>>>>         new_mpls_lse = (__be32 *)skb_mpls_header(skb);
>>>>         *new_mpls_lse = mpls->mpls_lse;
>>>>
>>>
>>> So the one question I would have about this is how attached are you to
>>> using the network_header to record the offset for the MPLS header?  I
>>> ask because I think from a hardware offloading perspective it would
>>> make it much easier if instead you used the inner_mac_header to
>>> represent the offset for the MPLS header.  This way device drivers
>>> could just skip over it like a VLAN and just use network and transport
>>> header values like they would otherwise.
>>>
>>
>> Where does the network_header relate to if I change the marker to inner_mac_header? Would it be skipped?
> 
> No, the network header would still be the network header.

If core MPLS code (ie., non-OVS) does not do skb_reset_network_header(skb) after adding the MPLS label nothing works. Not even ping with small packets. tcpdump shows a completely mangled packet. Right now resetting the network_header to mpls is required.