amd64 vs i386 in_cksum_hdr() with unaligned data

[Karim Fodil-Lemelin]

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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at 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);
  }