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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <62b6de98-026c-115f-ebae-950cb5501139@samba.org> Date: Mon, 24 Oct 2022 19:45:42 +0200 From: Stefan Metzmacher <metze@...ba.org> To: Pavel Begunkov <asml.silence@...il.com>, Jens Axboe <axboe@...nel.dk>, 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 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@...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... > > 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;
Powered by blists - more mailing lists