On Mon, Nov 7, 2011 at 10:33 AM, Justin Pettit <[email protected]> wrote: > Commit 8b6956 (datapath: Correctly update IP checksum with actions.) > introduced an issue that caused the checksum to not be properly > calculated on little endian systems when updating the IP TOS field. > This commit fixes the issue. > > Signed-off-by: Justin Pettit <[email protected]> > --- > datapath/actions.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/datapath/actions.c b/datapath/actions.c > index b5b92ba..efe5e98 100644 > --- a/datapath/actions.c > +++ b/datapath/actions.c > @@ -158,8 +158,8 @@ static void set_ip_tos(struct sk_buff *skb, struct iphdr > *nh, u8 new_tos) > /* 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_replace4(&nh->check, (__force __be32)htons(old), > + (__force __be32)htons(new));
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. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
