From owner-freebsd-net@FreeBSD.ORG Fri Feb 1 15:22:19 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 90C33BA4 for ; Fri, 1 Feb 2013 15:22:19 +0000 (UTC) (envelope-from fodillemlinkarim@gmail.com) Received: from mail-ie0-x22c.google.com (mail-ie0-x22c.google.com [IPv6:2607:f8b0:4001:c03::22c]) by mx1.freebsd.org (Postfix) with ESMTP id 668DAD24 for ; Fri, 1 Feb 2013 15:22:19 +0000 (UTC) Received: by mail-ie0-f172.google.com with SMTP id c10so3494199ieb.17 for ; Fri, 01 Feb 2013 07:22:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=DvNCPcEDoyLATeFYGrcP3lxRfBASyjcma6a0sSSgC0o=; b=WMftFc2k56BR6H4n45tKAaLW1U9nAkI92dAHBB+5Bw5CeyBSGWpK4e9KvtGYxZ+vSj sj2uUJhtrprgi8oDoVOdWJ+GMwl8vso79LTRy0MEMhdcMgcxCY1jl0NfLQl2Jb2WN3Qr 3HRIlwmOFFzBburDJjAQhZhxTy5NQWyiS5LqyIVyPq+MXk4/Zx5xe383nKMiQgqPqgIv ladJ8xGHJrYixUtke4LOa4ECSbvP19F0n64gH92ZEISoRZstK4QYhGr8f0dgYbmK9pYm uwijVSpaLUX4ehl0274MNiUfsE5SaFEVOMEmEqZgnD/DTDWBnuMaUUjt26jBiFQWfDuq 4A/A== X-Received: by 10.42.30.132 with SMTP id v4mr9701549icc.34.1359732138480; Fri, 01 Feb 2013 07:22:18 -0800 (PST) Received: from [192.168.1.73] ([208.85.112.101]) by mx.google.com with ESMTPS id as6sm2565509igc.8.2013.02.01.07.22.16 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 01 Feb 2013 07:22:17 -0800 (PST) Message-ID: <510BDDA2.6010503@gmail.com> Date: Fri, 01 Feb 2013 10:22:10 -0500 From: Karim Fodil-Lemelin User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130107 Thunderbird/17.0.2 MIME-Version: 1.0 To: freebsd-net@freebsd.org Subject: Re: amd64 vs i386 in_cksum_hdr() with unaligned data References: <510BD6FF.9090101@gmail.com> In-Reply-To: <510BD6FF.9090101@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Feb 2013 15:22:19 -0000 On 01/02/2013 9:53 AM, Karim Fodil-Lemelin wrote: > Hi -net, > > Sorry for the lengthy email. > > TLDR: If the IP header is not aligned on an even address then the > amd64 version of in_cksum_hdr() will not work while the i386 version > of it will. > > I came across this problem while working on custom software using the > FreeBSD bridge. Our code sometimes decapsulate packets with the > resulting header starting on an odd address and we need to send it > through the FreeBSD bridge where we hit in_cksum_hdr() in > bridge_pfil(). While this always worked on i386 we started seeing > 'reversed' checksums on amd64: > > 21:47:29.178620 IP (tos 0x0, ttl 63, id 3819, offset 0, flags [none], > proto ICMP (1), length 84) > 192.168.76.100 > 192.168.73.200: ICMP echo request, id 44019, seq > 2, length 64 > 21:47:29.179972 IP (tos 0x0, ttl 62, id 1701, offset 0, flags [none], > proto ICMP (1), length 84, bad cksum 875e (->5e87)!) > 192.168.73.200 > 192.168.76.100: ICMP echo reply, id 44019, seq 2, > length 64 > > Please note the reversed checksum on the ICMP reply (as if someone had > called htons on ip_sum ...). Needless to say this caused a lot of head > scratching over here. > > Now it looks like the i386 version of in_cksum_hdr() is totally > different then the amd64 one. A current workaround for us is to use > in_cksum() in if_bridge.c by applying this patch: > > diff --git a/freebsd/sys/net/if_bridge.c b/freebsd/sys/net/if_bridge.c > index 24a4522..fbd9aec 100644 > --- a/freebsd/sys/net/if_bridge.c > +++ b/freebsd/sys/net/if_bridge.c > @@ -3894,9 +3894,11 @@ ipfwpass: > #endif > /* Recalculate the ip checksum */ > ip->ip_sum = 0; > +#if 0 /* Dirty Hack */ > if (hlen == sizeof(struct ip)) > ip->ip_sum = in_cksum_hdr(ip); > else > +#endif > ip->ip_sum = in_cksum(*mp, hlen); > > break; > > et voila! Problem fixed. or not ... At least this gets us going but I > think the scope of this problem is much bigger then the bridge code > alone. > > The FreeBSD source uses in_cksum_hdr() in many other places then > if_bridge.c and while the i386 version of it is capable of dealing > with unaligned addresses the amd64 one is not (we haven't checked > other architectures). My question(s) to the list is what is the proper > way to fix this? Should we replace all occurrence of in_cksum_hdr() > with in_cksum()? Should we write another inline assembly of the > in_cksum_hdr function for 64bit? Should in_cksum_hdr() in amd64 > changed to deal with misaligned addresses? Other solutions? > > Thanks, > > Karim. > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" Quick follow up on this, the following patch fixed the issue for us: diff --git a/freebsd/sys/amd64/amd64/in_cksum.c b/freebsd/sys/amd64/amd64/in_cksum.c index ae02e91..71749e1 100644 --- a/freebsd/sys/amd64/amd64/in_cksum.c +++ b/freebsd/sys/amd64/amd64/in_cksum.c @@ -233,9 +233,13 @@ skip_start: u_int in_cksum_hdr(const struct ip *ip) { - u_int64_t sum = in_cksumdata(ip, sizeof(struct ip)); - union q_util q_util; - union l_util l_util; - REDUCE16; - return (~sum & 0xffff); + u_int64_t sum; + union q_util q_util; + union l_util l_util; + if ((uintptr_t)ip & 1) + sum = in_cksumdata(ip, sizeof(struct ip)) << 8; + else + sum = in_cksumdata(ip, sizeof(struct ip)); + REDUCE16; + return (~sum & 0xffff); }