[PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors

[PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors

[Stanislav Fomichev]

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

Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors

[David Ahern]

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>

Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors

[Mina Almasry]

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

Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors

[Eric Dumazet]

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>

Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors

[Jakub Kicinski]

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;

Re: [PATCH net-next] net: devmem: expose tcp_recvmsg_locked errors

[Stanislav Fomichev]

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.