From patchwork Fri Jan 6 18:02:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Hurley X-Patchwork-Id: 712064 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3twC7P2ZCQz9t15 for ; Sat, 7 Jan 2017 05:02:25 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=netronome-com.20150623.gappssmtp.com header.i=@netronome-com.20150623.gappssmtp.com header.b="LxRZZRR2"; dkim-atps=neutral Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 9CB38B4B; Fri, 6 Jan 2017 18:02:23 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 832FD40D for ; Fri, 6 Jan 2017 18:02:22 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com [74.125.82.45]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id BDEB919A for ; Fri, 6 Jan 2017 18:02:21 +0000 (UTC) Received: by mail-wm0-f45.google.com with SMTP id k184so40651705wme.1 for ; Fri, 06 Jan 2017 10:02:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=s7uTMnvJ+DjA+X/PfbT2zbo0qfaORmCeLU7zt68vti4=; b=LxRZZRR2sCtw8l0eIQ0n+I0akUEPooaAqV4YbOFVQQJuUe7kej8L1jZRU7/a0kIvKe MZ++rvyzjdfVN5CyMBbC8Yu2bEVLNX1cZFEWiYnWXl5E5CM4xc1rBwiydq+9+8/HY38J tVBH2W6SBCVDlZCLJ0g4Dfvi4bKGa7mYIp9jGTSl9zeorLWAzX0CQb+vT7nNMun3WN9L L1czwZA2t34+8fQ/WCIkswvKH/pUTDryCsIvk121CzuiaDXfwlTbbVgTiQyE+56Iw4qi F/s7B3fF0mB2Ircd7oIJqXwy/k7YmFhniIA6fhNINZo6LKCPzwwYovaME6tKQtFihupb Rv1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=s7uTMnvJ+DjA+X/PfbT2zbo0qfaORmCeLU7zt68vti4=; b=K9/x4gBQ86z9vQJLITJc9GLR91QJE8RwEDMH2oHnBnzIGEMHkmKoEMn7K0qEocEgqJ uG5lO51u4Q+VQIbhLJq5y6fxdL8brQm2WUG7vE7Brk54X188U+YaLl+QOXU38ep4j2in yn586Jo+YWsXZFpZYLH6wvkT8naigDfdJX0j/R3VA6CXvSyuMaS37l07Da/duqd6z6je uinPuN46H5sXYGDGOXTFaWWJRMuP0Z0ZPeQdZjEj1drTX29zK+h6Sx7qbhmdPaE/VVQF CcjBHfSisEBAPr+AbXAwUuTfcu2T8G7G0cmtKO9wsjK2DjG0iPBUUfUWOp1xRg/eF02v tb7A== X-Gm-Message-State: AIkVDXLvVyvy0msGns13/UnPNTilqQiuciACfDq8hYExh3MMtQbDorHZWPIksWxC8seo53EU X-Received: by 10.28.215.6 with SMTP id o6mr4277204wmg.5.1483725740118; Fri, 06 Jan 2017 10:02:20 -0800 (PST) Received: from jhurley-Precision-Tower-3420.netronome.com ([80.76.204.157]) by smtp.gmail.com with ESMTPSA id l6sm4524410wmd.5.2017.01.06.10.02.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 06 Jan 2017 10:02:19 -0800 (PST) From: John Hurley To: dev@openvswitch.org Date: Fri, 6 Jan 2017 18:02:10 +0000 Message-Id: <1483725730-7827-1-git-send-email-john.hurley@netronome.com> X-Mailer: git-send-email 2.7.4 X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: John Hurley Subject: [ovs-dev] [PATCH 1/1] datapath: Ensure correct L4 checksum with NAT helpers. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Setting the CHECKSUM_PARTIAL flag before sending to helper mods can mean that the kernel code will not modify the first part of the L4 checksum correctly after changing packet IPs/ports/payload in kernels <4.6. This can mean that the L4 checksum is incorrect when the packet egresses the system. Giving the packet a temp skb_dst with RTCF_LOCAL flag not set ensures the 'csum_*_magic' functions are hit in the kernel (if required) and the modified packet will get the correct checksum when fully processed. This has tested with FTP NAT helpers on kernel version 3.13. Signed-off-by: John Hurley Acked-by: Jarno Rajahalme --- datapath/conntrack.c | 47 +++++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/datapath/conntrack.c b/datapath/conntrack.c index d942884..6e0bfed 100644 --- a/datapath/conntrack.c +++ b/datapath/conntrack.c @@ -314,6 +314,11 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto) u8 nexthdr; int err; +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) + bool dst_set = false; + struct rtable rt = { .rt_flags = 0 }; +#endif + ct = nf_ct_get(skb, &ctinfo); if (!ct || ctinfo == IP_CT_RELATED_REPLY) return NF_ACCEPT; @@ -352,43 +357,25 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto) #if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) /* Linux 4.5 and older depend on skb_dst being set when recalculating * checksums after NAT helper has mangled TCP or UDP packet payload. - * This dependency is avoided when skb is CHECKSUM_PARTIAL or when UDP - * has no checksum. - * - * The dependency is not triggered when the main NAT code updates - * checksums after translating the IP header (address, port), so this - * fix only needs to be executed on packets that are both being NATted - * and that have a helper assigned. + * skb_dst is cast to a rtable struct and the flags examined. + * Forcing these flags to have RTCF_LOCAL not set ensures checksum mod + * is carried out in the same way as kernel versions > 4.5 */ - if (ct->status & IPS_NAT_MASK && skb->ip_summed != CHECKSUM_PARTIAL) { - u8 ipproto = (proto == NFPROTO_IPV4) - ? ip_hdr(skb)->protocol : nexthdr; - u16 offset = 0; - - switch (ipproto) { - case IPPROTO_TCP: - offset = offsetof(struct tcphdr, check); - break; - case IPPROTO_UDP: - /* Skip if no csum. */ - if (udp_hdr(skb)->check) - offset = offsetof(struct udphdr, check); - break; - } - if (offset) { - if (unlikely(!pskb_may_pull(skb, protoff + offset + 2))) - return NF_DROP; - - skb->csum_start = skb_headroom(skb) + protoff; - skb->csum_offset = offset; - skb->ip_summed = CHECKSUM_PARTIAL; - } + if (ct->status & IPS_NAT_MASK && skb->ip_summed != CHECKSUM_PARTIAL + && !skb_dst(skb)) { + dst_set = true; + skb_dst_set(skb, &rt.dst); } #endif err = helper->help(skb, protoff, ct, ctinfo); if (err != NF_ACCEPT) return err; +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) + if (dst_set) + skb_dst_set(skb, NULL); +#endif + /* Adjust seqs after helper. This is needed due to some helpers (e.g., * FTP with NAT) adusting the TCP payload size when mangling IP * addresses and/or port numbers in the text-based control connection.