Re: [PATCH] ipv6: suppress sparse warnings in IP6_ECN_set_ce()

[Eric Dumazet]

On Thu, 2016-08-11 at 12:04 +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.b...@intel.com>
> 
> Use the correct type for these manipulations, which is __wsum,
> instead of using __be32. This doesn't really change anything
> since __wsum really *is* __be32, but removes the address space
> warnings from sparse.
> 
> Cc: Eric Dumazet <eduma...@google.com>
> Fixes: 34ae6a1aa054 ("ipv6: update skb->csum when CE mark is propagated")
> Signed-off-by: Johannes Berg <johannes.b...@intel.com>
> ---
>  include/net/inet_ecn.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/inet_ecn.h b/include/net/inet_ecn.h
> index 0dc0a51da38f..89aa4e73fc37 100644
> --- a/include/net/inet_ecn.h
> +++ b/include/net/inet_ecn.h
> @@ -119,14 +119,14 @@ struct ipv6hdr;
>   */
>  static inline int IP6_ECN_set_ce(struct sk_buff *skb, struct ipv6hdr *iph)
>  {
> -     __be32 from, to;
> +     __wsum from, to;
>  
>       if (INET_ECN_is_not_ect(ipv6_get_dsfield(iph)))
>               return 0;
>  
> -     from = *(__be32 *)iph;
> -     to = from | htonl(INET_ECN_CE << 20);
> -     *(__be32 *)iph = to;
> +     from = *(__wsum *)iph;

Well, 4 first bytes of iph are not a __wsum technically speaking :(

> +     to = from | (__force __wsum)htonl(INET_ECN_CE << 20);

Hmm... this is a bit convoluted.
Doing a OR operation on a __wsum seems strange to me.

> +     *(__wsum *)iph = to;
>       if (skb->ip_summed == CHECKSUM_COMPLETE)
>               skb->csum = csum_add(csum_sub(skb->csum, from), to);
>       return 1;



What about something like that ?

No big deal, but this looks a bit cleaner to me.

Thanks.

diff --git a/include/net/inet_ecn.h b/include/net/inet_ecn.h
index 
0dc0a51da38faacab2ea275681f5f70e09a6c79e..dce2d586d9cecb9e9de381aa0926f3e3d3ec9568
 100644
--- a/include/net/inet_ecn.h
+++ b/include/net/inet_ecn.h
@@ -128,7 +128,8 @@ static inline int IP6_ECN_set_ce(struct sk_buff *skb, 
struct ipv6hdr *iph)
        to = from | htonl(INET_ECN_CE << 20);
        *(__be32 *)iph = to;
        if (skb->ip_summed == CHECKSUM_COMPLETE)
-               skb->csum = csum_add(csum_sub(skb->csum, from), to);
+               skb->csum = csum_add(csum_sub(skb->csum, (__force __wsum)from),
+                                    (__force __wsum)to);
        return 1;
 }