Date: Fri, 01 Feb 2013 10:22:10 -0500 From: Karim Fodil-Lemelin <fodillemlinkarim@gmail.com> To: freebsd-net@freebsd.org Subject: Re: amd64 vs i386 in_cksum_hdr() with unaligned data Message-ID: <510BDDA2.6010503@gmail.com> In-Reply-To: <510BD6FF.9090101@gmail.com> References: <510BD6FF.9090101@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?510BDDA2.6010503>