From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9AE97C433E3 for ; Tue, 28 Jul 2020 02:11:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7D21120759 for ; Tue, 28 Jul 2020 02:11:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726842AbgG1CLg (ORCPT ); Mon, 27 Jul 2020 22:11:36 -0400 Received: from smtp1.emailarray.com ([65.39.216.14]:18653 "EHLO smtp1.emailarray.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726298AbgG1CLg (ORCPT ); Mon, 27 Jul 2020 22:11:36 -0400 Received: (qmail 98317 invoked by uid 89); 28 Jul 2020 02:11:34 -0000 Received: from unknown (HELO localhost) (amxlbW9uQGZsdWdzdmFtcC5jb21AMTYzLjExNC4xMzIuMw==) (POLARISLOCAL) by smtp1.emailarray.com with SMTP; 28 Jul 2020 02:11:34 -0000 Date: Mon, 27 Jul 2020 19:11:30 -0700 From: Jonathan Lemon To: Eric Dumazet Cc: netdev , kernel-team , Christoph Hellwig , Robin Murphy , Andrew Morton , David Miller , Jakub Kicinski , Willem de Bruijn , Steffen Klassert , Saeed Mahameed , Maxim Mikityanskiy , bjorn.topel@intel.com, magnus.karlsson@intel.com, borisp@mellanox.com, david@redhat.com Subject: Re: [RFC PATCH v2 15/21] net/tcp: add MSG_NETDMA flag for sendmsg() Message-ID: <20200728021130.bjrlcj7tzebfxsz3@bsd-mbp.dhcp.thefacebook.com> References: <20200727052846.4070247-1-jonathan.lemon@gmail.com> <20200727052846.4070247-16-jonathan.lemon@gmail.com> <20200727155549.gbwosugbugknsneo@bsd-mbp.dhcp.thefacebook.com> <20200727173528.tfsrweswpyjxlqv6@bsd-mbp.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, Jul 27, 2020 at 10:44:59AM -0700, Eric Dumazet wrote: > On Mon, Jul 27, 2020 at 10:35 AM Jonathan Lemon > wrote: > > > > On Mon, Jul 27, 2020 at 09:09:48AM -0700, Eric Dumazet wrote: > > > On Mon, Jul 27, 2020 at 8:56 AM Jonathan Lemon wrote: > > > > > > > > On Mon, Jul 27, 2020 at 08:19:43AM -0700, Eric Dumazet wrote: > > > > > On Mon, Jul 27, 2020 at 12:51 AM Jonathan Lemon > > > > > wrote: > > > > > > > > > > > > This flag indicates that the attached data is a zero-copy send, > > > > > > and the pages should be retrieved from the netgpu module. The > > > > > > socket should should already have been attached to a netgpu queue. > > > > > > > > > > > > Signed-off-by: Jonathan Lemon > > > > > > --- > > > > > > include/linux/socket.h | 1 + > > > > > > net/ipv4/tcp.c | 8 ++++++++ > > > > > > 2 files changed, 9 insertions(+) > > > > > > > > > > > > diff --git a/include/linux/socket.h b/include/linux/socket.h > > > > > > index 04d2bc97f497..63816cc25dee 100644 > > > > > > --- a/include/linux/socket.h > > > > > > +++ b/include/linux/socket.h > > > > > > @@ -310,6 +310,7 @@ struct ucred { > > > > > > */ > > > > > > > > > > > > #define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */ > > > > > > +#define MSG_NETDMA 0x8000000 > > > > > > #define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */ > > > > > > #define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file > > > > > > descriptor received through > > > > > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > > > > > > index 261c28ccc8f6..340ce319edc9 100644 > > > > > > --- a/net/ipv4/tcp.c > > > > > > +++ b/net/ipv4/tcp.c > > > > > > @@ -1214,6 +1214,14 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size) > > > > > > uarg->zerocopy = 0; > > > > > > } > > > > > > > > > > > > + if (flags & MSG_NETDMA && size && sock_flag(sk, SOCK_ZEROCOPY)) { > > > > > > + zc = sk->sk_route_caps & NETIF_F_SG; > > > > > > + if (!zc) { > > > > > > + err = -EFAULT; > > > > > > + goto out_err; > > > > > > + } > > > > > > + } > > > > > > > > > > > > > > > > Sorry, no, we can not allow adding yet another branch into TCP fast > > > > > path for yet another variant of zero copy. > > > > > > > > I'm not in disagreement with that statement, but the existing zerocopy > > > > work makes some assumptions that aren't suitable. I take it that you'd > > > > rather have things folded together so the old/new code works together? > > > > > > Exact. Forcing users to use MSG_NETDMA, yet reusing SOCK_ZEROCOPY is silly. > > > > > > SOCK_ZEROCOPY has been added to that user space and kernel would agree > > > on MSG_ZEROCOPY being not a nop (as it was on old kernels) > > > > > > > > > > > Allocating an extra structure for every skbuff isn't ideal in my book. > > > > > > > > > > We do not allocate a structure for every skbuff. Please look again. > > > > I'm looking here: > > > > uarg = sock_zerocopy_realloc(sk, size, skb_zcopy(skb)); > > > > Doesn't sock_zerocopy_realloc() allocate a new structure if the skb > > doesn't have one already? > > > > > > > > > Overall, I think your patch series desperately tries to add changes in > > > > > TCP stack, while there is yet no proof > > > > > that you have to use TCP transport between the peers. > > > > > > > > The goal is having a reliable transport without resorting to RDMA. > > > > > > And why should it be TCP ? > > > > > > Are you dealing with lost packets, retransmits, timers, and al ? > > > > Yes? If there was a true lossless medium, RDMA would have taken over by > > now. Or are you suggesting that the transport protocol reliability > > should be performed in userspace? (not all the world is QUIC yet) > > > > The thing is : this patch series is a monster thing adding stuff that > is going to impact 100% % of TCP flows, > even if not used in this NETDMA context. > > So you need to convince us you are really desperate to get this in > upstream linux. > > I have implemented TCP RX zero copy without adding a single line in > standard TCP code. That's a bit of an exaggeration, as I see skb_zcopy_*() calls scattered around the normal TCP code path. I also haven't changed the normal TCP path either, other than doing some of the same things as skb_zcopy_*(). (ignoring the ugly moron about padding out the TCP header, which I'll put under a static_branch_unlikely). The thing is, the existing zero copy code is zero-copy to /host/ memory, which is not the same thing as zero-copy to other memory areas. -- Jonathan