| Message ID | 20211204045356.3659278-1-eric.dumazet@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 45cac6754529ae17345d8f5b632d9e602a091a20 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net-next] net: fix recent csum changes | expand |
On Fri, Dec 03, 2021 at 08:53:56PM -0800, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Vladimir reported csum issues after my recent change in skb_postpull_rcsum() > > Issue here is the following: > > initial skb->csum is the csum of > > [part to be pulled][rest of packet] > > Old code: > skb->csum = csum_sub(skb->csum, csum_partial(pull, pull_length, 0)); > > New code: > skb->csum = ~csum_partial(pull, pull_length, ~skb->csum); > > This is broken if the csum of [pulled part] > happens to be equal to skb->csum, because end > result of skb->csum is 0 in new code, instead > of being 0xffffffff > > David Laight suggested to use > > skb->csum = -csum_partial(pull, pull_length, -skb->csum); > > I based my patches on existing code present in include/net/seg6.h, > update_csum_diff4() and update_csum_diff16() which might need > a similar fix. > > I guess that my tests, mostly pulling 40 bytes of IPv6 header > were not providing enough entropy to hit this bug. > > v2: added wsum_negate() to make sparse happy. > > Fixes: 29c3002644bd ("net: optimize skb_postpull_rcsum()") > Fixes: 0bd28476f636 ("gro: optimize skb_gro_postpull_rcsum()") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Vladimir Oltean <vladimir.oltean@nxp.com> > Suggested-by: David Laight <David.Laight@ACULAB.COM> > Cc: David Lebrun <dlebrun@google.com> > --- Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com> Thanks!
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Fri, 3 Dec 2021 20:53:56 -0800 you wrote: > From: Eric Dumazet <edumazet@google.com> > > Vladimir reported csum issues after my recent change in skb_postpull_rcsum() > > Issue here is the following: > > initial skb->csum is the csum of > > [...] Here is the summary with links: - [v2,net-next] net: fix recent csum changes https://git.kernel.org/netdev/net-next/c/45cac6754529 You are awesome, thank you!
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index eae4bd3237a41cc1b60b44c91fbfe21dfdd8f117..dd262bd8ddbecc3638e40e198ebeecc115bbfffa 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -3486,7 +3486,8 @@ static inline void skb_postpull_rcsum(struct sk_buff *skb, const void *start, unsigned int len) { if (skb->ip_summed == CHECKSUM_COMPLETE) - skb->csum = ~csum_partial(start, len, ~skb->csum); + skb->csum = wsum_negate(csum_partial(start, len, + wsum_negate(skb->csum))); else if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_start_offset(skb) < 0) skb->ip_summed = CHECKSUM_NONE; diff --git a/include/net/checksum.h b/include/net/checksum.h index 5b96d5bd6e54532a7a82511ff5d7d4c6f18982d5..5218041e5c8f93cd369a2a3a46add3e6a5e41af7 100644 --- a/include/net/checksum.h +++ b/include/net/checksum.h @@ -180,4 +180,8 @@ static inline void remcsum_unadjust(__sum16 *psum, __wsum delta) *psum = csum_fold(csum_sub(delta, (__force __wsum)*psum)); } +static inline __wsum wsum_negate(__wsum val) +{ + return (__force __wsum)-((__force u32)val); +} #endif diff --git a/include/net/gro.h b/include/net/gro.h index b1139fca7c435ca0c353c4ed17628dd7f3df4401..8f75802d50fd734018d4b257c66fd0545095b593 100644 --- a/include/net/gro.h +++ b/include/net/gro.h @@ -173,8 +173,8 @@ static inline void skb_gro_postpull_rcsum(struct sk_buff *skb, const void *start, unsigned int len) { if (NAPI_GRO_CB(skb)->csum_valid) - NAPI_GRO_CB(skb)->csum = ~csum_partial(start, len, - ~NAPI_GRO_CB(skb)->csum); + NAPI_GRO_CB(skb)->csum = wsum_negate(csum_partial(start, len, + wsum_negate(NAPI_GRO_CB(skb)->csum))); } /* GRO checksum functions. These are logical equivalents of the normal