| Message ID | 20210203192952.1849843-1-willemdebruijn.kernel@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 52cbd23a119c6ebf40a527e53f3402d2ea38eccb |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] udp: fix skb_copy_and_csum_datagram with odd segment sizes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 5 maintainers not CCed: pabeni@redhat.com edumazet@google.com sagi@lightbitslabs.com mchehab+huawei@kernel.org ktkhai@virtuozzo.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 10880 this patch: 10880 |
| netdev/kdoc | success | Errors and warnings before: 4 this patch: 4 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 96 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 17033 this patch: 17033 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Wed, Feb 3, 2021 at 11:29 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > From: Willem de Bruijn <willemb@google.com> > > When iteratively computing a checksum with csum_block_add, track the > offset "pos" to correctly rotate in csum_block_add when offset is odd. > > The open coded implementation of skb_copy_and_csum_datagram did this. > With the switch to __skb_datagram_iter calling csum_and_copy_to_iter, > pos was reinitialized to 0 on each call. > > Bring back the pos by passing it along with the csum to the callback. > > Changes v1->v2 > - pass csum value, instead of csump pointer (Alexander Duyck) > > Link: https://lore.kernel.org/netdev/20210128152353.GB27281@optiplex/ > Fixes: 950fcaecd5cc ("datagram: consolidate datagram copy to iter helpers") > Reported-by: Oliver Graute <oliver.graute@gmail.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Looks good to me. Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
On 2/3/21 8:29 PM, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > > When iteratively computing a checksum with csum_block_add, track the > offset "pos" to correctly rotate in csum_block_add when offset is odd. > > The open coded implementation of skb_copy_and_csum_datagram did this. > With the switch to __skb_datagram_iter calling csum_and_copy_to_iter, > pos was reinitialized to 0 on each call. > > Bring back the pos by passing it along with the csum to the callback. > > Changes v1->v2 > - pass csum value, instead of csump pointer (Alexander Duyck) > > Link: https://lore.kernel.org/netdev/20210128152353.GB27281@optiplex/ > Fixes: 950fcaecd5cc ("datagram: consolidate datagram copy to iter helpers") > Reported-by: Oliver Graute <oliver.graute@gmail.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> > --- > Reviewed-by: Eric Dumazet <edumazet@google.com>
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Wed, 3 Feb 2021 14:29:52 -0500 you wrote: > From: Willem de Bruijn <willemb@google.com> > > When iteratively computing a checksum with csum_block_add, track the > offset "pos" to correctly rotate in csum_block_add when offset is odd. > > The open coded implementation of skb_copy_and_csum_datagram did this. > With the switch to __skb_datagram_iter calling csum_and_copy_to_iter, > pos was reinitialized to 0 on each call. > > [...] Here is the summary with links: - [net,v2] udp: fix skb_copy_and_csum_datagram with odd segment sizes https://git.kernel.org/netdev/net/c/52cbd23a119c You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/include/linux/uio.h b/include/linux/uio.h index 72d88566694e..27ff8eb786dc 100644 --- a/include/linux/uio.h +++ b/include/linux/uio.h @@ -260,7 +260,13 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count) { i->count = count; } -size_t csum_and_copy_to_iter(const void *addr, size_t bytes, void *csump, struct iov_iter *i); + +struct csum_state { + __wsum csum; + size_t off; +}; + +size_t csum_and_copy_to_iter(const void *addr, size_t bytes, void *csstate, struct iov_iter *i); size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i); bool csum_and_copy_from_iter_full(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i); size_t hash_and_copy_to_iter(const void *addr, size_t bytes, void *hashp, diff --git a/lib/iov_iter.c b/lib/iov_iter.c index a21e6a5792c5..f0b2ccb1bb01 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -592,14 +592,15 @@ static __wsum csum_and_memcpy(void *to, const void *from, size_t len, } static size_t csum_and_copy_to_pipe_iter(const void *addr, size_t bytes, - __wsum *csum, struct iov_iter *i) + struct csum_state *csstate, + struct iov_iter *i) { struct pipe_inode_info *pipe = i->pipe; unsigned int p_mask = pipe->ring_size - 1; + __wsum sum = csstate->csum; + size_t off = csstate->off; unsigned int i_head; size_t n, r; - size_t off = 0; - __wsum sum = *csum; if (!sanity(i)) return 0; @@ -621,7 +622,8 @@ static size_t csum_and_copy_to_pipe_iter(const void *addr, size_t bytes, i_head++; } while (n); i->count -= bytes; - *csum = sum; + csstate->csum = sum; + csstate->off = off; return bytes; } @@ -1522,18 +1524,19 @@ bool csum_and_copy_from_iter_full(void *addr, size_t bytes, __wsum *csum, } EXPORT_SYMBOL(csum_and_copy_from_iter_full); -size_t csum_and_copy_to_iter(const void *addr, size_t bytes, void *csump, +size_t csum_and_copy_to_iter(const void *addr, size_t bytes, void *_csstate, struct iov_iter *i) { + struct csum_state *csstate = _csstate; const char *from = addr; - __wsum *csum = csump; __wsum sum, next; - size_t off = 0; + size_t off; if (unlikely(iov_iter_is_pipe(i))) - return csum_and_copy_to_pipe_iter(addr, bytes, csum, i); + return csum_and_copy_to_pipe_iter(addr, bytes, _csstate, i); - sum = *csum; + sum = csstate->csum; + off = csstate->off; if (unlikely(iov_iter_is_discard(i))) { WARN_ON(1); /* for now */ return 0; @@ -1561,7 +1564,8 @@ size_t csum_and_copy_to_iter(const void *addr, size_t bytes, void *csump, off += v.iov_len; }) ) - *csum = sum; + csstate->csum = sum; + csstate->off = off; return bytes; } EXPORT_SYMBOL(csum_and_copy_to_iter); diff --git a/net/core/datagram.c b/net/core/datagram.c index 81809fa735a7..15ab9ffb27fe 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -721,8 +721,16 @@ static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset, struct iov_iter *to, int len, __wsum *csump) { - return __skb_datagram_iter(skb, offset, to, len, true, - csum_and_copy_to_iter, csump); + struct csum_state csdata = { .csum = *csump }; + int ret; + + ret = __skb_datagram_iter(skb, offset, to, len, true, + csum_and_copy_to_iter, &csdata); + if (ret) + return ret; + + *csump = csdata.csum; + return 0; } /**