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 04/14] netdev: support binding dma-buf to netdevice
Date: Wed, 29 May 2024 12:49:08 -0700 [thread overview]
Message-ID: <CAHS8izPVhDaokO9C+S4RR9b6+77OV2CsNb8jnGGKxNqGTa6DXg@mail.gmail.com> (raw)
In-Reply-To: <d85f4ba4-774f-4577-985f-45a5a1866da7@davidwei.uk>
On Sat, May 18, 2024 at 11:46 AM David Wei <dw@davidwei.uk> wrote:
>
> On 2024-05-10 16:21, Mina Almasry wrote:
> > +void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
> > +{
> > + struct netdev_rx_queue *rxq;
> > + unsigned long xa_idx;
> > + unsigned int rxq_idx;
> > +
> > + if (!binding)
> > + return;
> > +
> > + if (binding->list.next)
> > + list_del(&binding->list);
> > +
> > + xa_for_each(&binding->bound_rxq_list, xa_idx, rxq) {
> > + if (rxq->mp_params.mp_priv == binding) {
> > + /* We hold the rtnl_lock while binding/unbinding
> > + * dma-buf, so we can't race with another thread that
> > + * is also modifying this value. However, the page_pool
> > + * may read this config while it's creating its
> > + * rx-queues. WRITE_ONCE() here to match the
> > + * READ_ONCE() in the page_pool.
> > + */
> > + WRITE_ONCE(rxq->mp_params.mp_ops, NULL);
> > + WRITE_ONCE(rxq->mp_params.mp_priv, NULL);
> > +
> > + rxq_idx = get_netdev_rx_queue_index(rxq);
> > +
> > + netdev_rx_queue_restart(binding->dev, rxq_idx);
>
> What if netdev_rx_queue_restart() fails? Depending on where it failed, a
> queue might still be filled from struct net_devmem_dmabuf_binding. This
> is one downside of the current situation with netdev_rx_queue_restart()
> needing to do allocations each time.
>
> Perhaps a full reset if individual queue restart fails?
>
Sorry for the late reply, I've been out on vacation for a few days and
caught up to some other work.
Yes, netdev_rx_queue_restart() can fail, but I'm not sure how to
recover. Full reset would be an option, but it may be way too big of a
hammer to do a full reset on this failure. Also, last I discussed with
Jakub, AFAIU, there is no way for core to reset the driver? I had
suggested to Jakub to use ndo_stop/ndo_open to reset the driver on
queue binding/unbinding, but he rejected that as it could cause the
driver to fail to come back up, which would leave the machine stranded
from the network. This is why we implemented the queue API, as a way
to do the binding/unbinding without risking the machine stranding via
a full reset. This is the previous convo from months back[1].
So, all in all, I don't see anything amazing we can do here to
recover. How about just log? I will add a warning in the next
iteration.
(I applied most of the rest of your suggestions btw).
[1] https://patchwork.kernel.org/project/netdevbpf/patch/20231106024413.2801438-13-almasrymina@google.com/#25590262
--
Thanks,
Mina
next prev parent reply other threads:[~2024-05-29 19:49 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 [this message]
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
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=CAHS8izPVhDaokO9C+S4RR9b6+77OV2CsNb8jnGGKxNqGTa6DXg@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).