From: Mina Almasry <almasrymina@google.com>
To: David Wei <dw@davidwei.uk>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, linux-alpha@vger.kernel.org,
linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org,
sparclinux@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
linux-arch@vger.kernel.org, bpf@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-media@vger.kernel.org,
dri-devel@lists.freedesktop.org,
"Donald Hunter" <donald.hunter@gmail.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Paolo Abeni" <pabeni@redhat.com>,
"Jonathan Corbet" <corbet@lwn.net>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Ivan Kokshaysky" <ink@jurassic.park.msu.ru>,
"Matt Turner" <mattst88@gmail.com>,
"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
"Helge Deller" <deller@gmx.de>,
"Andreas Larsson" <andreas@gaisler.com>,
"Jesper Dangaard Brouer" <hawk@kernel.org>,
"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
"Steven Rostedt" <rostedt@goodmis.org>,
"Masami Hiramatsu" <mhiramat@kernel.org>,
"Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Martin KaFai Lau" <martin.lau@linux.dev>,
"Eduard Zingerman" <eddyz87@gmail.com>,
"Song Liu" <song@kernel.org>,
"Yonghong Song" <yonghong.song@linux.dev>,
"John Fastabend" <john.fastabend@gmail.com>,
"KP Singh" <kpsingh@kernel.org>,
"Stanislav Fomichev" <sdf@google.com>,
"Hao Luo" <haoluo@google.com>, "Jiri Olsa" <jolsa@kernel.org>,
"Steffen Klassert" <steffen.klassert@secunet.com>,
"Herbert Xu" <herbert@gondor.apana.org.au>,
"David Ahern" <dsahern@kernel.org>,
"Willem de Bruijn" <willemdebruijn.kernel@gmail.com>,
"Shuah Khan" <shuah@kernel.org>,
"Sumit Semwal" <sumit.semwal@linaro.org>,
"Christian König" <christian.koenig@amd.com>,
"Pavel Begunkov" <asml.silence@gmail.com>,
"Jason Gunthorpe" <jgg@ziepe.ca>,
"Yunsheng Lin" <linyunsheng@huawei.com>,
"Shailend Chand" <shailend@google.com>,
"Harshitha Ramamurthy" <hramamurthy@google.com>,
"Shakeel Butt" <shakeel.butt@linux.dev>,
"Jeroen de Borst" <jeroendb@google.com>,
"Praveen Kaligineedi" <pkaligineedi@google.com>,
"Willem de Bruijn" <willemb@google.com>,
"Kaiyuan Zhang" <kaiyuanz@google.com>
Subject: Re: [PATCH net-next v9 11/14] tcp: RX path for devmem TCP
Date: Tue, 28 May 2024 10:36:40 -0700 [thread overview]
Message-ID: <CAHS8izOe-uYjm0ttQgHOFpvp_Tj4_oRHV6d1Y1sWJAZJdCdCBA@mail.gmail.com> (raw)
In-Reply-To: <9097e78d-0e7d-43bd-bafd-e53a4872a4d1@davidwei.uk>
On Wed, May 22, 2024 at 11:02 PM David Wei <dw@davidwei.uk> wrote:
>
> On 2024-05-10 16:21, Mina Almasry wrote:
> > +/* On error, returns the -errno. On success, returns number of bytes sent to the
> > + * user. May not consume all of @remaining_len.
> > + */
> > +static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb,
> > + unsigned int offset, struct msghdr *msg,
> > + int remaining_len)
> > +{
> > + struct dmabuf_cmsg dmabuf_cmsg = { 0 };
> > + struct tcp_xa_pool tcp_xa_pool;
> > + unsigned int start;
> > + int i, copy, n;
> > + int sent = 0;
> > + int err = 0;
> > +
> > + tcp_xa_pool.max = 0;
> > + tcp_xa_pool.idx = 0;
> > + do {
> > + start = skb_headlen(skb);
> > +
> > + if (skb_frags_readable(skb)) {
> > + err = -ENODEV;
> > + goto out;
> > + }
> > +
> > + /* Copy header. */
> > + copy = start - offset;
> > + if (copy > 0) {
> > + copy = min(copy, remaining_len);
> > +
> > + n = copy_to_iter(skb->data + offset, copy,
> > + &msg->msg_iter);
> > + if (n != copy) {
> > + err = -EFAULT;
> > + goto out;
> > + }
> > +
> > + offset += copy;
> > + remaining_len -= copy;
> > +
> > + /* First a dmabuf_cmsg for # bytes copied to user
> > + * buffer.
> > + */
> > + memset(&dmabuf_cmsg, 0, sizeof(dmabuf_cmsg));
> > + dmabuf_cmsg.frag_size = copy;
> > + err = put_cmsg(msg, SOL_SOCKET, SO_DEVMEM_LINEAR,
> > + sizeof(dmabuf_cmsg), &dmabuf_cmsg);
> > + if (err || msg->msg_flags & MSG_CTRUNC) {
> > + msg->msg_flags &= ~MSG_CTRUNC;
> > + if (!err)
> > + err = -ETOOSMALL;
> > + goto out;
> > + }
> > +
> > + sent += copy;
> > +
> > + if (remaining_len == 0)
> > + goto out;
> > + }
> > +
> > + /* after that, send information of dmabuf pages through a
> > + * sequence of cmsg
> > + */
> > + for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> > + skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
> > + struct net_iov *niov;
> > + u64 frag_offset;
> > + int end;
> > +
> > + /* !skb_frags_readable() should indicate that ALL the
> > + * frags in this skb are dmabuf net_iovs. We're checking
> > + * for that flag above, but also check individual frags
> > + * here. If the tcp stack is not setting
> > + * skb_frags_readable() correctly, we still don't want
> > + * to crash here.
> > + */
> > + if (!skb_frag_net_iov(frag)) {
> > + net_err_ratelimited("Found non-dmabuf skb with net_iov");
> > + err = -ENODEV;
> > + goto out;
> > + }
> > +
> > + niov = skb_frag_net_iov(frag);
>
> Sorry if we've already discussed this.
>
> We have this additional hunk:
>
> + if (niov->pp->mp_ops != &dmabuf_devmem_ops) {
> + err = -ENODEV;
> + goto out;
> + }
>
> In case one of our skbs end up here, skb_frag_is_net_iov() and
> !skb_frags_readable(). Does this even matter? And if so then is there a
> better way to distinguish between our two types of net_iovs?
Thanks for bringing this up, yes, maybe we do need a way to
distinguish, but it's not 100% critical, no? It's mostly for debug
checking?
I would say add a helper, like net_iov_is_dmabuf() or net_iov_is_io_uring().
Checking for niov->pp->mp_ops seems a bit hacky to me, and may be
outright broken. IIRC niov's can be disconnected from the page_pool
via page_pool_clear_pp_info(), and niov->pp may be null. Abstractly
speaking the niov type maybe should be a property of the niov itself,
and not the pp the niov is attached to.
It is not immediately obvious to me what the best thing to do here is,
maybe it's best to add a flag to niov or to use niov->pp_magic for
this.
I would humbly ask that your follow up patchset takes care of this
bit, if possible. I think mine is doing quite a bit of heavy lifting
as is (and I think may be close to ready?), when it comes to concerns
of devmem + io_uring coexisting if you're able to take care, awesome,
if not, I can look into squashing some fix.
--
Thanks,
Mina
next prev parent reply other threads:[~2024-05-28 17:36 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 23:21 [PATCH net-next v9 00/14] Device Memory TCP Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 01/14] netdev: add netdev_rx_queue_restart() Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 02/14] net: page_pool: create hooks for custom page providers Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 03/14] net: netdev netlink api to bind dma-buf to a net device Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 04/14] netdev: support binding dma-buf to netdevice Mina Almasry
2024-05-15 10:01 ` Nikolay Aleksandrov
2024-05-15 10:19 ` Nikolay Aleksandrov
2024-05-18 18:46 ` David Wei
2024-05-29 19:49 ` Mina Almasry
2024-05-18 18:57 ` David Wei
2024-05-10 23:21 ` [PATCH net-next v9 05/14] netdev: netdevice devmem allocator Mina Almasry
2024-05-18 0:39 ` David Wei
2024-05-10 23:21 ` [PATCH net-next v9 06/14] page_pool: convert to use netmem Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 07/14] page_pool: devmem support Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 08/14] memory-provider: dmabuf devmem memory provider Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 09/14] net: support non paged skb frags Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 10/14] net: add support for skbs with unreadable frags Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 11/14] tcp: RX path for devmem TCP Mina Almasry
2024-05-23 6:02 ` David Wei
2024-05-28 17:36 ` Mina Almasry [this message]
2024-05-29 2:42 ` Pavel Begunkov
2024-05-29 17:20 ` Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 12/14] net: add SO_DEVMEM_DONTNEED setsockopt to release RX frags Mina Almasry
2024-05-15 9:52 ` Nikolay Aleksandrov
2024-05-10 23:21 ` [PATCH net-next v9 13/14] net: add devmem TCP documentation Mina Almasry
2024-05-10 23:21 ` [PATCH net-next v9 14/14] selftests: add ncdevmem, netcat for devmem TCP Mina Almasry
2024-05-13 23:31 ` [PATCH net-next v9 00/14] Device Memory TCP Jakub Kicinski
2024-05-14 17:15 ` Mina Almasry
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHS8izOe-uYjm0ttQgHOFpvp_Tj4_oRHV6d1Y1sWJAZJdCdCBA@mail.gmail.com \
--to=almasrymina@google.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=andreas@gaisler.com \
--cc=andrii@kernel.org \
--cc=arnd@arndb.de \
--cc=asml.silence@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=christian.koenig@amd.com \
--cc=corbet@lwn.net \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=deller@gmx.de \
--cc=donald.hunter@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=dsahern@kernel.org \
--cc=dw@davidwei.uk \
--cc=eddyz87@gmail.com \
--cc=edumazet@google.com \
--cc=haoluo@google.com \
--cc=hawk@kernel.org \
--cc=herbert@gondor.apana.org.au \
--cc=hramamurthy@google.com \
--cc=ilias.apalodimas@linaro.org \
--cc=ink@jurassic.park.msu.ru \
--cc=jeroendb@google.com \
--cc=jgg@ziepe.ca \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kaiyuanz@google.com \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=linyunsheng@huawei.com \
--cc=martin.lau@linux.dev \
--cc=mathieu.desnoyers@efficios.com \
--cc=mattst88@gmail.com \
--cc=mhiramat@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pkaligineedi@google.com \
--cc=richard.henderson@linaro.org \
--cc=rostedt@goodmis.org \
--cc=sdf@google.com \
--cc=shailend@google.com \
--cc=shakeel.butt@linux.dev \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=sparclinux@vger.kernel.org \
--cc=steffen.klassert@secunet.com \
--cc=sumit.semwal@linaro.org \
--cc=tsbogend@alpha.franken.de \
--cc=willemb@google.com \
--cc=willemdebruijn.kernel@gmail.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).