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.