[PATCH for-6.1 0/3] fail io_uring zc with sockets not supporting it

[PATCH for-6.1 0/3] fail io_uring zc with sockets not supporting it

[Pavel Begunkov]

Some sockets don't care about msghdr::ubuf_info and would execute the
request by copying data. Such fallback behaviour was always a pain in
my experience, so we'd rather want to fail such requests and have a more
robust api in the future.

Mark struct socket that support it with a new SOCK_SUPPORT_ZC flag.
I'm not entirely sure it's the best place for the flag but at least
we don't have to do a bunch of extra dereferences in the hot path.

P.S. patches 2 and 3 are not combined for backporting purposes.

Pavel Begunkov (3):
  net: flag sockets supporting msghdr originated zerocopy
  io_uring/net: fail zc send when unsupported by socket
  io_uring/net: fail zc sendmsg when unsupported by socket

 include/linux/net.h | 1 +
 io_uring/net.c      | 4 ++++
 net/ipv4/tcp.c      | 1 +
 net/ipv4/udp.c      | 1 +
 4 files changed, 7 insertions(+)

-- 
2.38.0

[PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Pavel Begunkov]

We need an efficient way in io_uring to check whether a socket supports
zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
socket flags fields.

Cc: <stable@vger.kernel.org> # 6.0
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/linux/net.h | 1 +
 net/ipv4/tcp.c      | 1 +
 net/ipv4/udp.c      | 1 +
 3 files changed, 3 insertions(+)

diff --git a/include/linux/net.h b/include/linux/net.h
index 711c3593c3b8..18d942bbdf6e 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -41,6 +41,7 @@ struct net;
 #define SOCK_NOSPACE		2
 #define SOCK_PASSCRED		3
 #define SOCK_PASSSEC		4
