[ovs-dev] [#8024 1/7] datapath: Properly calculate checksum when updating TOS field.

Jesse Gross jesse at nicira.com
Mon Nov 7 20:51:55 UTC 2011


On Mon, Nov 7, 2011 at 10:33 AM, Justin Pettit <jpettit at nicira.com> 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 <jpettit at nicira.com>
> ---
>  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.



More information about the dev mailing list