* [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors
@ 2025-09-08 17:50 Stanislav Fomichev
2025-09-08 20:44 ` David Ahern
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2025-09-08 17:50 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, ncardwell, kuniyu, dsahern, horms,
linux-kernel, Mina Almasry
tcp_recvmsg_dmabuf can export the following errors:
- EFAULT when linear copy fails
- ETOOSMALL when cmsg put fails
- ENODEV if one of the frags is readable
- ENOMEM on xarray failures
But they are all ignored and replaced by EFAULT in the caller
(tcp_recvmsg_locked). Expose real error to the userspace to
add more transparency on what specifically fails.
In non-devmem case (skb_copy_datagram_msg) doing `if (!copied)
copied=-EFAULT` is ok because skb_copy_datagram_msg can return only EFAULT.
Cc: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
net/ipv4/tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 588932c3cf1d..c56d53e32c29 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2820,7 +2820,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
used);
if (err <= 0) {
if (!copied)
- copied = -EFAULT;
+ copied = err;
break;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors
2025-09-08 17:50 [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors Stanislav Fomichev
@ 2025-09-08 20:44 ` David Ahern
2025-09-09 18:05 ` Mina Almasry
2025-09-09 18:09 ` Eric Dumazet
2025-09-10 1:35 ` Jakub Kicinski
2 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2025-09-08 20:44 UTC (permalink / raw)
To: Stanislav Fomichev, netdev
Cc: davem, edumazet, kuba, pabeni, ncardwell, kuniyu, horms,
linux-kernel, Mina Almasry
On 9/8/25 11:50 AM, Stanislav Fomichev wrote:
> tcp_recvmsg_dmabuf can export the following errors:
> - EFAULT when linear copy fails
> - ETOOSMALL when cmsg put fails
> - ENODEV if one of the frags is readable
> - ENOMEM on xarray failures
>
> But they are all ignored and replaced by EFAULT in the caller
> (tcp_recvmsg_locked). Expose real error to the userspace to
> add more transparency on what specifically fails.
>
> In non-devmem case (skb_copy_datagram_msg) doing `if (!copied)
> copied=-EFAULT` is ok because skb_copy_datagram_msg can return only EFAULT.
>
> Cc: Mina Almasry <almasrymina@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> net/ipv4/tcp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors
2025-09-08 20:44 ` David Ahern
@ 2025-09-09 18:05 ` Mina Almasry
0 siblings, 0 replies; 6+ messages in thread
From: Mina Almasry @ 2025-09-09 18:05 UTC (permalink / raw)
To: David Ahern
Cc: Stanislav Fomichev, netdev, davem, edumazet, kuba, pabeni,
ncardwell, kuniyu, horms, linux-kernel
On Mon, Sep 8, 2025 at 1:44 PM David Ahern <dsahern@kernel.org> wrote:
>
> On 9/8/25 11:50 AM, Stanislav Fomichev wrote:
> > tcp_recvmsg_dmabuf can export the following errors:
> > - EFAULT when linear copy fails
> > - ETOOSMALL when cmsg put fails
> > - ENODEV if one of the frags is readable
> > - ENOMEM on xarray failures
> >
> > But they are all ignored and replaced by EFAULT in the caller
> > (tcp_recvmsg_locked). Expose real error to the userspace to
> > add more transparency on what specifically fails.
> >
> > In non-devmem case (skb_copy_datagram_msg) doing `if (!copied)
> > copied=-EFAULT` is ok because skb_copy_datagram_msg can return only EFAULT.
> >
> > Cc: Mina Almasry <almasrymina@google.com>
> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> > ---
> > net/ipv4/tcp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
>
> Reviewed-by: David Ahern <dsahern@kernel.org>
>
Reviewed-by: Mina Almasry <almasrymina@google.com>
--
Thanks,
Mina
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors
2025-09-08 17:50 [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors Stanislav Fomichev
2025-09-08 20:44 ` David Ahern
@ 2025-09-09 18:09 ` Eric Dumazet
2025-09-10 1:35 ` Jakub Kicinski
2 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2025-09-09 18:09 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, davem, kuba, pabeni, ncardwell, kuniyu, dsahern, horms,
linux-kernel, Mina Almasry
On Mon, Sep 8, 2025 at 10:50 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> tcp_recvmsg_dmabuf can export the following errors:
> - EFAULT when linear copy fails
> - ETOOSMALL when cmsg put fails
> - ENODEV if one of the frags is readable
> - ENOMEM on xarray failures
>
> But they are all ignored and replaced by EFAULT in the caller
> (tcp_recvmsg_locked). Expose real error to the userspace to
> add more transparency on what specifically fails.
>
> In non-devmem case (skb_copy_datagram_msg) doing `if (!copied)
> copied=-EFAULT` is ok because skb_copy_datagram_msg can return only EFAULT.
>
> Cc: Mina Almasry <almasrymina@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors
2025-09-08 17:50 [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors Stanislav Fomichev
2025-09-08 20:44 ` David Ahern
2025-09-09 18:09 ` Eric Dumazet
@ 2025-09-10 1:35 ` Jakub Kicinski
2025-09-10 15:39 ` Stanislav Fomichev
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2025-09-10 1:35 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, davem, edumazet, pabeni, ncardwell, kuniyu, dsahern,
horms, linux-kernel, Mina Almasry
On Mon, 8 Sep 2025 10:50:45 -0700 Stanislav Fomichev wrote:
> if (err <= 0) {
Should we change this condition to be err < 0, then?
I don't see a path that'd return 0 but it's a bit odd to explicitly
handle 0 here and then let it override copied.
> if (!copied)
> - copied = -EFAULT;
> + copied = err;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors
2025-09-10 1:35 ` Jakub Kicinski
@ 2025-09-10 15:39 ` Stanislav Fomichev
0 siblings, 0 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2025-09-10 15:39 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Stanislav Fomichev, netdev, davem, edumazet, pabeni, ncardwell,
kuniyu, dsahern, horms, linux-kernel, Mina Almasry
On 09/09, Jakub Kicinski wrote:
> On Mon, 8 Sep 2025 10:50:45 -0700 Stanislav Fomichev wrote:
> > if (err <= 0) {
>
> Should we change this condition to be err < 0, then?
> I don't see a path that'd return 0 but it's a bit odd to explicitly
> handle 0 here and then let it override copied.
As you say, we don't seem to be returning 0 from tcp_recvmsg_dmabuf, so
this is mostly about the readability. But makes sense overall, will
repost.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-10 15:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-08 17:50 [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors Stanislav Fomichev
2025-09-08 20:44 ` David Ahern
2025-09-09 18:05 ` Mina Almasry
2025-09-09 18:09 ` Eric Dumazet
2025-09-10 1:35 ` Jakub Kicinski
2025-09-10 15:39 ` Stanislav Fomichev
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).