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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Oct 2022 16:12:31 +0200
From:   Stefan Metzmacher <metze@...ba.org>
To:     Jens Axboe <axboe@...nel.dk>,
        Pavel Begunkov <asml.silence@...il.com>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Paolo Abeni <pabeni@...hat.com>,
        "David S . Miller" <davem@...emloft.net>, io-uring@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH for-6.1 1/3] net: flag sockets supporting msghdr
 originated zerocopy

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@...r.kernel.org> # 6.0
>>>>> Signed-off-by: Pavel Begunkov <asml.silence@...il.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@...nel.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;