| Message ID | 20210511113400.1722975-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | udp: Switch the order of arguments to copy_linear_skb | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| 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: 139 this patch: 139 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 24 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 198 this patch: 198 |
| netdev/header_inline | success | Link |
From: Matthew Wilcox > Sent: 11 May 2021 12:34 > > All other skb functions use (off, len); this is the only one which > uses (len, off). Make it consistent. I wouldn't change the order of the arguments without some other change that ensures old code fails to compile. (Like tweaking the function name.) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, May 11, 2021 at 01:11:42PM +0000, David Laight wrote: > From: Matthew Wilcox > > Sent: 11 May 2021 12:34 > > > > All other skb functions use (off, len); this is the only one which > > uses (len, off). Make it consistent. > > I wouldn't change the order of the arguments without some other > change that ensures old code fails to compile. > (Like tweaking the function name.) Yes, some random essentially internal function that has had no new users since it was created in 2017 should get a new name *eyeroll*. Please find more useful things to critique. Or, you know, write some damned code yourself instead of just having opinions.
From: Matthew Wilcox > Sent: 11 May 2021 14:39 > > On Tue, May 11, 2021 at 01:11:42PM +0000, David Laight wrote: > > From: Matthew Wilcox > > > Sent: 11 May 2021 12:34 > > > > > > All other skb functions use (off, len); this is the only one which > > > uses (len, off). Make it consistent. > > > > I wouldn't change the order of the arguments without some other > > change that ensures old code fails to compile. > > (Like tweaking the function name.) > > Yes, some random essentially internal function that has had no new > users since it was created in 2017 should get a new name *eyeroll*. > > Please find more useful things to critique. Or, you know, write some > damned code yourself instead of just having opinions. You could easily completely screw up any code that isn't committed to the kernel source tree. It isn't the sort of bug I'd want to diagnose. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, May 11, 2021 at 01:44:45PM +0000, David Laight wrote: > From: Matthew Wilcox > > Sent: 11 May 2021 14:39 > > > > On Tue, May 11, 2021 at 01:11:42PM +0000, David Laight wrote: > > > From: Matthew Wilcox > > > > Sent: 11 May 2021 12:34 > > > > > > > > All other skb functions use (off, len); this is the only one which > > > > uses (len, off). Make it consistent. > > > > > > I wouldn't change the order of the arguments without some other > > > change that ensures old code fails to compile. > > > (Like tweaking the function name.) > > > > Yes, some random essentially internal function that has had no new > > users since it was created in 2017 should get a new name *eyeroll*. > > > > Please find more useful things to critique. Or, you know, write some > > damned code yourself instead of just having opinions. > > You could easily completely screw up any code that isn't committed > to the kernel source tree. > It isn't the sort of bug I'd want to diagnose. Simple, get your kernel driver into the main kernel tree (remember we are talking about drivers released under a GPL-compatible license here, if your code doesn't fall under this category, good luck, you are on your own here, you leech). -- GregKH
diff --git a/include/net/udp.h b/include/net/udp.h index 360df454356c..c4a7a4c56e75 100644 --- a/include/net/udp.h +++ b/include/net/udp.h @@ -392,7 +392,7 @@ static inline bool udp_skb_is_linear(struct sk_buff *skb) } #endif -static inline int copy_linear_skb(struct sk_buff *skb, int len, int off, +static inline int copy_linear_skb(struct sk_buff *skb, int off, int len, struct iov_iter *to) { int n; diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 15f5504adf5b..783c466e6071 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1859,7 +1859,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock, if (checksum_valid || udp_skb_csum_unnecessary(skb)) { if (udp_skb_is_linear(skb)) - err = copy_linear_skb(skb, copied, off, &msg->msg_iter); + err = copy_linear_skb(skb, off, copied, &msg->msg_iter); else err = skb_copy_datagram_msg(skb, off, msg, copied); } else { diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 199b080d418a..24b202dd370e 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -369,7 +369,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, if (checksum_valid || udp_skb_csum_unnecessary(skb)) { if (udp_skb_is_linear(skb)) - err = copy_linear_skb(skb, copied, off, &msg->msg_iter); + err = copy_linear_skb(skb, off, copied, &msg->msg_iter); else err = skb_copy_datagram_msg(skb, off, msg, copied); } else {
All other skb functions use (off, len); this is the only one which uses (len, off). Make it consistent. Cc: Paolo Abeni <pabeni@redhat.com> Cc: Eric Dumazet <edumazet@google.com> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/net/udp.h | 2 +- net/ipv4/udp.c | 2 +- net/ipv6/udp.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)