+#define SOCK_SUPPORT_ZC		5
 
 #ifndef ARCH_HAS_SOCKET_TYPES
 /**
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 0c51abeee172..aeb7b9eaddc7 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -457,6 +457,7 @@ void tcp_init_sock(struct sock *sk)
 	WRITE_ONCE(sk->sk_sndbuf, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[1]));
 	WRITE_ONCE(sk->sk_rcvbuf, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_rmem[1]));
 
+	set_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
 	sk_sockets_allocated_inc(sk);
 }
 EXPORT_SYMBOL(tcp_init_sock);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d63118ce5900..03616fc5162c 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1620,6 +1620,7 @@ int udp_init_sock(struct sock *sk)
 {
 	skb_queue_head_init(&udp_sk(sk)->reader_queue);
 	sk->sk_destruct = udp_destruct_sock;
+	set_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(udp_init_sock);
-- 
2.38.0

Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Jakub Kicinski]

On Fri, 21 Oct 2022 11:16:39 +0100 Pavel Begunkov wrote:
> We need an efficient way in io_uring to check whether a socket supports
> zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
> socket flags fields.
> 
> Cc: <stable@vger.kernel.org> # 6.0
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  include/linux/net.h | 1 +
>  net/ipv4/tcp.c      | 1 +
>  net/ipv4/udp.c      | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 711c3593c3b8..18d942bbdf6e 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -41,6 +41,7 @@ struct net;
>  #define SOCK_NOSPACE		2
>  #define SOCK_PASSCRED		3
>  #define SOCK_PASSSEC		4
> +#define SOCK_SUPPORT_ZC		5

Acked-by: Jakub Kicinski <kuba@kernel.org>

Any idea on when this will make it to Linus? If within a week we can
probably delay:
https://lore.kernel.org/all/dc549a4d5c1d2031c64794c8e12bed55afb85c3e.1666287924.git.pabeni@redhat.com/
and avoid the conflict.

Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Pavel Begunkov]

On 10/21/22 17:14, Jakub Kicinski wrote:
> On Fri, 21 Oct 2022 11:16:39 +0100 Pavel Begunkov wrote:
>> We need an efficient way in io_uring to check whether a socket supports
>> zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
>> socket flags fields.
>>
>> Cc: <stable@vger.kernel.org> # 6.0
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   include/linux/net.h | 1 +
>>   net/ipv4/tcp.c      | 1 +
>>   net/ipv4/udp.c      | 1 +
>>   3 files changed, 3 insertions(+)
>>
>> diff --git a/include/linux/net.h b/include/linux/net.h
>> index 711c3593c3b8..18d942bbdf6e 100644
>> --- a/include/linux/net.h
>> +++ b/include/linux/net.h
>> @@ -41,6 +41,7 @@ struct net;
>>   #define SOCK_NOSPACE		2
>>   #define SOCK_PASSCRED		3
>>   #define SOCK_PASSSEC		4
>> +#define SOCK_SUPPORT_ZC		5
> 
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> 
> Any idea on when this will make it to Linus? If within a week we can
> probably delay:

After a chat with Jens, IIUC he can take it and send out to
Linus early. So, sounds like a good plan


> https://lore.kernel.org/all/dc549a4d5c1d2031c64794c8e12bed55afb85c3e.1666287924.git.pabeni@redhat.com/
> and avoid the conflict.

-- 
Pavel Begunkov

Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Jens Axboe]

On 10/22/22 9:57 AM, Pavel Begunkov wrote:
> On 10/21/22 17:14, Jakub Kicinski wrote:
>> On Fri, 21 Oct 2022 11:16:39 +0100 Pavel Begunkov wrote:
>>> We need an efficient way in io_uring to check whether a socket supports
>>> zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
>>> socket flags fields.
>>>
>>> Cc: <stable@vger.kernel.org> # 6.0
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>> ---
>>> ? include/linux/net.h | 1 +
>>> ? net/ipv4/tcp.c????? | 1 +
>>> ? net/ipv4/udp.c????? | 1 +
>>> ? 3 files changed, 3 insertions(+)
>>>
>>> diff --git a/include/linux/net.h b/include/linux/net.h
>>> index 711c3593c3b8..18d942bbdf6e 100644
>>> --- a/include/linux/net.h
>>> +++ b/include/linux/net.h
>>> @@ -41,6 +41,7 @@ struct net;
>>> ? #define SOCK_NOSPACE??????? 2
>>> ? #define SOCK_PASSCRED??????? 3
>>> ? #define SOCK_PASSSEC??????? 4
>>> +#define SOCK_SUPPORT_ZC??????? 5
>>
>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>>
>> Any idea on when this will make it to Linus? If within a week we can
>> probably delay:
> 
> After a chat with Jens, IIUC he can take it and send out to
> Linus early. So, sounds like a good plan

Yes, and let's retain the name for now, can always be changed if we need
to make it more granular. I'll ship this off before -rc2.

-- 
Jens Axboe

Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Stefan Metzmacher]

Am 22.10.22 um 18:07 schrieb Jens Axboe:
> On 10/22/22 9:57 AM, Pavel Begunkov wrote:
>> On 10/21/22 17:14, Jakub Kicinski wrote:
>>> On Fri, 21 Oct 2022 11:16:39 +0100 Pavel Begunkov wrote:
>>>> We need an efficient way in io_uring to check whether a socket supports
>>>> zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
>>>> socket flags fields.
>>>>
>>>> Cc: <stable@vger.kernel.org> # 6.0
>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>> ---
>>>> ? include/linux/net.h | 1 +
>>>> ? net/ipv4/tcp.c????? | 1 +
>>>> ? net/ipv4/udp.c????? | 1 +
>>>> ? 3 files changed, 3 insertions(+)
>>>>
>>>> diff --git a/include/linux/net.h b/include/linux/net.h
>>>> index 711c3593c3b8..18d942bbdf6e 100644
>>>> --- a/include/linux/net.h
>>>> +++ b/include/linux/net.h
>>>> @@ -41,6 +41,7 @@ struct net;
>>>> ? #define SOCK_NOSPACE??????? 2
>>>> ? #define SOCK_PASSCRED??????? 3
>>>> ? #define SOCK_PASSSEC??????? 4
>>>> +#define SOCK_SUPPORT_ZC??????? 5
>>>
>>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>>>
>>> Any idea on when this will make it to Linus? If within a week we can
>>> probably delay:
>>
>> After a chat with Jens, IIUC he can take it and send out to
>> Linus early. So, sounds like a good plan
> 
> Yes, and let's retain the name for now, can always be changed if we need
> to make it more granular. I'll ship this off before -rc2.

I'm now always getting -EOPNOTSUPP from SENDMSG_ZC for tcp connections...

metze

Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Stefan Metzmacher]

Am 24.10.22 um 14:49 schrieb Stefan Metzmacher:
> Am 22.10.22 um 18:07 schrieb Jens Axboe:
>> On 10/22/22 9:57 AM, Pavel Begunkov wrote:
>>> On 10/21/22 17:14, Jakub Kicinski wrote:
>>>> On Fri, 21 Oct 2022 11:16:39 +0100 Pavel Begunkov wrote:
>>>>> We need an efficient way in io_uring to check whether a socket supports
>>>>> zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
>>>>> socket flags fields.
>>>>>
>>>>> Cc: <stable@vger.kernel.org> # 6.0
>>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>>> ---
>>>>> ? include/linux/net.h | 1 +
>>>>> ? net/ipv4/tcp.c????? | 1 +
>>>>> ? net/ipv4/udp.c????? | 1 +
>>>>> ? 3 files changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/include/linux/net.h b/include/linux/net.h
>>>>> index 711c3593c3b8..18d942bbdf6e 100644
>>>>> --- a/include/linux/net.h
>>>>> +++ b/include/linux/net.h
>>>>> @@ -41,6 +41,7 @@ struct net;
>>>>> ? #define SOCK_NOSPACE??????? 2
>>>>> ? #define SOCK_PASSCRED??????? 3
>>>>> ? #define SOCK_PASSSEC??????? 4
>>>>> +#define SOCK_SUPPORT_ZC??????? 5
>>>>
>>>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>>>>
>>>> Any idea on when this will make it to Linus? If within a week we can
>>>> probably delay:
>>>
>>> After a chat with Jens, IIUC he can take it and send out to
>>> Linus early. So, sounds like a good plan
>>
>> Yes, and let's retain the name for now, can always be changed if we need
>> to make it more granular. I'll ship this off before -rc2.
> 
> I'm now always getting -EOPNOTSUPP from SENDMSG_ZC for tcp connections...

The problem is that inet_accept() doesn't set SOCK_SUPPORT_ZC...

This hack below fixes it for me, but I'm sure there's a nicer way...

The natural way would be to overload inet_csk_accept for tcp,
but sk1->sk_prot->accept() is called with sk2->sk_socket == NULL,
because sock_graft() is called later...

metze

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 3dd02396517d..0f35f2a32756 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -736,6 +736,7 @@ EXPORT_SYMBOL(inet_stream_connect);
   *	Accept a pending connection. The TCP layer now gives BSD semantics.
   */

