On Nov 7, 2011, at 12:51 PM, Jesse Gross wrote: > You're right that this is a bug (and one that I think has existed for > much longer than just that commit from a year ago). I think the fix > is correct but can you use csum_replace2() and then drop the casts? > It's functionally exactly the same but since it was the casts the > helped to hide this problem in the first place, I'd like to avoid > them. You'll have to backport csum_replace2() but it is very small.
A revised version of the patch is below. Let me know if this looks reasonable. (I updated the TTL patch to also use the csum_replace2 function.) --Justin -=-=-=-=-=-=-=-=-=-=-=-=-=- commit 3909af614eaa5737165b9d1fa652c9769b2cf2ac Author: Justin Pettit <[email protected]> Date: Sun Nov 6 23:37:21 2011 -0800 datapath: Properly calculate checksum when updating TOS field. When updating the IP TOS field, the checksum was not properly calculated on little endian systems. This commit fixes the issue. Signed-off-by: Justin Pettit <[email protected]> diff --git a/acinclude.m4 b/acinclude.m4 index 648132a..fd89bea 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -248,6 +248,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ [OVS_DEFINE([HAVE_CSUM_TYPES])]) OVS_GREP_IFELSE([$KSRC/include/net/checksum.h], [csum_replace4]) + OVS_GREP_IFELSE([$KSRC/include/net/checksum.h], [csum_replace2]) OVS_GREP_IFELSE([$KSRC/include/net/checksum.h], [csum_unfold]) OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [NLA_NUL_STRING]) diff --git a/datapath/actions.c b/datapath/actions.c index b5b92ba..afd1791 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -158,8 +158,7 @@ static void set_ip_tos(struct sk_buff *skb, struct iphdr *nh /* Set the DSCP bits and preserve the ECN bits. */ old = nh->tos; new = new_tos | (nh->tos & INET_ECN_MASK); - csum_replace4(&nh->check, (__force __be32)old, - (__force __be32)new); + csum_replace2(&nh->check, htons(old), htons(new)); nh->tos = new; } diff --git a/datapath/linux/compat/include/net/checksum.h b/datapath/linux/compa index 73f2f59..9f0b882 100644 --- a/datapath/linux/compat/include/net/checksum.h +++ b/datapath/linux/compat/include/net/checksum.h @@ -27,6 +27,13 @@ static inline void csum_replace4(__sum16 *sum, __be32 from, _ } #endif +#ifndef HAVE_CSUM_REPLACE2 +static inline void csum_replace2(__sum16 *sum, __be16 from, __be16 to) +{ + csum_replace4(sum, (__force __be32)from, (__force __be32)to); +} +#endif + #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) #define inet_proto_csum_replace2(sum, skb, from, to, pseudohdr) \ inet_proto_csum_replace4(sum, skb, (__force __be32)(from), \ _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
