| 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: <20240827190805.7f82deb0@kernel.org>
Date: Tue, 27 Aug 2024 19:08:05 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Mina Almasry <almasrymina@...gle.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, linux-alpha@...r.kernel.org,
linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
sparclinux@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
linux-arch@...r.kernel.org, bpf@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-media@...r.kernel.org,
dri-devel@...ts.freedesktop.org, Donald Hunter <donald.hunter@...il.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Jonathan Corbet
<corbet@....net>, Richard Henderson <richard.henderson@...aro.org>, Ivan
Kokshaysky <ink@...assic.park.msu.ru>, Matt Turner <mattst88@...il.com>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>, "James E.J. Bottomley"
<James.Bottomley@...senPartnership.com>, Helge Deller <deller@....de>,
Andreas Larsson <andreas@...sler.com>, Jesper Dangaard Brouer
<hawk@...nel.org>, Ilias Apalodimas <ilias.apalodimas@...aro.org>, Steven
Rostedt <rostedt@...dmis.org>, Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Arnd Bergmann
<arnd@...db.de>, Steffen Klassert <steffen.klassert@...unet.com>, Herbert
Xu <herbert@...dor.apana.org.au>, David Ahern <dsahern@...nel.org>, Willem
de Bruijn <willemdebruijn.kernel@...il.com>, "Björn Töpel" <bjorn@...nel.org>, Magnus Karlsson
<magnus.karlsson@...el.com>, Maciej Fijalkowski
<maciej.fijalkowski@...el.com>, Jonathan Lemon <jonathan.lemon@...il.com>,
Shuah Khan <shuah@...nel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel
Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>,
Sumit Semwal <sumit.semwal@...aro.org>, "Christian König"
<christian.koenig@....com>, Pavel Begunkov <asml.silence@...il.com>, David
Wei <dw@...idwei.uk>, Jason Gunthorpe <jgg@...pe.ca>, Yunsheng Lin
<linyunsheng@...wei.com>, Shailend Chand <shailend@...gle.com>, Harshitha
Ramamurthy <hramamurthy@...gle.com>, Shakeel Butt <shakeel.butt@...ux.dev>,
Jeroen de Borst <jeroendb@...gle.com>, Praveen Kaligineedi
<pkaligineedi@...gle.com>, Bagas Sanjaya <bagasdotme@...il.com>, Christoph
Hellwig <hch@...radead.org>, Nikolay Aleksandrov <razor@...ckwall.org>,
Taehee Yoo <ap420073@...il.com>, Willem de Bruijn <willemb@...gle.com>,
Kaiyuan Zhang <kaiyuanz@...gle.com>, Daniel Vetter <daniel.vetter@...ll.ch>
Subject: Re: [PATCH net-next v22 03/13] netdev: support binding dma-buf to
netdevice
On Sun, 25 Aug 2024 04:15:01 +0000 Mina Almasry wrote:
> +u32 dev_get_min_mp_channel_count(const struct net_device *dev)
> +{
> + u32 i, max = 0;
> +
> + ASSERT_RTNL();
> +
> + for (i = 0; i < dev->real_num_rx_queues; i++)
> + if (dev->_rx[i].mp_params.mp_priv)
> + /* The channel count is the idx plus 1. */
> + max = i + 1;
invert the loop so you're walking from highest indexes and you can
return i + 1;
return 0;
> + return max;
> +}
> +
> /**
> * dev_index_reserve() - allocate an ifindex in a namespace
> * @net: the applicable net namespace
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> +#include <linux/types.h>
> +#include <linux/mm.h>
> +#include <linux/netdevice.h>
> +#include <trace/events/page_pool.h>
> +#include <net/netdev_rx_queue.h>
> +#include <net/page_pool/types.h>
> +#include <net/page_pool/helpers.h>
> +#include <linux/genalloc.h>
> +#include <linux/dma-buf.h>
> +#include <net/devmem.h>
> +#include <net/netdev_queues.h>
Please sort include files alphabetically.
> +#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR)
Could you create a hidden Kconfig for this feature and use it to make
building this entire file conditional? Hidden Kconfig has no
description and no help, like config NET_DEVLINK, but it can have
dependencies.
> +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->list.next)
> + list_del(&binding->list);
> +
> + xa_for_each(&binding->bound_rxqs, xa_idx, rxq) {
> + if (rxq->mp_params.mp_priv == binding) {
WARN_ON(rxq->mp_params.mp_priv != binding) ?
We know we're bound to this queue, nobody should be able to replace
the mp, right?
> + rxq->mp_params.mp_priv = NULL;
> +
> + rxq_idx = get_netdev_rx_queue_index(rxq);
> +
> + WARN_ON(netdev_rx_queue_restart(binding->dev, rxq_idx));
> + }
> + }
> +
> + xa_erase(&net_devmem_dmabuf_bindings, binding->id);
> +
> + net_devmem_dmabuf_binding_put(binding);
> +}
> +
> +int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
> + struct net_devmem_dmabuf_binding *binding)
> +{
> + struct netdev_rx_queue *rxq;
> + u32 xa_idx;
> + int err;
> +
> + if (rxq_idx >= dev->real_num_rx_queues)
> + return -ERANGE;
> +
> + rxq = __netif_get_rx_queue(dev, rxq_idx);
> + if (rxq->mp_params.mp_priv)
> + return -EEXIST;
> +
> +#ifdef CONFIG_XDP_SOCKETS
> + if (rxq->pool)
> + return -EEXIST;
EBUSY plus extack "designated queue already in use by AF_XDP"
> +#endif
> +
> + if (dev_xdp_prog_count(dev))
> + return -EEXIST;
Also needs an extack, but since it's not queue-specific should
it not live inside net_devmem_bind_dmabuf() ? Or do you anticipate
reuse of this function by non-dmabuf code?
> +void dev_dmabuf_uninstall(struct net_device *dev)
> +{
> + struct net_devmem_dmabuf_binding *binding;
> + struct netdev_rx_queue *rxq;
> + unsigned long xa_idx;
> + unsigned int i;
> +
> + for (i = 0; i < dev->real_num_rx_queues; i++) {
> + binding = dev->_rx[i].mp_params.mp_priv;
> + if (!binding)
> + continue;
> +
> + xa_for_each(&binding->bound_rxqs, xa_idx, rxq)
> + if (rxq == &dev->_rx[i])
> + xa_erase(&binding->bound_rxqs, xa_idx);
break;
I don't think we can store the same queue twice
> + }
> +}
> +#endif
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 2d726e65211d..269faa37f84e 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -10,6 +10,7 @@
> #include <net/netdev_rx_queue.h>
> #include <net/netdev_queues.h>
> #include <net/busy_poll.h>
> +#include <net/devmem.h>
include order
> + return genlmsg_reply(rsp, info);
Should we goto err_unbind if genlmsg_reply() fails?
Shouldn't really happen unless socket is full but simple enough to fix.
Powered by blists - more mailing lists