+#include <net/transp_v6.h>
  int inet_accept(struct socket *sock, struct socket *newsock, int flags,
  		bool kern)
  {
@@ -754,6 +755,8 @@ int inet_accept(struct socket *sock, struct socket *newsock, int flags,
  		  (TCPF_ESTABLISHED | TCPF_SYN_RECV |
  		  TCPF_CLOSE_WAIT | TCPF_CLOSE)));

+	if (sk2->sk_prot == &tcp_prot || sk2->sk_prot == &tcpv6_prot)
+		set_bit(SOCK_SUPPORT_ZC, &newsock->flags);
  	sock_graft(sk2, newsock);

  	newsock->state = SS_CONNECTED;

Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Pavel Begunkov]

On 10/24/22 15:12, Stefan Metzmacher wrote:
> Am 24.10.22 um 14:49 schrieb Stefan Metzmacher:
>> Am 22.10.22 um 18:07 schrieb Jens Axboe:
>>> On 10/22/22 9:57 AM, Pavel Begunkov wrote:
>>>> On 10/21/22 17:14, Jakub Kicinski wrote:
>>>>> On Fri, 21 Oct 2022 11:16:39 +0100 Pavel Begunkov wrote:
>>>>>> We need an efficient way in io_uring to check whether a socket supports
>>>>>> zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
>>>>>> socket flags fields.
>>>>>>
>>>>>> Cc: <stable@vger.kernel.org> # 6.0
>>>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>>>> ---
>>>>>> ? include/linux/net.h | 1 +
>>>>>> ? net/ipv4/tcp.c????? | 1 +
>>>>>> ? net/ipv4/udp.c????? | 1 +
>>>>>> ? 3 files changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/include/linux/net.h b/include/linux/net.h
>>>>>> index 711c3593c3b8..18d942bbdf6e 100644
>>>>>> --- a/include/linux/net.h
>>>>>> +++ b/include/linux/net.h
>>>>>> @@ -41,6 +41,7 @@ struct net;
>>>>>> ? #define SOCK_NOSPACE??????? 2
>>>>>> ? #define SOCK_PASSCRED??????? 3
>>>>>> ? #define SOCK_PASSSEC??????? 4
>>>>>> +#define SOCK_SUPPORT_ZC??????? 5
>>>>>
>>>>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>>>>>
>>>>> Any idea on when this will make it to Linus? If within a week we can
>>>>> probably delay:
>>>>
>>>> After a chat with Jens, IIUC he can take it and send out to
>>>> Linus early. So, sounds like a good plan
>>>
>>> Yes, and let's retain the name for now, can always be changed if we need
>>> to make it more granular. I'll ship this off before -rc2.
>>
>> I'm now always getting -EOPNOTSUPP from SENDMSG_ZC for tcp connections...
> 
> The problem is that inet_accept() doesn't set SOCK_SUPPORT_ZC...

