* [PATCH] vsock: ratelimit unknown ioctl error message @ 2020-10-23 12:21 Colin King 2020-10-23 14:09 ` Stefano Garzarella 0 siblings, 1 reply; 12+ messages in thread From: Colin King @ 2020-10-23 12:21 UTC (permalink / raw) To: David S . Miller, Jakub Kicinski, Stefano Garzarella, netdev Cc: kernel-janitors, linux-kernel From: Colin Ian King <colin.king@canonical.com> When exercising the kernel with stress-ng with some ioctl tests the "Unknown ioctl" error message is spamming the kernel log at a high rate. Rate limit this message to reduce the noise. Signed-off-by: Colin Ian King <colin.king@canonical.com> --- net/vmw_vsock/af_vsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 9e93bc201cc0..b8feb9223454 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -2072,7 +2072,7 @@ static long vsock_dev_do_ioctl(struct file *filp, break; default: - pr_err("Unknown ioctl %d\n", cmd); + pr_err_ratelimited("Unknown ioctl %d\n", cmd); retval = -EINVAL; } -- 2.27.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-23 12:21 [PATCH] vsock: ratelimit unknown ioctl error message Colin King @ 2020-10-23 14:09 ` Stefano Garzarella 2020-10-23 20:25 ` Jakub Kicinski 2020-10-23 21:30 ` David Laight 0 siblings, 2 replies; 12+ messages in thread From: Stefano Garzarella @ 2020-10-23 14:09 UTC (permalink / raw) To: Colin King Cc: David S . Miller, Jakub Kicinski, netdev, kernel-janitors, linux-kernel On Fri, Oct 23, 2020 at 01:21:13PM +0100, Colin King wrote: >From: Colin Ian King <colin.king@canonical.com> > >When exercising the kernel with stress-ng with some ioctl tests the >"Unknown ioctl" error message is spamming the kernel log at a high >rate. Rate limit this message to reduce the noise. > >Signed-off-by: Colin Ian King <colin.king@canonical.com> >--- > net/vmw_vsock/af_vsock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >index 9e93bc201cc0..b8feb9223454 100644 >--- a/net/vmw_vsock/af_vsock.c >+++ b/net/vmw_vsock/af_vsock.c >@@ -2072,7 +2072,7 @@ static long vsock_dev_do_ioctl(struct file *filp, > break; > > default: >- pr_err("Unknown ioctl %d\n", cmd); >+ pr_err_ratelimited("Unknown ioctl %d\n", cmd); Make sense, or maybe can we remove the error message returning only the -EINVAL? Both cases are fine for me: Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > retval = -EINVAL; > } > >-- >2.27.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-23 14:09 ` Stefano Garzarella @ 2020-10-23 20:25 ` Jakub Kicinski 2020-10-23 21:30 ` David Laight 1 sibling, 0 replies; 12+ messages in thread From: Jakub Kicinski @ 2020-10-23 20:25 UTC (permalink / raw) To: Stefano Garzarella Cc: Colin King, David S . Miller, netdev, kernel-janitors, linux-kernel On Fri, 23 Oct 2020 16:09:47 +0200 Stefano Garzarella wrote: > On Fri, Oct 23, 2020 at 01:21:13PM +0100, Colin King wrote: > >From: Colin Ian King <colin.king@canonical.com> > > > >When exercising the kernel with stress-ng with some ioctl tests the > >"Unknown ioctl" error message is spamming the kernel log at a high > >rate. Rate limit this message to reduce the noise. > > > >Signed-off-by: Colin Ian King <colin.king@canonical.com> > >--- > > net/vmw_vsock/af_vsock.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c > >index 9e93bc201cc0..b8feb9223454 100644 > >--- a/net/vmw_vsock/af_vsock.c > >+++ b/net/vmw_vsock/af_vsock.c > >@@ -2072,7 +2072,7 @@ static long vsock_dev_do_ioctl(struct file *filp, > > break; > > > > default: > >- pr_err("Unknown ioctl %d\n", cmd); > >+ pr_err_ratelimited("Unknown ioctl %d\n", cmd); > > Make sense, or maybe can we remove the error message returning only the > -EINVAL? +1 > Both cases are fine for me: > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-23 14:09 ` Stefano Garzarella 2020-10-23 20:25 ` Jakub Kicinski @ 2020-10-23 21:30 ` David Laight 2020-10-26 8:43 ` Stefano Garzarella 1 sibling, 1 reply; 12+ messages in thread From: David Laight @ 2020-10-23 21:30 UTC (permalink / raw) To: 'Stefano Garzarella', Colin King Cc: David S . Miller, Jakub Kicinski, netdev, kernel-janitors, linux-kernel From: Stefano Garzarella > Sent: 23 October 2020 15:10 > > On Fri, Oct 23, 2020 at 01:21:13PM +0100, Colin King wrote: > >From: Colin Ian King <colin.king@canonical.com> > > > >When exercising the kernel with stress-ng with some ioctl tests the > >"Unknown ioctl" error message is spamming the kernel log at a high > >rate. Rate limit this message to reduce the noise. > > > >Signed-off-by: Colin Ian King <colin.king@canonical.com> > >--- > > net/vmw_vsock/af_vsock.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c > >index 9e93bc201cc0..b8feb9223454 100644 > >--- a/net/vmw_vsock/af_vsock.c > >+++ b/net/vmw_vsock/af_vsock.c > >@@ -2072,7 +2072,7 @@ static long vsock_dev_do_ioctl(struct file *filp, > > break; > > > > default: > >- pr_err("Unknown ioctl %d\n", cmd); > >+ pr_err_ratelimited("Unknown ioctl %d\n", cmd); > > Make sense, or maybe can we remove the error message returning only the > -EINVAL? Isn't the canonical error for unknown ioctl codes -ENOTTY? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-23 21:30 ` David Laight @ 2020-10-26 8:43 ` Stefano Garzarella 2020-10-26 9:13 ` David Laight 0 siblings, 1 reply; 12+ messages in thread From: Stefano Garzarella @ 2020-10-26 8:43 UTC (permalink / raw) To: David Laight Cc: Colin King, David S . Miller, Jakub Kicinski, netdev, kernel-janitors, linux-kernel On Fri, Oct 23, 2020 at 09:30:59PM +0000, David Laight wrote: > >From: Stefano Garzarella >> Sent: 23 October 2020 15:10 >> >> On Fri, Oct 23, 2020 at 01:21:13PM +0100, Colin King wrote: >> >From: Colin Ian King <colin.king@canonical.com> >> > >> >When exercising the kernel with stress-ng with some ioctl tests the >> >"Unknown ioctl" error message is spamming the kernel log at a high >> >rate. Rate limit this message to reduce the noise. >> > >> >Signed-off-by: Colin Ian King <colin.king@canonical.com> >> >--- >> > net/vmw_vsock/af_vsock.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >> >index 9e93bc201cc0..b8feb9223454 100644 >> >--- a/net/vmw_vsock/af_vsock.c >> >+++ b/net/vmw_vsock/af_vsock.c >> >@@ -2072,7 +2072,7 @@ static long vsock_dev_do_ioctl(struct file *filp, >> > break; >> > >> > default: >> >- pr_err("Unknown ioctl %d\n", cmd); >> >+ pr_err_ratelimited("Unknown ioctl %d\n", cmd); >> >> Make sense, or maybe can we remove the error message returning only the >> -EINVAL? > >Isn't the canonical error for unknown ioctl codes -ENOTTY? > Oh, thanks for pointing that out! I had not paid attention to the error returned, but looking at it I noticed that perhaps the most appropriate would be -ENOIOCTLCMD. In the ioctl syscall we return -ENOTTY, if the callback returns -ENOIOCTLCMD. What do you think? Stefano ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-26 8:43 ` Stefano Garzarella @ 2020-10-26 9:13 ` David Laight 2020-10-26 9:39 ` Stefano Garzarella 0 siblings, 1 reply; 12+ messages in thread From: David Laight @ 2020-10-26 9:13 UTC (permalink / raw) To: 'Stefano Garzarella' Cc: Colin King, David S . Miller, Jakub Kicinski, netdev, kernel-janitors, linux-kernel From: Stefano Garzarella > Sent: 26 October 2020 08:43 ... > >Isn't the canonical error for unknown ioctl codes -ENOTTY? > > > > Oh, thanks for pointing that out! > > I had not paid attention to the error returned, but looking at it I > noticed that perhaps the most appropriate would be -ENOIOCTLCMD. > In the ioctl syscall we return -ENOTTY, if the callback returns > -ENOIOCTLCMD. > > What do you think? It is 729 v 443 in favour of ENOTTY (based on grep). No idea where ENOIOCTLCMD comes from, but ENOTTY probably goes back to the early 1970s. The fact that the ioctl wrapper converts the value is a good hint that userspace expects ENOTTY. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-26 9:13 ` David Laight @ 2020-10-26 9:39 ` Stefano Garzarella 2020-10-26 9:46 ` David Laight 0 siblings, 1 reply; 12+ messages in thread From: Stefano Garzarella @ 2020-10-26 9:39 UTC (permalink / raw) To: David Laight Cc: Colin King, David S . Miller, Jakub Kicinski, netdev, kernel-janitors, linux-kernel On Mon, Oct 26, 2020 at 09:13:23AM +0000, David Laight wrote: >From: Stefano Garzarella >> Sent: 26 October 2020 08:43 >... >> >Isn't the canonical error for unknown ioctl codes -ENOTTY? >> > >> >> Oh, thanks for pointing that out! >> >> I had not paid attention to the error returned, but looking at it I >> noticed that perhaps the most appropriate would be -ENOIOCTLCMD. >> In the ioctl syscall we return -ENOTTY, if the callback returns >> -ENOIOCTLCMD. >> >> What do you think? > >It is 729 v 443 in favour of ENOTTY (based on grep). Under net/ it is 6 vs 83 in favour of ENOIOCTLCMD. > >No idea where ENOIOCTLCMD comes from, but ENOTTY probably >goes back to the early 1970s. Me too. > >The fact that the ioctl wrapper converts the value is a good >hint that userspace expects ENOTTY. Agree on that, but since we are not interfacing directly with userspace, I think it is better to return the more specific error (ENOIOCTLCMD). Thanks, Stefano ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-26 9:39 ` Stefano Garzarella @ 2020-10-26 9:46 ` David Laight 2020-10-26 10:01 ` Stefano Garzarella 0 siblings, 1 reply; 12+ messages in thread From: David Laight @ 2020-10-26 9:46 UTC (permalink / raw) To: 'Stefano Garzarella' Cc: Colin King, David S . Miller, Jakub Kicinski, netdev, kernel-janitors, linux-kernel From: Stefano Garzarella > Sent: 26 October 2020 09:39 > > On Mon, Oct 26, 2020 at 09:13:23AM +0000, David Laight wrote: > >From: Stefano Garzarella > >> Sent: 26 October 2020 08:43 > >... > >> >Isn't the canonical error for unknown ioctl codes -ENOTTY? > >> > > >> > >> Oh, thanks for pointing that out! > >> > >> I had not paid attention to the error returned, but looking at it I > >> noticed that perhaps the most appropriate would be -ENOIOCTLCMD. > >> In the ioctl syscall we return -ENOTTY, if the callback returns > >> -ENOIOCTLCMD. > >> > >> What do you think? > > > >It is 729 v 443 in favour of ENOTTY (based on grep). > > Under net/ it is 6 vs 83 in favour of ENOIOCTLCMD. > > > > >No idea where ENOIOCTLCMD comes from, but ENOTTY probably > >goes back to the early 1970s. > > Me too. > > > > >The fact that the ioctl wrapper converts the value is a good > >hint that userspace expects ENOTTY. > > Agree on that, but since we are not interfacing directly with userspace, > I think it is better to return the more specific error (ENOIOCTLCMD). I bet Linux thought it could use a different error code then found that 'unknown ioctl' was spelt ENOTTY. Back in the old days error values were probably almost unique. strerror(EAGIAN) was "No more processes" for a long time! David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-26 9:46 ` David Laight @ 2020-10-26 10:01 ` Stefano Garzarella 2020-10-26 17:55 ` Jakub Kicinski 0 siblings, 1 reply; 12+ messages in thread From: Stefano Garzarella @ 2020-10-26 10:01 UTC (permalink / raw) To: David Laight, Jakub Kicinski Cc: Colin King, David S . Miller, netdev, kernel-janitors, linux-kernel On Mon, Oct 26, 2020 at 09:46:17AM +0000, David Laight wrote: >From: Stefano Garzarella >> Sent: 26 October 2020 09:39 >> >> On Mon, Oct 26, 2020 at 09:13:23AM +0000, David Laight wrote: >> >From: Stefano Garzarella >> >> Sent: 26 October 2020 08:43 >> >... >> >> >Isn't the canonical error for unknown ioctl codes -ENOTTY? >> >> > >> >> >> >> Oh, thanks for pointing that out! >> >> >> >> I had not paid attention to the error returned, but looking at it I >> >> noticed that perhaps the most appropriate would be -ENOIOCTLCMD. >> >> In the ioctl syscall we return -ENOTTY, if the callback returns >> >> -ENOIOCTLCMD. >> >> >> >> What do you think? >> > >> >It is 729 v 443 in favour of ENOTTY (based on grep). >> >> Under net/ it is 6 vs 83 in favour of ENOIOCTLCMD. >> >> > >> >No idea where ENOIOCTLCMD comes from, but ENOTTY probably >> >goes back to the early 1970s. >> >> Me too. >> >> > >> >The fact that the ioctl wrapper converts the value is a good >> >hint that userspace expects ENOTTY. >> >> Agree on that, but since we are not interfacing directly with userspace, >> I think it is better to return the more specific error (ENOIOCTLCMD). > >I bet Linux thought it could use a different error code then >found that 'unknown ioctl' was spelt ENOTTY. It could be :-) Anyway, as you pointed out, I think we should change the -EINVAL with -ENOTTY or -ENOIOCTLCMD. @Jakub what do you suggest? Thanks, Stefano ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-26 10:01 ` Stefano Garzarella @ 2020-10-26 17:55 ` Jakub Kicinski 2020-10-26 18:24 ` Stefano Garzarella 0 siblings, 1 reply; 12+ messages in thread From: Jakub Kicinski @ 2020-10-26 17:55 UTC (permalink / raw) To: Stefano Garzarella Cc: David Laight, Colin King, David S . Miller, netdev, kernel-janitors, linux-kernel On Mon, 26 Oct 2020 11:01:12 +0100 Stefano Garzarella wrote: > On Mon, Oct 26, 2020 at 09:46:17AM +0000, David Laight wrote: > >From: Stefano Garzarella > >> Sent: 26 October 2020 09:39 > >> > >> On Mon, Oct 26, 2020 at 09:13:23AM +0000, David Laight wrote: > >> >From: Stefano Garzarella > >> >> Sent: 26 October 2020 08:43 > >> >... > >> >> >Isn't the canonical error for unknown ioctl codes -ENOTTY? > >> >> > > >> >> > >> >> Oh, thanks for pointing that out! > >> >> > >> >> I had not paid attention to the error returned, but looking at it I > >> >> noticed that perhaps the most appropriate would be -ENOIOCTLCMD. > >> >> In the ioctl syscall we return -ENOTTY, if the callback returns > >> >> -ENOIOCTLCMD. > >> >> > >> >> What do you think? > >> > > >> >It is 729 v 443 in favour of ENOTTY (based on grep). > >> > >> Under net/ it is 6 vs 83 in favour of ENOIOCTLCMD. > >> > >> > > >> >No idea where ENOIOCTLCMD comes from, but ENOTTY probably > >> >goes back to the early 1970s. > >> > >> Me too. > >> > >> > > >> >The fact that the ioctl wrapper converts the value is a good > >> >hint that userspace expects ENOTTY. > >> > >> Agree on that, but since we are not interfacing directly with userspace, > >> I think it is better to return the more specific error (ENOIOCTLCMD). > > > >I bet Linux thought it could use a different error code then > >found that 'unknown ioctl' was spelt ENOTTY. > > It could be :-) > > Anyway, as you pointed out, I think we should change the -EINVAL with > -ENOTTY or -ENOIOCTLCMD. > > @Jakub what do you suggest? ENOIOCTLCMD is a kernel-internal high return code (515) which should be returned by the driver, but it's then caught inside the core and translated to ENOTTY which is then returned to user space. So you're both right, I guess? But the driver should use ENOIOCTLCMD. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-26 17:55 ` Jakub Kicinski @ 2020-10-26 18:24 ` Stefano Garzarella 2020-10-26 18:37 ` Jakub Kicinski 0 siblings, 1 reply; 12+ messages in thread From: Stefano Garzarella @ 2020-10-26 18:24 UTC (permalink / raw) To: Colin King, Jakub Kicinski Cc: David Laight, David S . Miller, netdev, kernel-janitors, linux-kernel On Mon, Oct 26, 2020 at 10:55:48AM -0700, Jakub Kicinski wrote: >On Mon, 26 Oct 2020 11:01:12 +0100 Stefano Garzarella wrote: >> On Mon, Oct 26, 2020 at 09:46:17AM +0000, David Laight wrote: >> >From: Stefano Garzarella >> >> Sent: 26 October 2020 09:39 >> >> >> >> On Mon, Oct 26, 2020 at 09:13:23AM +0000, David Laight wrote: >> >> >From: Stefano Garzarella >> >> >> Sent: 26 October 2020 08:43 >> >> >... >> >> >> >Isn't the canonical error for unknown ioctl codes -ENOTTY? >> >> >> > >> >> >> >> >> >> Oh, thanks for pointing that out! >> >> >> >> >> >> I had not paid attention to the error returned, but looking at it I >> >> >> noticed that perhaps the most appropriate would be -ENOIOCTLCMD. >> >> >> In the ioctl syscall we return -ENOTTY, if the callback returns >> >> >> -ENOIOCTLCMD. >> >> >> >> >> >> What do you think? >> >> > >> >> >It is 729 v 443 in favour of ENOTTY (based on grep). >> >> >> >> Under net/ it is 6 vs 83 in favour of ENOIOCTLCMD. >> >> >> >> > >> >> >No idea where ENOIOCTLCMD comes from, but ENOTTY probably >> >> >goes back to the early 1970s. >> >> >> >> Me too. >> >> >> >> > >> >> >The fact that the ioctl wrapper converts the value is a good >> >> >hint that userspace expects ENOTTY. >> >> >> >> Agree on that, but since we are not interfacing directly with userspace, >> >> I think it is better to return the more specific error (ENOIOCTLCMD). >> > >> >I bet Linux thought it could use a different error code then >> >found that 'unknown ioctl' was spelt ENOTTY. >> >> It could be :-) >> >> Anyway, as you pointed out, I think we should change the -EINVAL with >> -ENOTTY or -ENOIOCTLCMD. >> >> @Jakub what do you suggest? > >ENOIOCTLCMD is a kernel-internal high return code (515) which should >be returned by the driver, but it's then caught inside the core and >translated to ENOTTY which is then returned to user space. > >So you're both right, I guess? But the driver should use ENOIOCTLCMD. > Thanks for clarify! @Colin, can you send a v2 removing the error message and updating the return value? Thanks, Stefano ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] vsock: ratelimit unknown ioctl error message 2020-10-26 18:24 ` Stefano Garzarella @ 2020-10-26 18:37 ` Jakub Kicinski 0 siblings, 0 replies; 12+ messages in thread From: Jakub Kicinski @ 2020-10-26 18:37 UTC (permalink / raw) To: Stefano Garzarella Cc: Colin King, David Laight, David S . Miller, netdev, kernel-janitors, linux-kernel On Mon, 26 Oct 2020 19:24:57 +0100 Stefano Garzarella wrote: > @Colin, can you send a v2 removing the error message and updating the > return value? Not as a single patch, please, these are two different changes. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-10-26 18:37 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-23 12:21 [PATCH] vsock: ratelimit unknown ioctl error message Colin King 2020-10-23 14:09 ` Stefano Garzarella 2020-10-23 20:25 ` Jakub Kicinski 2020-10-23 21:30 ` David Laight 2020-10-26 8:43 ` Stefano Garzarella 2020-10-26 9:13 ` David Laight 2020-10-26 9:39 ` Stefano Garzarella 2020-10-26 9:46 ` David Laight 2020-10-26 10:01 ` Stefano Garzarella 2020-10-26 17:55 ` Jakub Kicinski 2020-10-26 18:24 ` Stefano Garzarella 2020-10-26 18:37 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).