netdev - Re: [PATCH net v2 0/3] Tunneling fixes

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>] [day] [month] [year] [list]

Message-ID: <2320b0ab-b48c-292a-52d0-8e2db3ccf521@hpe.com>
Date:   Tue, 18 Oct 2016 18:03:28 +0200
From:   Juerg Haefliger <juerg.haefliger@....com>
To:     jesse@...nel.org
Cc:     netdev@...r.kernel.org, Juerg Haefliger <juerg.haefliger@....com>
Subject: Re: [PATCH net v2 0/3] Tunneling fixes

> This series fixes a problem that was reported where encapsulated packets
> do not have their encapsulation offload markers stripped off when being
> decapsulated. This causes a significant performance drop if the packets
> are later retransmitted.
>
> Fixing this revealed two other bugs which are also addressed as prerequisites:
>  * GRO can aggregate packets for multiple layers of encapsulation which the
>    stack cannot properly handle.
>  * IPIP packets which are combined by GRO are not marked properly with their
>    GSO type.
>
> Note that this is based off the net-next tree as the current target for
> bug fixes.

I need to backport this series to the 4.4 kernel to fix a performance issue we're seeing. The series
applies but commit a09a4c8dd1ec (tunnels: Remove encapsulation offloads on decap) breaks compilation
when CONFIG_IPV6_SIT is enabled. This is because the patch uses iptunnel_pull_header() whose usage
changed with commit 7f290c94352e (iptunnel: scrub packet in iptunnel_pull_header) which is not in 4.4.

7f290c94352e seems to be a cleanup patch which also requires c9e78efb6f66 (vxlan: move vxlan device
lookup before iptunnel_pull_header) and potentially others. Rather than pulling in a slew of cleanup
patches, I was wondering if the following from commit a09a4c8dd1ec can be rewritten without using
the 'new' iptunnel_pull_header() function:

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index f45b8ffc2840..83384308d032 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -681,14 +681,16 @@ static int ipip6_rcv(struct sk_buff *skb)
                skb->mac_header = skb->network_header;
                skb_reset_network_header(skb);
                IPCB(skb)->flags = 0;
-               skb->protocol = htons(ETH_P_IPV6);
+               skb->dev = tunnel->dev;

                if (packet_is_spoofed(skb, iph, tunnel)) {
                        tunnel->dev->stats.rx_errors++;
                        goto out;
                }

-               __skb_tunnel_rx(skb, tunnel->dev, tunnel->net);
+               if (iptunnel_pull_header(skb, 0, htons(ETH_P_IPV6),
+                   !net_eq(tunnel->net, dev_net(tunnel->dev))))
+                       goto out;


Thanks
...Juerg


> v2: No code changes, just additional information in commit messages and
>     a new cover letter.
>
> Jesse Gross (3):
>   ipip: Properly mark ipip GRO packets as encapsulated.
>   tunnels: Don't apply GRO to multiple layers of encapsulation.
>   tunnels: Remove encapsulation offloads on decap.
>
>  include/linux/netdevice.h |  4 ++--
>  include/net/ip_tunnels.h  | 16 ++++++++++++++++
>  net/core/dev.c            |  2 +-
>  net/ipv4/af_inet.c        | 24 ++++++++++++++++++++++--
>  net/ipv4/fou.c            | 13 +++++++++++--
>  net/ipv4/gre_offload.c    |  5 +++++
>  net/ipv4/ip_tunnel_core.c |  3 ++-
>  net/ipv4/udp_offload.c    |  6 +++---
>  net/ipv6/ip6_offload.c    | 15 ++++++++++++++-
>  net/ipv6/sit.c            |  6 ++++--
>  10 files changed, 80 insertions(+), 14 deletions(-)



Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)