Yeah, you're right, accept is not handled, not great. Thanks
for tracking it down. And I'll add a test for it


> This hack below fixes it for me, but I'm sure there's a nicer way...
> 
> The natural way would be to overload inet_csk_accept for tcp,
> but sk1->sk_prot->accept() is called with sk2->sk_socket == NULL,
> because sock_graft() is called later...

I think it's acceptable for a fix. sk_is_tcp() sounds a bit better
assuming that sk_type/protocol are set there.


> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 3dd02396517d..0f35f2a32756 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -736,6 +736,7 @@ EXPORT_SYMBOL(inet_stream_connect);
>    *    Accept a pending connection. The TCP layer now gives BSD semantics.
>    */
> 
> +#include <net/transp_v6.h>
>   int inet_accept(struct socket *sock, struct socket *newsock, int flags,
>           bool kern)
>   {
> @@ -754,6 +755,8 @@ int inet_accept(struct socket *sock, struct socket *newsock, int flags,
>             (TCPF_ESTABLISHED | TCPF_SYN_RECV |
>             TCPF_CLOSE_WAIT | TCPF_CLOSE)));
> 
> +    if (sk2->sk_prot == &tcp_prot || sk2->sk_prot == &tcpv6_prot)
> +        set_bit(SOCK_SUPPORT_ZC, &newsock->flags);
>       sock_graft(sk2, newsock);
> 
>       newsock->state = SS_CONNECTED;

-- 
Pavel Begunkov

Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr originated zerocopy

[Stefan Metzmacher]

Am 24.10.22 um 19:00 schrieb Pavel Begunkov:
> On 10/24/22 15:12, Stefan Metzmacher wrote:
>> Am 24.10.22 um 14:49 schrieb Stefan Metzmacher:
>>> Am 22.10.22 um 18:07 schrieb Jens Axboe:
>>>> On 10/22/22 9:57 AM, Pavel Begunkov wrote:
>>>>> On 10/21/22 17:14, Jakub Kicinski wrote:
>>>>>> On Fri, 21 Oct 2022 11:16:39 +0100 Pavel Begunkov wrote:
>>>>>>> We need an efficient way in io_uring to check whether a socket supports
>>>>>>> zerocopy with msghdr provided ubuf_info. Add a new flag into the struct
>>>>>>> socket flags fields.
>>>>>>>
>>>>>>> Cc: <stable@vger.kernel.org> # 6.0
>>>>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>>>>> ---
>>>>>>> ? include/linux/net.h | 1 +
>>>>>>> ? net/ipv4/tcp.c????? | 1 +
>>>>>>> ? net/ipv4/udp.c????? | 1 +
>>>>>>> ? 3 files changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/include/linux/net.h b/include/linux/net.h
>>>>>>> index 711c3593c3b8..18d942bbdf6e 100644
>>>>>>> --- a/include/linux/net.h
>>>>>>> +++ b/include/linux/net.h
>>>>>>> @@ -41,6 +41,7 @@ struct net;
>>>>>>> ? #define SOCK_NOSPACE??????? 2
>>>>>>> ? #define SOCK_PASSCRED??????? 3
>>>>>>> ? #define SOCK_PASSSEC??????? 4
>>>>>>> +#define SOCK_SUPPORT_ZC??????? 5
>>>>>>
>>>>>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>>>>>>
>>>>>> Any idea on when this will make it to Linus? If within a week we can
>>>>>> probably delay:
>>>>>
>>>>> After a chat with Jens, IIUC he can take it and send out to
>>>>> Linus early. So, sounds like a good plan
>>>>
>>>> Yes, and let's retain the name for now, can always be changed if we need
>>>> to make it more granular. I'll ship this off before -rc2.
>>>
>>> I'm now always getting -EOPNOTSUPP from SENDMSG_ZC for tcp connections...
>>
>> The problem is that inet_accept() doesn't set SOCK_SUPPORT_ZC...
> 
> Yeah, you're right, accept is not handled, not great. Thanks
> for tracking it down. And I'll add a test for it
> 
> 
>> This hack below fixes it for me, but I'm sure there's a nicer way...
>>
>> The natural way would be to overload inet_csk_accept for tcp,
>> but sk1->sk_prot->accept() is called with sk2->sk_socket == NULL,
>> because sock_graft() is called later...
> 
> I think it's acceptable for a fix. sk_is_tcp() sounds a bit better
> assuming that sk_type/protocol are set there.
> 
> 
>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>> index 3dd02396517d..0f35f2a32756 100644
>> --- a/net/ipv4/af_inet.c
>> +++ b/net/ipv4/af_inet.c
>> @@ -736,6 +736,7 @@ EXPORT_SYMBOL(inet_stream_connect);
>>    *    Accept a pending connection. The TCP layer now gives BSD semantics.
>>    */
>>
>> +#include <net/transp_v6.h>
>>   int inet_accept(struct socket *sock, struct socket *newsock, int flags,
>>           bool kern)
>>   {
>> @@ -754,6 +755,8 @@ int inet_accept(struct socket *sock, struct socket *newsock, int flags,
>>             (TCPF_ESTABLISHED | TCPF_SYN_RECV |
>>             TCPF_CLOSE_WAIT | TCPF_CLOSE)));
>>
>> +    if (sk2->sk_prot == &tcp_prot || sk2->sk_prot == &tcpv6_prot)
>> +        set_bit(SOCK_SUPPORT_ZC, &newsock->flags);
>>       sock_graft(sk2, newsock);

Hmm, I think we just need to check if the flag was set on the listening socket,
otherwise we have a hard coded value again...

This also works for me, and seems like a much nicer patch:

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 3dd02396517d..4728087c42a5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -754,6 +754,8 @@ int inet_accept(struct socket *sock, struct socket *newsock, int flags,
  		  (TCPF_ESTABLISHED | TCPF_SYN_RECV |
  		  TCPF_CLOSE_WAIT | TCPF_CLOSE)));

+	if (test_bit(SOCK_SUPPORT_ZC, &sock->flags))
+		set_bit(SOCK_SUPPORT_ZC, &newsock->flags);
  	sock_graft(sk2, newsock);

  	newsock->state = SS_CONNECTED;

[PATCH for-6.1 2/3] io_uring/net: fail zc send when unsupported by socket

[Pavel Begunkov]

If a protocol doesn't support zerocopy it will silently fall back to
copying. This type of behaviour has always been a source of troubles
so it's better to fail such requests instead.

Cc: <stable@vger.kernel.org> # 6.0
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/net.c b/io_uring/net.c
index 8c7226b5bf41..26ff3675214d 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1056,6 +1056,8 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
 	sock = sock_from_file(req->file);
 	if (unlikely(!sock))
 		return -ENOTSOCK;
+	if (!test_bit(SOCK_SUPPORT_ZC, &sock->flags))
+		return -EOPNOTSUPP;
 
 	msg.msg_name = NULL;
 	msg.msg_control = NULL;
-- 
2.38.0

[PATCH for-6.1 3/3] io_uring/net: fail zc sendmsg when unsupported by socket

[Pavel Begunkov]

The previous patch fails zerocopy send requests for protocols that don't
support it, do the same for zerocopy sendmsg.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/net.c b/io_uring/net.c
index 26ff3675214d..15dea91625e2 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1153,6 +1153,8 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
 	sock = sock_from_file(req->file);
 	if (unlikely(!sock))
 		return -ENOTSOCK;
+	if (!test_bit(SOCK_SUPPORT_ZC, &sock->flags))
+		return -EOPNOTSUPP;
 
 	if (req_has_async_data(req)) {
 		kmsg = req->async_data;
-- 
2.38.0

Re: [PATCH for-6.1 0/3] fail io_uring zc with sockets not supporting it

[Stefan Metzmacher]

Hi Pavel,

> Some sockets don't care about msghdr::ubuf_info and would execute the
> request by copying data. Such fallback behaviour was always a pain in
> my experience, so we'd rather want to fail such requests and have a more
> robust api in the future.
> 
> Mark struct socket that support it with a new SOCK_SUPPORT_ZC flag.
> I'm not entirely sure it's the best place for the flag but at least
> we don't have to do a bunch of extra dereferences in the hot path.

I'd give the flag another name that indicates msg_ubuf and
have a 2nd flag that can indicate support for SO_ZEROCOPY in sk_setsockopt()

The SO_ZEROCOPY version is also provided by AF_RDS.

metze

Re: [PATCH for-6.1 0/3] fail io_uring zc with sockets not supporting it

[Pavel Begunkov]

On 10/21/22 11:27, Stefan Metzmacher wrote:
> Hi Pavel,
> 
>> Some sockets don't care about msghdr::ubuf_info and would execute the
>> request by copying data. Such fallback behaviour was always a pain in
>> my experience, so we'd rather want to fail such requests and have a more
>> robust api in the future.
>>
>> Mark struct socket that support it with a new SOCK_SUPPORT_ZC flag.
>> I'm not entirely sure it's the best place for the flag but at least
>> we don't have to do a bunch of extra dereferences in the hot path.
> 
> I'd give the flag another name that indicates msg_ubuf and

Could be renamed, e.g. SOCK_SUPPORT_MSGHDR_UBUF or maybe
SOCK_SUPPORT_EXTERNAL_UBUF

> have a 2nd flag that can indicate support for SO_ZEROCOPY in sk_setsockopt()

There is absolutely no reason to introduce a second flag here, it has
nothing to do with SO_ZEROCOPY.

> The SO_ZEROCOPY version is also provided by AF_RDS.

-- 
Pavel Begunkov

Re: [PATCH for-6.1 0/3] fail io_uring zc with sockets not supporting it

[Stefan Metzmacher]

Am 21.10.22 um 12:42 schrieb Pavel Begunkov:
> On 10/21/22 11:27, Stefan Metzmacher wrote:
>> Hi Pavel,
>>
>>> Some sockets don't care about msghdr::ubuf_info and would execute the
>>> request by copying data. Such fallback behaviour was always a pain in
>>> my experience, so we'd rather want to fail such requests and have a more
>>> robust api in the future.
>>>
>>> Mark struct socket that support it with a new SOCK_SUPPORT_ZC flag.
>>> I'm not entirely sure it's the best place for the flag but at least
>>> we don't have to do a bunch of extra dereferences in the hot path.
>>
>> I'd give the flag another name that indicates msg_ubuf and
> 
> Could be renamed, e.g. SOCK_SUPPORT_MSGHDR_UBUF 

That's good or SOCK_SUPPORT_ZC_MSGHDR_UBUF.

>> have a 2nd flag that can indicate support for SO_ZEROCOPY in sk_setsockopt()
> 
> There is absolutely no reason to introduce a second flag here, it has
> nothing to do with SO_ZEROCOPY.

I meant as a separate change to replace the hard coded logic in
sk_setsockopt()... But I don't care much about it, it's unlikely
that I ever want to use SO_ZEROCOPY...

metze

Re: [PATCH for-6.1 0/3] fail io_uring zc with sockets not supporting it

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by Jens Axboe <axboe@kernel.dk>:

On Fri, 21 Oct 2022 11:16:38 +0100 you wrote:
> Some sockets don't care about msghdr::ubuf_info and would execute the
> request by copying data. Such fallback behaviour was always a pain in
> my experience, so we'd rather want to fail such requests and have a more
> robust api in the future.
> 
> Mark struct socket that support it with a new SOCK_SUPPORT_ZC flag.
> I'm not entirely sure it's the best place for the flag but at least
> we don't have to do a bunch of extra dereferences in the hot path.
> 
> [...]

Here is the summary with links:
  - [for-6.1,1/3] net: flag sockets supporting msghdr originated zerocopy
    https://git.kernel.org/netdev/net/c/e993ffe3da4b
  - [for-6.1,2/3] io_uring/net: fail zc send when unsupported by socket
    https://git.kernel.org/netdev/net/c/edf81438799c
  - [for-6.1,3/3] io_uring/net: fail zc sendmsg when unsupported by socket
    https://git.kernel.org/netdev/net/c/cc767e7c6913

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH for-6.1 0/3] fail io_uring zc with sockets not supporting it

[Jakub Kicinski]

On Mon, 24 Oct 2022 20:40:38 +0000 patchwork-bot+netdevbpf@kernel.org
wrote:
> Here is the summary with links:
>   - [for-6.1,1/3] net: flag sockets supporting msghdr originated zerocopy
>     https://git.kernel.org/netdev/net/c/e993ffe3da4b
>   - [for-6.1,2/3] io_uring/net: fail zc send when unsupported by socket
>     https://git.kernel.org/netdev/net/c/edf81438799c
>   - [for-6.1,3/3] io_uring/net: fail zc sendmsg when unsupported by socket
>     https://git.kernel.org/netdev/net/c/cc767e7c6913

Ugh, ignore, looks like we forgot to drop this series from our PW 
and bot thought we've applied it when we forwarded the tree.