| Message ID | 20240628003253.1694510-4-almasrymina@google.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Device Memory TCP | expand |
On Fri, 2024-06-28 at 00:32 +0000, Mina Almasry wrote: > +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out) > +{ > + struct net_devmem_dmabuf_binding *binding; > + static u32 id_alloc_next; > + struct scatterlist *sg; > + struct dma_buf *dmabuf; > + unsigned int sg_idx, i; > + unsigned long virtual; > + int err; > + > + dmabuf = dma_buf_get(dmabuf_fd); > + if (IS_ERR(dmabuf)) > + return -EBADFD; > + > + binding = kzalloc_node(sizeof(*binding), GFP_KERNEL, > + dev_to_node(&dev->dev)); > + if (!binding) { > + err = -ENOMEM; > + goto err_put_dmabuf; > + } > + > + binding->dev = dev; > + > + err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id, > + binding, xa_limit_32b, &id_alloc_next, > + GFP_KERNEL); > + if (err < 0) > + goto err_free_binding; > + > + xa_init_flags(&binding->bound_rxq_list, XA_FLAGS_ALLOC); > + > + refcount_set(&binding->ref, 1); > + > + binding->dmabuf = dmabuf; > + > + binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent); > + if (IS_ERR(binding->attachment)) { > + err = PTR_ERR(binding->attachment); > + goto err_free_id; > + } > + > + binding->sgt = > + dma_buf_map_attachment(binding->attachment, DMA_FROM_DEVICE); > + if (IS_ERR(binding->sgt)) { > + err = PTR_ERR(binding->sgt); > + goto err_detach; > + } > + > + /* For simplicity we expect to make PAGE_SIZE allocations, but the > + * binding can be much more flexible than that. We may be able to > + * allocate MTU sized chunks here. Leave that for future work... > + */ > + binding->chunk_pool = > + gen_pool_create(PAGE_SHIFT, dev_to_node(&dev->dev)); > + if (!binding->chunk_pool) { > + err = -ENOMEM; > + goto err_unmap; > + } > + > + virtual = 0; > + for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) { > + dma_addr_t dma_addr = sg_dma_address(sg); > + struct dmabuf_genpool_chunk_owner *owner; > + size_t len = sg_dma_len(sg); > + struct net_iov *niov; > + > + owner = kzalloc_node(sizeof(*owner), GFP_KERNEL, > + dev_to_node(&dev->dev)); I'm sorry for not catching this earlier, but it looks like the above allocation lacks a NULL check. Thanks, Paolo
On Tue, 2024-07-02 at 13:08 +0200, Paolo Abeni wrote: > On Fri, 2024-06-28 at 00:32 +0000, Mina Almasry wrote: > > +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > > + struct net_devmem_dmabuf_binding **out) > > +{ > > + struct net_devmem_dmabuf_binding *binding; > > + static u32 id_alloc_next; > > + struct scatterlist *sg; > > + struct dma_buf *dmabuf; > > + unsigned int sg_idx, i; > > + unsigned long virtual; > > + int err; > > + > > + dmabuf = dma_buf_get(dmabuf_fd); > > + if (IS_ERR(dmabuf)) > > + return -EBADFD; > > + > > + binding = kzalloc_node(sizeof(*binding), GFP_KERNEL, > > + dev_to_node(&dev->dev)); > > + if (!binding) { > > + err = -ENOMEM; > > + goto err_put_dmabuf; > > + } > > + > > + binding->dev = dev; > > + > > + err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id, > > + binding, xa_limit_32b, &id_alloc_next, > > + GFP_KERNEL); > > + if (err < 0) > > + goto err_free_binding; > > + > > + xa_init_flags(&binding->bound_rxq_list, XA_FLAGS_ALLOC); > > + > > + refcount_set(&binding->ref, 1); > > + > > + binding->dmabuf = dmabuf; > > + > > + binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent); > > + if (IS_ERR(binding->attachment)) { > > + err = PTR_ERR(binding->attachment); > > + goto err_free_id; > > + } > > + > > + binding->sgt = > > + dma_buf_map_attachment(binding->attachment, DMA_FROM_DEVICE); > > + if (IS_ERR(binding->sgt)) { > > + err = PTR_ERR(binding->sgt); > > + goto err_detach; > > + } > > + > > + /* For simplicity we expect to make PAGE_SIZE allocations, but the > > + * binding can be much more flexible than that. We may be able to > > + * allocate MTU sized chunks here. Leave that for future work... > > + */ > > + binding->chunk_pool = > > + gen_pool_create(PAGE_SHIFT, dev_to_node(&dev->dev)); > > + if (!binding->chunk_pool) { > > + err = -ENOMEM; > > + goto err_unmap; > > + } > > + > > + virtual = 0; > > + for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) { > > + dma_addr_t dma_addr = sg_dma_address(sg); > > + struct dmabuf_genpool_chunk_owner *owner; > > + size_t len = sg_dma_len(sg); > > + struct net_iov *niov; > > + > > + owner = kzalloc_node(sizeof(*owner), GFP_KERNEL, > > + dev_to_node(&dev->dev)); > > I'm sorry for not catching this earlier, but it looks like the above > allocation lacks a NULL check. FTR, given the size of the series, the number of iterations already done and the issue not preventing the functionality, I agree to merge the series as-is, and handle this with a follow-up. Thanks, Paolo
On Fri, 28 Jun 2024 00:32:40 +0000 Mina Almasry wrote: > +/* Protected by rtnl_lock() */ > +static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1); > + > +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; nit: I don't see how it can happen, no defensive programming, please > + if (binding->list.next) > + list_del(&binding->list); > + > + xa_for_each(&binding->bound_rxq_list, xa_idx, rxq) { nit: s/bound_rxq_list/bound_rxqs/ ? it's not a list > + 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_priv, NULL); Is this really sufficient in terms of locking? @binding is not RCU-protected and neither is the reader guaranteed to be in an RCU critical section. Actually the "reader" tries to take a ref and use this struct so it's not even a pure reader. Let's add a lock or use one of the existing locks Or, perhaps time to add a mutex to struct net_device > + rxq_idx = get_netdev_rx_queue_index(rxq); > + > + 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->num_rx_queues) > + return -ERANGE; > + > + rxq = __netif_get_rx_queue(dev, rxq_idx); > + if (rxq->mp_params.mp_priv) > + return -EEXIST; Makes me wonder - do we need an API to unbind or we assume application will only have one binding per socket and close it every time? I guess that's fine for future extension. > + err = xa_alloc(&binding->bound_rxq_list, &xa_idx, rxq, xa_limit_32b, > + GFP_KERNEL); > + if (err) > + return err; > + > + /* 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 driver may read this config while it's creating its * rx-queues. > + * WRITE_ONCE() here to match the READ_ONCE() in the driver. > + */ > + WRITE_ONCE(rxq->mp_params.mp_priv, binding); > + > + err = netdev_rx_queue_restart(dev, rxq_idx); > + if (err) > + goto err_xa_erase; > + > + return 0; > + > +err_xa_erase: > + WRITE_ONCE(rxq->mp_params.mp_priv, NULL); > + xa_erase(&binding->bound_rxq_list, xa_idx); > + > + return err; > +} > + > +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out) > +{ > + struct net_devmem_dmabuf_binding *binding; > + static u32 id_alloc_next; > + struct scatterlist *sg; > + struct dma_buf *dmabuf; > + unsigned int sg_idx, i; > + unsigned long virtual; > + int err; > + > + dmabuf = dma_buf_get(dmabuf_fd); > + if (IS_ERR(dmabuf)) > + return -EBADFD; nit: I think error pointers are nicer than **out parameters :( you can ERR_CAST() all the DMABUF errors > + binding = kzalloc_node(sizeof(*binding), GFP_KERNEL, > + dev_to_node(&dev->dev)); > + if (!binding) { > + err = -ENOMEM; > + goto err_put_dmabuf; > + } > + > + binding->dev = dev; > + > + err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id, > + binding, xa_limit_32b, &id_alloc_next, > + GFP_KERNEL); > + if (err < 0) > + goto err_free_binding; > + > + xa_init_flags(&binding->bound_rxq_list, XA_FLAGS_ALLOC); > + > + refcount_set(&binding->ref, 1); > + > + binding->dmabuf = dmabuf; > + > + binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent); > + if (IS_ERR(binding->attachment)) { > + err = PTR_ERR(binding->attachment); > + goto err_free_id; > + } > -/* Stub */ > int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info) > { > - return 0; > + struct nlattr *tb[ARRAY_SIZE(netdev_queue_dmabuf_nl_policy)]; > + struct net_devmem_dmabuf_binding *out_binding; > + struct list_head *sock_binding_list; > + u32 ifindex, dmabuf_fd, rxq_idx; > + struct net_device *netdev; > + struct sk_buff *rsp; > + struct nlattr *attr; > + int rem, err = 0; > + void *hdr; > + > + if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_DEV_IFINDEX) || > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_DMABUF_FD) || > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_QUEUES)) > + return -EINVAL; > + > + ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]); > + dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_BIND_DMABUF_DMABUF_FD]); > + > + rtnl_lock(); > + > + netdev = __dev_get_by_index(genl_info_net(info), ifindex); > + if (!netdev) { || !netif_device_present(netdev) > + err = -ENODEV; > + goto err_unlock; > + } > + > + err = net_devmem_bind_dmabuf(netdev, dmabuf_fd, &out_binding); > + if (err) > + goto err_unlock; > + > + nla_for_each_attr(attr, genlmsg_data(info->genlhdr), > + genlmsg_len(info->genlhdr), rem) { > + > + if (nla_type(attr) != NETDEV_A_BIND_DMABUF_QUEUES) > + continue; nit: nla_for_each_attr_type() > + err = nla_parse_nested( > + tb, ARRAY_SIZE(netdev_queue_dmabuf_nl_policy) - 1, attr, > + netdev_queue_dmabuf_nl_policy, info->extack); > + if (err < 0) > + goto err_unbind; > + > + rxq_idx = nla_get_u32(tb[NETDEV_A_QUEUE_DMABUF_IDX]); > + > + err = net_devmem_bind_dmabuf_to_queue(netdev, rxq_idx, > + out_binding); > + if (err) > + goto err_unbind; > + } > + > + sock_binding_list = genl_sk_priv_get(&netdev_nl_family, > + NETLINK_CB(skb).sk); > + if (IS_ERR(sock_binding_list)) { > + err = PTR_ERR(sock_binding_list); > + goto err_unbind; > + } > + > + list_add(&out_binding->list, sock_binding_list); > + > + rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL); > + if (!rsp) { > + err = -ENOMEM; > + goto err_unbind; > + } > + > + hdr = genlmsg_iput(rsp, info); > + if (!hdr) { > + err = -EMSGSIZE; > + goto err_genlmsg_free; > + } I'd move genl_sk_priv_get(), genlmsg_new() and genlmsg_iput() before we take rtnl_lock(), but I admit it's a bit late for this sort of feedback.. :) > + nla_put_u32(rsp, NETDEV_A_BIND_DMABUF_DMABUF_ID, out_binding->id); > + genlmsg_end(rsp, hdr); > + > + rtnl_unlock(); > + > + return genlmsg_reply(rsp, info); > + > +err_genlmsg_free: > + nlmsg_free(rsp); > +err_unbind: > + net_devmem_unbind_dmabuf(out_binding); > +err_unlock: > + rtnl_unlock(); > + return err; > }
On Tue, Jul 2, 2024 at 6:09 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Fri, 28 Jun 2024 00:32:40 +0000 Mina Almasry wrote: > > + if (binding->list.next) > > + list_del(&binding->list); > > + > > + xa_for_each(&binding->bound_rxq_list, xa_idx, rxq) { > > nit: s/bound_rxq_list/bound_rxqs/ ? it's not a list > > > + 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_priv, NULL); > > Is this really sufficient in terms of locking? @binding is not > RCU-protected and neither is the reader guaranteed to be in > an RCU critical section. Actually the "reader" tries to take a ref > and use this struct so it's not even a pure reader. > > Let's add a lock or use one of the existing locks > Can we just use rtnl_lock() for this synchronization? It seems it's already locked everywhere that access mp_params.mp_priv, so the WRITE/READ_ONCE are actually superfluous. Both the dmabuf bind/unbind already lock rtnl_lock, and the only other place that access mp_params.mp_priv is page_pool_init(). I think it's reasonable to assume rtnl_lock is also held during page_pool_init, no? AFAICT it would be very weird for some code path to be reconfiguring the driver page_pools without holding rtnl_lock? What I wanna do here is delete the incorrect comment, remove the READ/WRITE_ONCE, and maybe add a DEBUG_NET_WARN_ON(!rtnl_is_locked()) in mp_dmabuf_devmem_init() but probably that is too defensive. Will apply the other comments, thanks.
On Wed, 3 Jul 2024 09:56:45 -0700 Mina Almasry wrote: > > Is this really sufficient in terms of locking? @binding is not > > RCU-protected and neither is the reader guaranteed to be in > > an RCU critical section. Actually the "reader" tries to take a ref > > and use this struct so it's not even a pure reader. > > > > Let's add a lock or use one of the existing locks > > Can we just use rtnl_lock() for this synchronization? It seems it's > already locked everywhere that access mp_params.mp_priv, so the > WRITE/READ_ONCE are actually superfluous. Both the dmabuf bind/unbind > already lock rtnl_lock, and the only other place that access > mp_params.mp_priv is page_pool_init(). I think it's reasonable to > assume rtnl_lock is also held during page_pool_init, no? AFAICT it > would be very weird for some code path to be reconfiguring the driver > page_pools without holding rtnl_lock? > > What I wanna do here is delete the incorrect comment, remove the > READ/WRITE_ONCE, and maybe add a DEBUG_NET_WARN_ON(!rtnl_is_locked()) > in mp_dmabuf_devmem_init() but probably that is too defensive. The only concern I have is driver error recovery paths. They may be async and may happen outside of rtnl_lock. Same situation we have with the queue <> NAPI <> IRQ mapping helpers. queue <> NAPI <> IRQ helpers require rtnl_lock today, and Intel recently had a number of fixes because that complicates their error recovery paths. But I guess any locking here will take non-trivial amount of analysis. Let's go with rtnl_lock, that's fine.
On Fri, Jun 28, 2024 at 9:43 AM Mina Almasry <almasrymina@google.com> wrote: > Hi Mina, Thanks a lot for this work! > Add a netdev_dmabuf_binding struct which represents the > dma-buf-to-netdevice binding. The netlink API will bind the dma-buf to > rx queues on the netdevice. On the binding, the dma_buf_attach > & dma_buf_map_attachment will occur. The entries in the sg_table from > mapping will be inserted into a genpool to make it ready > for allocation. > > The chunks in the genpool are owned by a dmabuf_chunk_owner struct which > holds the dma-buf offset of the base of the chunk and the dma_addr of > the chunk. Both are needed to use allocations that come from this chunk. > > We create a new type that represents an allocation from the genpool: > net_iov. We setup the net_iov allocation size in the > genpool to PAGE_SIZE for simplicity: to match the PAGE_SIZE normally > allocated by the page pool and given to the drivers. > > The user can unbind the dmabuf from the netdevice by closing the netlink > socket that established the binding. We do this so that the binding is > automatically unbound even if the userspace process crashes. > > The binding and unbinding leaves an indicator in struct netdev_rx_queue > that the given queue is bound, but the binding doesn't take effect until > the driver actually reconfigures its queues, and re-initializes its page > pool. > > The netdev_dmabuf_binding struct is refcounted, and releases its > resources only when all the refs are released. > > Signed-off-by: Willem de Bruijn <willemb@google.com> > Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com> > Signed-off-by: Mina Almasry <almasrymina@google.com> > Reviewed-by: Pavel Begunkov <asml.silence@gmail.com> # excluding netlink > > --- > > v13: > - Fixed a couple of places that still listed DMA_BIDIRECTIONAL (Pavel). > - Added reviewed-by from Pavel. > > v11: > - Fix build error with CONFIG_DMA_SHARED_BUFFER && > !CONFIG_GENERIC_ALLOCATOR > - Rebased on top of no memory provider ops. > > v10: > - Moved net_iov_dma_addr() to devmem.h and made it devmem specific > helper (David). > > v9: https://lore.kernel.org/all/20240403002053.2376017-5-almasrymina@google.com/ > - Removed net_devmem_restart_rx_queues and put it in its own patch > (David). > > v8: > - move dmabuf_devmem_ops usage to later patch to avoid patch-by-patch > build error. > > v7: > - Use IS_ERR() instead of IS_ERR_OR_NULL() for the dma_buf_get() return > value. > - Changes netdev_* naming in devmem.c to net_devmem_* (Yunsheng). > - DMA_BIDIRECTIONAL -> DMA_FROM_DEVICE (Yunsheng). > - Added a comment around recovering of the old rx queue in > net_devmem_restart_rx_queue(), and added freeing of old_mem if the > restart of the old queue fails. (Yunsheng). > - Use kernel-family sock-priv (Jakub). > - Put pp_memory_provider_params in netdev_rx_queue instead of the > dma-buf specific binding (Pavel & David). > - Move queue management ops to queue_mgmt_ops instead of netdev_ops > (Jakub). > - Remove excess whitespaces (Jakub). > - Use genlmsg_iput (Jakub). > > v6: > - Validate rx queue index > - Refactor new functions into devmem.c (Pavel) > > v5: > - Renamed page_pool_iov to net_iov, and moved that support to devmem.h > or netmem.h. > > v1: > - Introduce devmem.h instead of bloating netdevice.h (Jakub) > - ENOTSUPP -> EOPNOTSUPP (checkpatch.pl I think) > - Remove unneeded rcu protection for binding->list (rtnl protected) > - Removed extraneous err_binding_put: label. > - Removed dma_addr += len (Paolo). > - Don't override err on netdev_bind_dmabuf_to_queue failure. > - Rename devmem -> dmabuf (David). > - Add id to dmabuf binding (David/Stan). > - Fix missing xa_destroy bound_rq_list. > - Use queue api to reset bound RX queues (Jakub). > - Update netlink API for rx-queue type (tx/re) (Jakub). > > RFC v3: > - Support multi rx-queue binding > > --- > Documentation/netlink/specs/netdev.yaml | 4 + > include/net/devmem.h | 111 +++++++++++ > include/net/netdev_rx_queue.h | 2 + > include/net/netmem.h | 10 + > include/net/page_pool/types.h | 6 + > net/core/Makefile | 2 +- > net/core/dev.c | 3 + > net/core/devmem.c | 252 ++++++++++++++++++++++++ > net/core/netdev-genl-gen.c | 4 + > net/core/netdev-genl-gen.h | 4 + > net/core/netdev-genl.c | 101 +++++++++- > 11 files changed, 496 insertions(+), 3 deletions(-) > create mode 100644 include/net/devmem.h > create mode 100644 net/core/devmem.c > > diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml > index 899ac0882a098..d6d7cb01c145c 100644 > --- a/Documentation/netlink/specs/netdev.yaml > +++ b/Documentation/netlink/specs/netdev.yaml > @@ -673,6 +673,10 @@ operations: > - tx-packets > - tx-bytes > > +kernel-family: > + headers: [ "linux/list.h"] > + sock-priv: struct list_head > + > mcast-groups: > list: > - > diff --git a/include/net/devmem.h b/include/net/devmem.h > new file mode 100644 > index 0000000000000..eaf3fd965d7a8 > --- /dev/null > +++ b/include/net/devmem.h > @@ -0,0 +1,111 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Device memory TCP support > + * > + * Authors: Mina Almasry <almasrymina@google.com> > + * Willem de Bruijn <willemb@google.com> > + * Kaiyuan Zhang <kaiyuanz@google.com> > + * > + */ > +#ifndef _NET_DEVMEM_H > +#define _NET_DEVMEM_H > + > +struct net_devmem_dmabuf_binding { > + struct dma_buf *dmabuf; > + struct dma_buf_attachment *attachment; > + struct sg_table *sgt; > + struct net_device *dev; > + struct gen_pool *chunk_pool; > + > + /* The user holds a ref (via the netlink API) for as long as they want > + * the binding to remain alive. Each page pool using this binding holds > + * a ref to keep the binding alive. Each allocated net_iov holds a > + * ref. > + * > + * The binding undos itself and unmaps the underlying dmabuf once all > + * those refs are dropped and the binding is no longer desired or in > + * use. > + */ > + refcount_t ref; > + > + /* The list of bindings currently active. Used for netlink to notify us > + * of the user dropping the bind. > + */ > + struct list_head list; > + > + /* rxq's this binding is active on. */ > + struct xarray bound_rxq_list; > + > + /* ID of this binding. Globally unique to all bindings currently > + * active. > + */ > + u32 id; > +}; > + > +/* Owner of the dma-buf chunks inserted into the gen pool. Each scatterlist > + * entry from the dmabuf is inserted into the genpool as a chunk, and needs > + * this owner struct to keep track of some metadata necessary to create > + * allocations from this chunk. > + */ > +struct dmabuf_genpool_chunk_owner { > + /* Offset into the dma-buf where this chunk starts. */ > + unsigned long base_virtual; > + > + /* dma_addr of the start of the chunk. */ > + dma_addr_t base_dma_addr; > + > + /* Array of net_iovs for this chunk. */ > + struct net_iov *niovs; > + size_t num_niovs; > + > + struct net_devmem_dmabuf_binding *binding; > +}; > + > +#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR) > +void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding); > +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out); > +void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding); > +int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, > + struct net_devmem_dmabuf_binding *binding); > +#else > +static inline void > +__net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding) > +{ > +} > + > +static inline int net_devmem_bind_dmabuf(struct net_device *dev, > + unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out) > +{ > + return -EOPNOTSUPP; > +} > +static inline void > +net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding) > +{ > +} > + > +static inline int > +net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, > + struct net_devmem_dmabuf_binding *binding) > +{ > + return -EOPNOTSUPP; > +} > +#endif > + > +static inline void > +net_devmem_dmabuf_binding_get(struct net_devmem_dmabuf_binding *binding) > +{ > + refcount_inc(&binding->ref); > +} > + > +static inline void > +net_devmem_dmabuf_binding_put(struct net_devmem_dmabuf_binding *binding) > +{ > + if (!refcount_dec_and_test(&binding->ref)) > + return; > + > + __net_devmem_dmabuf_binding_free(binding); > +} > + > +#endif /* _NET_DEVMEM_H */ > diff --git a/include/net/netdev_rx_queue.h b/include/net/netdev_rx_queue.h > index e78ca52d67fbf..ac34f5fb4f71d 100644 > --- a/include/net/netdev_rx_queue.h > +++ b/include/net/netdev_rx_queue.h > @@ -6,6 +6,7 @@ > #include <linux/netdevice.h> > #include <linux/sysfs.h> > #include <net/xdp.h> > +#include <net/page_pool/types.h> > > /* This structure contains an instance of an RX queue. */ > struct netdev_rx_queue { > @@ -25,6 +26,7 @@ struct netdev_rx_queue { > * Readers and writers must hold RTNL > */ > struct napi_struct *napi; > + struct pp_memory_provider_params mp_params; > } ____cacheline_aligned_in_smp; > > /* > diff --git a/include/net/netmem.h b/include/net/netmem.h > index d8b810245c1da..72e932a1a9489 100644 > --- a/include/net/netmem.h > +++ b/include/net/netmem.h > @@ -8,6 +8,16 @@ > #ifndef _NET_NETMEM_H > #define _NET_NETMEM_H > > +#include <net/devmem.h> > + > +/* net_iov */ > + > +struct net_iov { > + struct dmabuf_genpool_chunk_owner *owner; > +}; > + > +/* netmem */ > + > /** > * typedef netmem_ref - a nonexistent type marking a reference to generic > * network memory. > diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h > index 7e8477057f3d1..9f3c3ee2ee755 100644 > --- a/include/net/page_pool/types.h > +++ b/include/net/page_pool/types.h > @@ -128,6 +128,10 @@ struct page_pool_stats { > }; > #endif > > +struct pp_memory_provider_params { > + void *mp_priv; > +}; > + > struct page_pool { > struct page_pool_params_fast p; > > @@ -194,6 +198,8 @@ struct page_pool { > */ > struct ptr_ring ring; > > + void *mp_priv; > + > #ifdef CONFIG_PAGE_POOL_STATS > /* recycle stats are per-cpu to avoid locking */ > struct page_pool_recycle_stats __percpu *recycle_stats; > diff --git a/net/core/Makefile b/net/core/Makefile > index f82232b358a2c..6b43611fb4a43 100644 > --- a/net/core/Makefile > +++ b/net/core/Makefile > @@ -13,7 +13,7 @@ obj-y += dev.o dev_addr_lists.o dst.o netevent.o \ > neighbour.o rtnetlink.o utils.o link_watch.o filter.o \ > sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \ > fib_notifier.o xdp.o flow_offload.o gro.o \ > - netdev-genl.o netdev-genl-gen.o gso.o > + netdev-genl.o netdev-genl-gen.o gso.o devmem.o > > obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o > > diff --git a/net/core/dev.c b/net/core/dev.c > index 0a23d7da7fbc6..5e5e2d266b83f 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -158,6 +158,9 @@ > #include <net/page_pool/types.h> > #include <net/page_pool/helpers.h> > #include <net/rps.h> > +#include <linux/genalloc.h> > +#include <linux/dma-buf.h> > +#include <net/devmem.h> > > #include "dev.h" > #include "net-sysfs.h" > diff --git a/net/core/devmem.c b/net/core/devmem.c > new file mode 100644 > index 0000000000000..cfb5a2f69dcd2 > --- /dev/null > +++ b/net/core/devmem.c > @@ -0,0 +1,252 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Devmem TCP > + * > + * Authors: Mina Almasry <almasrymina@google.com> > + * Willem de Bruijn <willemdebruijn.kernel@gmail.com> > + * Kaiyuan Zhang <kaiyuanz@google.com > + */ > + > +#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> > + > +/* Device memory support */ > + > +#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR) > +static void net_devmem_dmabuf_free_chunk_owner(struct gen_pool *genpool, > + struct gen_pool_chunk *chunk, > + void *not_used) > +{ > + struct dmabuf_genpool_chunk_owner *owner = chunk->owner; > + > + kvfree(owner->niovs); > + kfree(owner); > +} > + > +void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding) > +{ > + size_t size, avail; > + > + gen_pool_for_each_chunk(binding->chunk_pool, > + net_devmem_dmabuf_free_chunk_owner, NULL); > + > + size = gen_pool_size(binding->chunk_pool); > + avail = gen_pool_avail(binding->chunk_pool); > + > + if (!WARN(size != avail, "can't destroy genpool. size=%zu, avail=%zu", > + size, avail)) > + gen_pool_destroy(binding->chunk_pool); > + > + dma_buf_unmap_attachment(binding->attachment, binding->sgt, > + DMA_FROM_DEVICE); > + dma_buf_detach(binding->dmabuf, binding->attachment); > + dma_buf_put(binding->dmabuf); > + xa_destroy(&binding->bound_rxq_list); > + kfree(binding); > +} > + > +/* Protected by rtnl_lock() */ > +static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1); > + > +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_priv, NULL); > + > + rxq_idx = get_netdev_rx_queue_index(rxq); > + > + 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->num_rx_queues) > + return -ERANGE; > + I think it should be dev->real_num_rx_queues, not dev->num_rx_queues. And I think we need to check whether an interface is up somewhere here. > + rxq = __netif_get_rx_queue(dev, rxq_idx); > + if (rxq->mp_params.mp_priv) > + return -EEXIST; > + > + err = xa_alloc(&binding->bound_rxq_list, &xa_idx, rxq, xa_limit_32b, > + GFP_KERNEL); > + if (err) > + return err; > + > + /* 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 driver may read this config while it's creating its * rx-queues. > + * WRITE_ONCE() here to match the READ_ONCE() in the driver. > + */ > + WRITE_ONCE(rxq->mp_params.mp_priv, binding); > + > + err = netdev_rx_queue_restart(dev, rxq_idx); > + if (err) > + goto err_xa_erase; > + > + return 0; > + > +err_xa_erase: > + WRITE_ONCE(rxq->mp_params.mp_priv, NULL); > + xa_erase(&binding->bound_rxq_list, xa_idx); > + > + return err; > +} > + > +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out) > +{ > + struct net_devmem_dmabuf_binding *binding; > + static u32 id_alloc_next; > + struct scatterlist *sg; > + struct dma_buf *dmabuf; > + unsigned int sg_idx, i; > + unsigned long virtual; > + int err; > + > + dmabuf = dma_buf_get(dmabuf_fd); > + if (IS_ERR(dmabuf)) > + return -EBADFD; > + > + binding = kzalloc_node(sizeof(*binding), GFP_KERNEL, > + dev_to_node(&dev->dev)); > + if (!binding) { > + err = -ENOMEM; > + goto err_put_dmabuf; > + } > + > + binding->dev = dev; > + > + err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id, > + binding, xa_limit_32b, &id_alloc_next, > + GFP_KERNEL); > + if (err < 0) > + goto err_free_binding; > + > + xa_init_flags(&binding->bound_rxq_list, XA_FLAGS_ALLOC); > + > + refcount_set(&binding->ref, 1); > + > + binding->dmabuf = dmabuf; > + > + binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent); > + if (IS_ERR(binding->attachment)) { > + err = PTR_ERR(binding->attachment); > + goto err_free_id; > + } > + > + binding->sgt = > + dma_buf_map_attachment(binding->attachment, DMA_FROM_DEVICE); > + if (IS_ERR(binding->sgt)) { > + err = PTR_ERR(binding->sgt); > + goto err_detach; > + } > + > + /* For simplicity we expect to make PAGE_SIZE allocations, but the > + * binding can be much more flexible than that. We may be able to > + * allocate MTU sized chunks here. Leave that for future work... > + */ > + binding->chunk_pool = > + gen_pool_create(PAGE_SHIFT, dev_to_node(&dev->dev)); > + if (!binding->chunk_pool) { > + err = -ENOMEM; > + goto err_unmap; > + } > + > + virtual = 0; > + for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) { > + dma_addr_t dma_addr = sg_dma_address(sg); > + struct dmabuf_genpool_chunk_owner *owner; > + size_t len = sg_dma_len(sg); > + struct net_iov *niov; > + > + owner = kzalloc_node(sizeof(*owner), GFP_KERNEL, > + dev_to_node(&dev->dev)); > + owner->base_virtual = virtual; > + owner->base_dma_addr = dma_addr; > + owner->num_niovs = len / PAGE_SIZE; > + owner->binding = binding; > + > + err = gen_pool_add_owner(binding->chunk_pool, dma_addr, > + dma_addr, len, dev_to_node(&dev->dev), > + owner); > + if (err) { > + err = -EINVAL; > + goto err_free_chunks; > + } > + > + owner->niovs = kvmalloc_array(owner->num_niovs, > + sizeof(*owner->niovs), > + GFP_KERNEL); > + if (!owner->niovs) { > + err = -ENOMEM; > + goto err_free_chunks; > + } > + > + for (i = 0; i < owner->num_niovs; i++) { > + niov = &owner->niovs[i]; > + niov->owner = owner; > + } > + > + virtual += len; > + } > + > + *out = binding; > + > + return 0; > + > +err_free_chunks: > + gen_pool_for_each_chunk(binding->chunk_pool, > + net_devmem_dmabuf_free_chunk_owner, NULL); > + gen_pool_destroy(binding->chunk_pool); > +err_unmap: > + dma_buf_unmap_attachment(binding->attachment, binding->sgt, > + DMA_FROM_DEVICE); > +err_detach: > + dma_buf_detach(dmabuf, binding->attachment); > +err_free_id: > + xa_erase(&net_devmem_dmabuf_bindings, binding->id); > +err_free_binding: > + kfree(binding); > +err_put_dmabuf: > + dma_buf_put(dmabuf); > + return err; > +} > +#endif > diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c > index 9acd0d893765a..3dcd25049e593 100644 > --- a/net/core/netdev-genl-gen.c > +++ b/net/core/netdev-genl-gen.c > @@ -9,6 +9,7 @@ > #include "netdev-genl-gen.h" > > #include <uapi/linux/netdev.h> > +#include <linux/list.h> > > /* Integer value ranges */ > static const struct netlink_range_validation netdev_a_page_pool_id_range = { > @@ -187,4 +188,7 @@ struct genl_family netdev_nl_family __ro_after_init = { > .n_split_ops = ARRAY_SIZE(netdev_nl_ops), > .mcgrps = netdev_nl_mcgrps, > .n_mcgrps = ARRAY_SIZE(netdev_nl_mcgrps), > + .sock_priv_size = sizeof(struct list_head), > + .sock_priv_init = (void *)netdev_nl_sock_priv_init, > + .sock_priv_destroy = (void *)netdev_nl_sock_priv_destroy, > }; > diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h > index ca5a0983f2834..2c431b7dcbc84 100644 > --- a/net/core/netdev-genl-gen.h > +++ b/net/core/netdev-genl-gen.h > @@ -10,6 +10,7 @@ > #include <net/genetlink.h> > > #include <uapi/linux/netdev.h> > +#include <linux/list.h> > > /* Common nested types */ > extern const struct nla_policy netdev_page_pool_info_nl_policy[NETDEV_A_PAGE_POOL_IFINDEX + 1]; > @@ -40,4 +41,7 @@ enum { > > extern struct genl_family netdev_nl_family; > > +void netdev_nl_sock_priv_init(struct list_head *priv); > +void netdev_nl_sock_priv_destroy(struct list_head *priv); > + > #endif /* _LINUX_NETDEV_GEN_H */ > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c > index 2d726e65211dd..133884eb13349 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 "netdev-genl-gen.h" > #include "dev.h" > @@ -721,10 +722,92 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb, > return err; > } > > -/* Stub */ > int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info) > { > - return 0; > + struct nlattr *tb[ARRAY_SIZE(netdev_queue_dmabuf_nl_policy)]; > + struct net_devmem_dmabuf_binding *out_binding; > + struct list_head *sock_binding_list; > + u32 ifindex, dmabuf_fd, rxq_idx; > + struct net_device *netdev; > + struct sk_buff *rsp; > + struct nlattr *attr; > + int rem, err = 0; > + void *hdr; > + > + if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_DEV_IFINDEX) || > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_DMABUF_FD) || > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_QUEUES)) > + return -EINVAL; > + > + ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]); > + dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_BIND_DMABUF_DMABUF_FD]); > + > + rtnl_lock(); > + > + netdev = __dev_get_by_index(genl_info_net(info), ifindex); > + if (!netdev) { > + err = -ENODEV; > + goto err_unlock; > + } > + > + err = net_devmem_bind_dmabuf(netdev, dmabuf_fd, &out_binding); > + if (err) > + goto err_unlock; > + > + nla_for_each_attr(attr, genlmsg_data(info->genlhdr), > + genlmsg_len(info->genlhdr), rem) { > + if (nla_type(attr) != NETDEV_A_BIND_DMABUF_QUEUES) > + continue; > + > + err = nla_parse_nested( > + tb, ARRAY_SIZE(netdev_queue_dmabuf_nl_policy) - 1, attr, > + netdev_queue_dmabuf_nl_policy, info->extack); > + if (err < 0) > + goto err_unbind; > + > + rxq_idx = nla_get_u32(tb[NETDEV_A_QUEUE_DMABUF_IDX]); > + > + err = net_devmem_bind_dmabuf_to_queue(netdev, rxq_idx, > + out_binding); > + if (err) > + goto err_unbind; > + } > + > + sock_binding_list = genl_sk_priv_get(&netdev_nl_family, > + NETLINK_CB(skb).sk); > + if (IS_ERR(sock_binding_list)) { > + err = PTR_ERR(sock_binding_list); > + goto err_unbind; > + } > + > + list_add(&out_binding->list, sock_binding_list); > + > + rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL); > + if (!rsp) { > + err = -ENOMEM; > + goto err_unbind; > + } > + > + hdr = genlmsg_iput(rsp, info); > + if (!hdr) { > + err = -EMSGSIZE; > + goto err_genlmsg_free; > + } > + > + nla_put_u32(rsp, NETDEV_A_BIND_DMABUF_DMABUF_ID, out_binding->id); > + genlmsg_end(rsp, hdr); > + > + rtnl_unlock(); > + > + return genlmsg_reply(rsp, info); > + > +err_genlmsg_free: > + nlmsg_free(rsp); > +err_unbind: > + net_devmem_unbind_dmabuf(out_binding); > +err_unlock: > + rtnl_unlock(); > + return err; > } > > static int netdev_genl_netdevice_event(struct notifier_block *nb, > @@ -771,3 +854,17 @@ static int __init netdev_genl_init(void) > } > > subsys_initcall(netdev_genl_init); > + > +void netdev_nl_sock_priv_init(struct list_head *priv) > +{ > + INIT_LIST_HEAD(priv); > +} > + > +void netdev_nl_sock_priv_destroy(struct list_head *priv) > +{ > + struct net_devmem_dmabuf_binding *binding; > + struct net_devmem_dmabuf_binding *temp; > + > + list_for_each_entry_safe(binding, temp, priv, list) > + net_devmem_unbind_dmabuf(binding); > +} > -- > 2.45.2.803.g4e1b14247a-goog > > Thanks a lot! Taehee Yoo
On Fri, Jun 28, 2024 at 9:43 AM Mina Almasry <almasrymina@google.com> wrote: > Hi Mina, > Add a netdev_dmabuf_binding struct which represents the > dma-buf-to-netdevice binding. The netlink API will bind the dma-buf to > rx queues on the netdevice. On the binding, the dma_buf_attach > & dma_buf_map_attachment will occur. The entries in the sg_table from > mapping will be inserted into a genpool to make it ready > for allocation. > > The chunks in the genpool are owned by a dmabuf_chunk_owner struct which > holds the dma-buf offset of the base of the chunk and the dma_addr of > the chunk. Both are needed to use allocations that come from this chunk. > > We create a new type that represents an allocation from the genpool: > net_iov. We setup the net_iov allocation size in the > genpool to PAGE_SIZE for simplicity: to match the PAGE_SIZE normally > allocated by the page pool and given to the drivers. > > The user can unbind the dmabuf from the netdevice by closing the netlink > socket that established the binding. We do this so that the binding is > automatically unbound even if the userspace process crashes. > > The binding and unbinding leaves an indicator in struct netdev_rx_queue > that the given queue is bound, but the binding doesn't take effect until > the driver actually reconfigures its queues, and re-initializes its page > pool. > > The netdev_dmabuf_binding struct is refcounted, and releases its > resources only when all the refs are released. > > Signed-off-by: Willem de Bruijn <willemb@google.com> > Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com> > Signed-off-by: Mina Almasry <almasrymina@google.com> > Reviewed-by: Pavel Begunkov <asml.silence@gmail.com> # excluding netlink > > --- > > v13: > - Fixed a couple of places that still listed DMA_BIDIRECTIONAL (Pavel). > - Added reviewed-by from Pavel. > > v11: > - Fix build error with CONFIG_DMA_SHARED_BUFFER && > !CONFIG_GENERIC_ALLOCATOR > - Rebased on top of no memory provider ops. > > v10: > - Moved net_iov_dma_addr() to devmem.h and made it devmem specific > helper (David). > > v9: https://lore.kernel.org/all/20240403002053.2376017-5-almasrymina@google.com/ > - Removed net_devmem_restart_rx_queues and put it in its own patch > (David). > > v8: > - move dmabuf_devmem_ops usage to later patch to avoid patch-by-patch > build error. > > v7: > - Use IS_ERR() instead of IS_ERR_OR_NULL() for the dma_buf_get() return > value. > - Changes netdev_* naming in devmem.c to net_devmem_* (Yunsheng). > - DMA_BIDIRECTIONAL -> DMA_FROM_DEVICE (Yunsheng). > - Added a comment around recovering of the old rx queue in > net_devmem_restart_rx_queue(), and added freeing of old_mem if the > restart of the old queue fails. (Yunsheng). > - Use kernel-family sock-priv (Jakub). > - Put pp_memory_provider_params in netdev_rx_queue instead of the > dma-buf specific binding (Pavel & David). > - Move queue management ops to queue_mgmt_ops instead of netdev_ops > (Jakub). > - Remove excess whitespaces (Jakub). > - Use genlmsg_iput (Jakub). > > v6: > - Validate rx queue index > - Refactor new functions into devmem.c (Pavel) > > v5: > - Renamed page_pool_iov to net_iov, and moved that support to devmem.h > or netmem.h. > > v1: > - Introduce devmem.h instead of bloating netdevice.h (Jakub) > - ENOTSUPP -> EOPNOTSUPP (checkpatch.pl I think) > - Remove unneeded rcu protection for binding->list (rtnl protected) > - Removed extraneous err_binding_put: label. > - Removed dma_addr += len (Paolo). > - Don't override err on netdev_bind_dmabuf_to_queue failure. > - Rename devmem -> dmabuf (David). > - Add id to dmabuf binding (David/Stan). > - Fix missing xa_destroy bound_rq_list. > - Use queue api to reset bound RX queues (Jakub). > - Update netlink API for rx-queue type (tx/re) (Jakub). > > RFC v3: > - Support multi rx-queue binding > > --- > Documentation/netlink/specs/netdev.yaml | 4 + > include/net/devmem.h | 111 +++++++++++ > include/net/netdev_rx_queue.h | 2 + > include/net/netmem.h | 10 + > include/net/page_pool/types.h | 6 + > net/core/Makefile | 2 +- > net/core/dev.c | 3 + > net/core/devmem.c | 252 ++++++++++++++++++++++++ > net/core/netdev-genl-gen.c | 4 + > net/core/netdev-genl-gen.h | 4 + > net/core/netdev-genl.c | 101 +++++++++- > 11 files changed, 496 insertions(+), 3 deletions(-) > create mode 100644 include/net/devmem.h > create mode 100644 net/core/devmem.c > > diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml > index 899ac0882a098..d6d7cb01c145c 100644 > --- a/Documentation/netlink/specs/netdev.yaml > +++ b/Documentation/netlink/specs/netdev.yaml > @@ -673,6 +673,10 @@ operations: > - tx-packets > - tx-bytes > > +kernel-family: > + headers: [ "linux/list.h"] > + sock-priv: struct list_head > + > mcast-groups: > list: > - > diff --git a/include/net/devmem.h b/include/net/devmem.h > new file mode 100644 > index 0000000000000..eaf3fd965d7a8 > --- /dev/null > +++ b/include/net/devmem.h > @@ -0,0 +1,111 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Device memory TCP support > + * > + * Authors: Mina Almasry <almasrymina@google.com> > + * Willem de Bruijn <willemb@google.com> > + * Kaiyuan Zhang <kaiyuanz@google.com> > + * > + */ > +#ifndef _NET_DEVMEM_H > +#define _NET_DEVMEM_H > + > +struct net_devmem_dmabuf_binding { > + struct dma_buf *dmabuf; > + struct dma_buf_attachment *attachment; > + struct sg_table *sgt; > + struct net_device *dev; > + struct gen_pool *chunk_pool; > + > + /* The user holds a ref (via the netlink API) for as long as they want > + * the binding to remain alive. Each page pool using this binding holds > + * a ref to keep the binding alive. Each allocated net_iov holds a > + * ref. > + * > + * The binding undos itself and unmaps the underlying dmabuf once all > + * those refs are dropped and the binding is no longer desired or in > + * use. > + */ > + refcount_t ref; > + > + /* The list of bindings currently active. Used for netlink to notify us > + * of the user dropping the bind. > + */ > + struct list_head list; > + > + /* rxq's this binding is active on. */ > + struct xarray bound_rxq_list; > + > + /* ID of this binding. Globally unique to all bindings currently > + * active. > + */ > + u32 id; > +}; > + > +/* Owner of the dma-buf chunks inserted into the gen pool. Each scatterlist > + * entry from the dmabuf is inserted into the genpool as a chunk, and needs > + * this owner struct to keep track of some metadata necessary to create > + * allocations from this chunk. > + */ > +struct dmabuf_genpool_chunk_owner { > + /* Offset into the dma-buf where this chunk starts. */ > + unsigned long base_virtual; > + > + /* dma_addr of the start of the chunk. */ > + dma_addr_t base_dma_addr; > + > + /* Array of net_iovs for this chunk. */ > + struct net_iov *niovs; > + size_t num_niovs; > + > + struct net_devmem_dmabuf_binding *binding; > +}; > + > +#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR) > +void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding); > +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out); > +void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding); > +int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, > + struct net_devmem_dmabuf_binding *binding); > +#else > +static inline void > +__net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding) > +{ > +} > + > +static inline int net_devmem_bind_dmabuf(struct net_device *dev, > + unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out) > +{ > + return -EOPNOTSUPP; > +} > +static inline void > +net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding) > +{ > +} > + > +static inline int > +net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, > + struct net_devmem_dmabuf_binding *binding) > +{ > + return -EOPNOTSUPP; > +} > +#endif > + > +static inline void > +net_devmem_dmabuf_binding_get(struct net_devmem_dmabuf_binding *binding) > +{ > + refcount_inc(&binding->ref); > +} > + > +static inline void > +net_devmem_dmabuf_binding_put(struct net_devmem_dmabuf_binding *binding) > +{ > + if (!refcount_dec_and_test(&binding->ref)) > + return; > + > + __net_devmem_dmabuf_binding_free(binding); > +} > + > +#endif /* _NET_DEVMEM_H */ > diff --git a/include/net/netdev_rx_queue.h b/include/net/netdev_rx_queue.h > index e78ca52d67fbf..ac34f5fb4f71d 100644 > --- a/include/net/netdev_rx_queue.h > +++ b/include/net/netdev_rx_queue.h > @@ -6,6 +6,7 @@ > #include <linux/netdevice.h> > #include <linux/sysfs.h> > #include <net/xdp.h> > +#include <net/page_pool/types.h> > > /* This structure contains an instance of an RX queue. */ > struct netdev_rx_queue { > @@ -25,6 +26,7 @@ struct netdev_rx_queue { > * Readers and writers must hold RTNL > */ > struct napi_struct *napi; > + struct pp_memory_provider_params mp_params; > } ____cacheline_aligned_in_smp; > > /* > diff --git a/include/net/netmem.h b/include/net/netmem.h > index d8b810245c1da..72e932a1a9489 100644 > --- a/include/net/netmem.h > +++ b/include/net/netmem.h > @@ -8,6 +8,16 @@ > #ifndef _NET_NETMEM_H > #define _NET_NETMEM_H > > +#include <net/devmem.h> > + > +/* net_iov */ > + > +struct net_iov { > + struct dmabuf_genpool_chunk_owner *owner; > +}; > + > +/* netmem */ > + > /** > * typedef netmem_ref - a nonexistent type marking a reference to generic > * network memory. > diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h > index 7e8477057f3d1..9f3c3ee2ee755 100644 > --- a/include/net/page_pool/types.h > +++ b/include/net/page_pool/types.h > @@ -128,6 +128,10 @@ struct page_pool_stats { > }; > #endif > > +struct pp_memory_provider_params { > + void *mp_priv; > +}; > + > struct page_pool { > struct page_pool_params_fast p; > > @@ -194,6 +198,8 @@ struct page_pool { > */ > struct ptr_ring ring; > > + void *mp_priv; > + > #ifdef CONFIG_PAGE_POOL_STATS > /* recycle stats are per-cpu to avoid locking */ > struct page_pool_recycle_stats __percpu *recycle_stats; > diff --git a/net/core/Makefile b/net/core/Makefile > index f82232b358a2c..6b43611fb4a43 100644 > --- a/net/core/Makefile > +++ b/net/core/Makefile > @@ -13,7 +13,7 @@ obj-y += dev.o dev_addr_lists.o dst.o netevent.o \ > neighbour.o rtnetlink.o utils.o link_watch.o filter.o \ > sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \ > fib_notifier.o xdp.o flow_offload.o gro.o \ > - netdev-genl.o netdev-genl-gen.o gso.o > + netdev-genl.o netdev-genl-gen.o gso.o devmem.o > > obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o > > diff --git a/net/core/dev.c b/net/core/dev.c > index 0a23d7da7fbc6..5e5e2d266b83f 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -158,6 +158,9 @@ > #include <net/page_pool/types.h> > #include <net/page_pool/helpers.h> > #include <net/rps.h> > +#include <linux/genalloc.h> > +#include <linux/dma-buf.h> > +#include <net/devmem.h> > > #include "dev.h" > #include "net-sysfs.h" > diff --git a/net/core/devmem.c b/net/core/devmem.c > new file mode 100644 > index 0000000000000..cfb5a2f69dcd2 > --- /dev/null > +++ b/net/core/devmem.c > @@ -0,0 +1,252 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Devmem TCP > + * > + * Authors: Mina Almasry <almasrymina@google.com> > + * Willem de Bruijn <willemdebruijn.kernel@gmail.com> > + * Kaiyuan Zhang <kaiyuanz@google.com > + */ > + > +#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> > + > +/* Device memory support */ > + > +#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR) > +static void net_devmem_dmabuf_free_chunk_owner(struct gen_pool *genpool, > + struct gen_pool_chunk *chunk, > + void *not_used) > +{ > + struct dmabuf_genpool_chunk_owner *owner = chunk->owner; > + > + kvfree(owner->niovs); > + kfree(owner); > +} > + > +void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding) > +{ > + size_t size, avail; > + > + gen_pool_for_each_chunk(binding->chunk_pool, > + net_devmem_dmabuf_free_chunk_owner, NULL); > + > + size = gen_pool_size(binding->chunk_pool); > + avail = gen_pool_avail(binding->chunk_pool); > + > + if (!WARN(size != avail, "can't destroy genpool. size=%zu, avail=%zu", > + size, avail)) > + gen_pool_destroy(binding->chunk_pool); > + > + dma_buf_unmap_attachment(binding->attachment, binding->sgt, > + DMA_FROM_DEVICE); > + dma_buf_detach(binding->dmabuf, binding->attachment); > + dma_buf_put(binding->dmabuf); > + xa_destroy(&binding->bound_rxq_list); > + kfree(binding); > +} > + > +/* Protected by rtnl_lock() */ > +static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1); > + > +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_priv, NULL); > + > + rxq_idx = get_netdev_rx_queue_index(rxq); > + > + 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->num_rx_queues) > + return -ERANGE; > + > + rxq = __netif_get_rx_queue(dev, rxq_idx); > + if (rxq->mp_params.mp_priv) > + return -EEXIST; > + > + err = xa_alloc(&binding->bound_rxq_list, &xa_idx, rxq, xa_limit_32b, > + GFP_KERNEL); > + if (err) > + return err; > + > + /* 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 driver may read this config while it's creating its * rx-queues. > + * WRITE_ONCE() here to match the READ_ONCE() in the driver. > + */ > + WRITE_ONCE(rxq->mp_params.mp_priv, binding); > + > + err = netdev_rx_queue_restart(dev, rxq_idx); > + if (err) > + goto err_xa_erase; > + > + return 0; > + > +err_xa_erase: > + WRITE_ONCE(rxq->mp_params.mp_priv, NULL); > + xa_erase(&binding->bound_rxq_list, xa_idx); > + > + return err; > +} > + > +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, > + struct net_devmem_dmabuf_binding **out) > +{ > + struct net_devmem_dmabuf_binding *binding; > + static u32 id_alloc_next; > + struct scatterlist *sg; > + struct dma_buf *dmabuf; > + unsigned int sg_idx, i; > + unsigned long virtual; > + int err; > + > + dmabuf = dma_buf_get(dmabuf_fd); > + if (IS_ERR(dmabuf)) > + return -EBADFD; > + > + binding = kzalloc_node(sizeof(*binding), GFP_KERNEL, > + dev_to_node(&dev->dev)); > + if (!binding) { > + err = -ENOMEM; > + goto err_put_dmabuf; > + } > + > + binding->dev = dev; > + > + err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id, > + binding, xa_limit_32b, &id_alloc_next, > + GFP_KERNEL); > + if (err < 0) > + goto err_free_binding; > + > + xa_init_flags(&binding->bound_rxq_list, XA_FLAGS_ALLOC); > + > + refcount_set(&binding->ref, 1); > + > + binding->dmabuf = dmabuf; > + > + binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent); > + if (IS_ERR(binding->attachment)) { > + err = PTR_ERR(binding->attachment); > + goto err_free_id; > + } > + > + binding->sgt = > + dma_buf_map_attachment(binding->attachment, DMA_FROM_DEVICE); > + if (IS_ERR(binding->sgt)) { > + err = PTR_ERR(binding->sgt); > + goto err_detach; > + } > + > + /* For simplicity we expect to make PAGE_SIZE allocations, but the > + * binding can be much more flexible than that. We may be able to > + * allocate MTU sized chunks here. Leave that for future work... > + */ > + binding->chunk_pool = > + gen_pool_create(PAGE_SHIFT, dev_to_node(&dev->dev)); > + if (!binding->chunk_pool) { > + err = -ENOMEM; > + goto err_unmap; > + } > + > + virtual = 0; > + for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) { > + dma_addr_t dma_addr = sg_dma_address(sg); > + struct dmabuf_genpool_chunk_owner *owner; > + size_t len = sg_dma_len(sg); > + struct net_iov *niov; > + > + owner = kzalloc_node(sizeof(*owner), GFP_KERNEL, > + dev_to_node(&dev->dev)); > + owner->base_virtual = virtual; > + owner->base_dma_addr = dma_addr; > + owner->num_niovs = len / PAGE_SIZE; > + owner->binding = binding; > + > + err = gen_pool_add_owner(binding->chunk_pool, dma_addr, > + dma_addr, len, dev_to_node(&dev->dev), > + owner); > + if (err) { > + err = -EINVAL; > + goto err_free_chunks; > + } > + > + owner->niovs = kvmalloc_array(owner->num_niovs, > + sizeof(*owner->niovs), > + GFP_KERNEL); > + if (!owner->niovs) { > + err = -ENOMEM; > + goto err_free_chunks; > + } > + > + for (i = 0; i < owner->num_niovs; i++) { > + niov = &owner->niovs[i]; > + niov->owner = owner; > + } > + > + virtual += len; > + } > + > + *out = binding; > + > + return 0; > + > +err_free_chunks: > + gen_pool_for_each_chunk(binding->chunk_pool, > + net_devmem_dmabuf_free_chunk_owner, NULL); > + gen_pool_destroy(binding->chunk_pool); > +err_unmap: > + dma_buf_unmap_attachment(binding->attachment, binding->sgt, > + DMA_FROM_DEVICE); > +err_detach: > + dma_buf_detach(dmabuf, binding->attachment); > +err_free_id: > + xa_erase(&net_devmem_dmabuf_bindings, binding->id); > +err_free_binding: > + kfree(binding); > +err_put_dmabuf: > + dma_buf_put(dmabuf); > + return err; > +} > +#endif > diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c > index 9acd0d893765a..3dcd25049e593 100644 > --- a/net/core/netdev-genl-gen.c > +++ b/net/core/netdev-genl-gen.c > @@ -9,6 +9,7 @@ > #include "netdev-genl-gen.h" > > #include <uapi/linux/netdev.h> > +#include <linux/list.h> > > /* Integer value ranges */ > static const struct netlink_range_validation netdev_a_page_pool_id_range = { > @@ -187,4 +188,7 @@ struct genl_family netdev_nl_family __ro_after_init = { > .n_split_ops = ARRAY_SIZE(netdev_nl_ops), > .mcgrps = netdev_nl_mcgrps, > .n_mcgrps = ARRAY_SIZE(netdev_nl_mcgrps), > + .sock_priv_size = sizeof(struct list_head), > + .sock_priv_init = (void *)netdev_nl_sock_priv_init, > + .sock_priv_destroy = (void *)netdev_nl_sock_priv_destroy, > }; > diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h > index ca5a0983f2834..2c431b7dcbc84 100644 > --- a/net/core/netdev-genl-gen.h > +++ b/net/core/netdev-genl-gen.h > @@ -10,6 +10,7 @@ > #include <net/genetlink.h> > > #include <uapi/linux/netdev.h> > +#include <linux/list.h> > > /* Common nested types */ > extern const struct nla_policy netdev_page_pool_info_nl_policy[NETDEV_A_PAGE_POOL_IFINDEX + 1]; > @@ -40,4 +41,7 @@ enum { > > extern struct genl_family netdev_nl_family; > > +void netdev_nl_sock_priv_init(struct list_head *priv); > +void netdev_nl_sock_priv_destroy(struct list_head *priv); > + > #endif /* _LINUX_NETDEV_GEN_H */ > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c > index 2d726e65211dd..133884eb13349 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 "netdev-genl-gen.h" > #include "dev.h" > @@ -721,10 +722,92 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb, > return err; > } > > -/* Stub */ > int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info) > { > - return 0; > + struct nlattr *tb[ARRAY_SIZE(netdev_queue_dmabuf_nl_policy)]; > + struct net_devmem_dmabuf_binding *out_binding; > + struct list_head *sock_binding_list; > + u32 ifindex, dmabuf_fd, rxq_idx; > + struct net_device *netdev; > + struct sk_buff *rsp; > + struct nlattr *attr; > + int rem, err = 0; > + void *hdr; > + > + if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_DEV_IFINDEX) || > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_DMABUF_FD) || > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_QUEUES)) > + return -EINVAL; > + > + ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]); > + dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_BIND_DMABUF_DMABUF_FD]); > + > + rtnl_lock(); > + > + netdev = __dev_get_by_index(genl_info_net(info), ifindex); > + if (!netdev) { > + err = -ENODEV; > + goto err_unlock; > + } > + > + err = net_devmem_bind_dmabuf(netdev, dmabuf_fd, &out_binding); > + if (err) > + goto err_unlock; > + > + nla_for_each_attr(attr, genlmsg_data(info->genlhdr), > + genlmsg_len(info->genlhdr), rem) { > + if (nla_type(attr) != NETDEV_A_BIND_DMABUF_QUEUES) > + continue; > + > + err = nla_parse_nested( > + tb, ARRAY_SIZE(netdev_queue_dmabuf_nl_policy) - 1, attr, > + netdev_queue_dmabuf_nl_policy, info->extack); > + if (err < 0) > + goto err_unbind; > + > + rxq_idx = nla_get_u32(tb[NETDEV_A_QUEUE_DMABUF_IDX]); > + > + err = net_devmem_bind_dmabuf_to_queue(netdev, rxq_idx, > + out_binding); > + if (err) > + goto err_unbind; > + } > + > + sock_binding_list = genl_sk_priv_get(&netdev_nl_family, > + NETLINK_CB(skb).sk); > + if (IS_ERR(sock_binding_list)) { > + err = PTR_ERR(sock_binding_list); > + goto err_unbind; > + } > + > + list_add(&out_binding->list, sock_binding_list); > + > + rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL); > + if (!rsp) { > + err = -ENOMEM; > + goto err_unbind; > + } > + > + hdr = genlmsg_iput(rsp, info); > + if (!hdr) { > + err = -EMSGSIZE; > + goto err_genlmsg_free; > + } > + > + nla_put_u32(rsp, NETDEV_A_BIND_DMABUF_DMABUF_ID, out_binding->id); > + genlmsg_end(rsp, hdr); > + > + rtnl_unlock(); > + > + return genlmsg_reply(rsp, info); > + > +err_genlmsg_free: > + nlmsg_free(rsp); > +err_unbind: > + net_devmem_unbind_dmabuf(out_binding); > +err_unlock: > + rtnl_unlock(); > + return err; > } > > static int netdev_genl_netdevice_event(struct notifier_block *nb, > @@ -771,3 +854,17 @@ static int __init netdev_genl_init(void) > } > > subsys_initcall(netdev_genl_init); > + > +void netdev_nl_sock_priv_init(struct list_head *priv) > +{ > + INIT_LIST_HEAD(priv); > +} > + > +void netdev_nl_sock_priv_destroy(struct list_head *priv) > +{ > + struct net_devmem_dmabuf_binding *binding; > + struct net_devmem_dmabuf_binding *temp; > + > + list_for_each_entry_safe(binding, temp, priv, list) > + net_devmem_unbind_dmabuf(binding); > +} > -- > 2.45.2.803.g4e1b14247a-goog > > I found several locking warnings while testing. [ 1135.125874] WARNING: CPU: 1 PID: 1644 at drivers/dma-buf/dma-buf.c:1123 dma_buf_map_attachment+0x164/0x2f0 [ 1135.136255] Modules linked in: 8021q garp mrp xt_nat xt_tcpudp veth xt_conntrack nft_chain_nat xt_MASQUERADE nf_nat nf _conntrack_netlink nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xfrm_user xt_addrtype nft_compat nf_tables br_netfilter bri dge stp llc qrtr crct10dif_pclmul overlay crc32_generic crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 sha256 _ssse3 sha1_ssse3 xts amdgpu cts wmi_bmof aesni_intel amdxcp i2c_algo_bit crypto_simd drm_ttm_helper cryptd ttm drm_exec bnxt_en ionic gpu_sched drm_suballoc_helper drm_buddy ptp video drm_display_helper drm_kms_helper wmi cfg80211 drm drm_pa nel_orientation_quirks backlight nfnetlink bpf_preload ip_tables x_tables [ 1135.196164] CPU: 1 PID: 1644 Comm: ncdevmem Not tainted 6.10.0-rc5+ #43 6e089cf25edb5a71cabb8ab97c9dfbf7e96b1a3a [ 1135.207060] Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021 [ 1135.215959] RIP: 0010:dma_buf_map_attachment+0x164/0x2f0 [ 1135.221996] Code: ea 03 80 3c 02 00 0f 85 4e 01 00 00 49 8b bc 24 b8 00 00 00 be ff ff ff ff 48 83 c7 70 e8 54 e6 e2 0 0 85 c0 0f 85 32 ff ff ff <0f> 0b e9 2b ff ff ff 89 ee 48 89 df e8 6b f1 ff ff 48 85 c0 0f 84 [ 1135.241464] RSP: 0018:ffff888224c2f5d0 EFLAGS: 00010246 [ 1135.247409] RAX: 0000000000000000 RBX: ffff88821dcc65f0 RCX: 0000000000000001 [ 1135.255259] RDX: 0000000000000001 RSI: ffffffff86abed00 RDI: ffffffff86d56be0 [ 1135.263109] RBP: 0000000000000002 R08: 0000000000000001 R09: ffffed1044985ea0 [ 1135.270960] R10: 0000000000000001 R11: 0000000000000000 R12: ffff88812da88400 [ 1135.278808] R13: ffff88821dcc65f0 R14: ffff888224c2f838 R15: 0000000000000005 [ 1135.286650] FS: 00007fe9b77b4740(0000) GS:ffff88881b200000(0000) knlGS:0000000000000000 [ 1135.295447] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1135.301912] CR2: 0000556a6a61f870 CR3: 00000001094a0000 CR4: 00000000007506f0 [ 1135.309893] PKRU: 55555554 [ 1135.313321] Call Trace: [ 1135.316486] <TASK> [ 1135.319307] ? __warn+0xc8/0x2f0 [ 1135.323290] ? dma_buf_map_attachment+0x164/0x2f0 [ 1135.328907] ? report_bug+0x326/0x3c0 [ 1135.333368] ? handle_bug+0x3c/0x70 [ 1135.337568] ? exc_invalid_op+0x14/0x50 [ 1135.342202] ? asm_exc_invalid_op+0x16/0x20 [ 1135.347134] ? dma_buf_map_attachment+0x164/0x2f0 [ 1135.352686] net_devmem_bind_dmabuf+0x2af/0xab0 [ 1135.357940] ? __nla_validate_parse+0x109e/0x2830 [ 1135.363430] netdev_nl_bind_rx_doit+0x26f/0xe00 [ 1135.368675] ? __pfx___nla_validate_parse+0x10/0x10 [ 1135.374333] ? __pfx_netdev_nl_bind_rx_doit+0x10/0x10 [ 1135.380094] ? trace_kmalloc+0x2d/0xd0 [ 1135.384637] ? __kmalloc_noprof+0x1f5/0x430 [ 1135.389539] ? __pfx_mark_lock.part.0+0x10/0x10 [ 1135.394851] ? __nla_parse+0x22/0x30 [ 1135.399139] ? genl_family_rcv_msg_attrs_parse.constprop.0+0x162/0x240 [ 1135.406403] genl_family_rcv_msg_doit+0x1d4/0x2b0 [ 1135.411872] ? __pfx_genl_family_rcv_msg_doit+0x10/0x10 [ 1135.417892] genl_rcv_msg+0x3fb/0x6c0 [ 1136.178258] WARNING: CPU: 1 PID: 1644 at drivers/dma-buf/dma-buf.c:1226 dma_buf_unmap_attachment+0x267/0x320 [ 1136.188842] Modules linked in: 8021q garp mrp xt_nat xt_tcpudp veth xt_conntrack nft_chain_nat xt_MASQUERADE nf_nat nf _conntrack_netlink nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xfrm_user xt_addrtype nft_compat nf_tables br_netfilter bri dge stp llc qrtr crct10dif_pclmul overlay crc32_generic crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 sha256 _ssse3 sha1_ssse3 xts amdgpu cts wmi_bmof aesni_intel amdxcp i2c_algo_bit crypto_simd drm_ttm_helper cryptd ttm drm_exec bnxt_en ionic gpu_sched drm_suballoc_helper drm_buddy ptp video drm_display_helper drm_kms_helper wmi cfg80211 drm drm_pa nel_orientation_quirks backlight nfnetlink bpf_preload ip_tables x_tables [ 1136.248891] CPU: 1 PID: 1644 Comm: ncdevmem Tainted: G W 6.10.0-rc5+ #43 6e089cf25edb5a71cabb8ab97c9dfbf7e96b1a3a [ 1136.261273] Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021 [ 1136.270266] RIP: 0010:dma_buf_unmap_attachment+0x267/0x320 [ 1136.276468] Code: c1 ea 03 80 3c 02 00 0f 85 c1 00 00 00 48 8b bb b8 00 00 00 be ff ff ff ff 48 83 c7 70 e8 11 e1 e2 0 0 85 c0 0f 85 42 fe ff ff <0f> 0b e9 3b fe ff ff 48 89 cf 4c 89 44 24 10 e8 35 c8 21 ff 4c 8b [ 1136.295967] RSP: 0018:ffff888224c2fb78 EFLAGS: 00010246 [ 1136.301930] RAX: 0000000000000000 RBX: ffff88812da88400 RCX: 0000000000000001 [ 1136.309804] RDX: 0000000000000001 RSI: ffffffff86abed00 RDI: ffffffff86d56be0 [ 1136.317810] RBP: ffff88810908e250 R08: 0000000000000001 R09: fffffbfff21514d8 [ 1136.325670] R10: 0000000000000001 R11: 0000000000000000 R12: ffff88821dcc65f0 [ 1136.333608] R13: 0000000000010000 R14: ffffed102265e337 R15: 1ffff11044985f81 [ 1136.341462] FS: 00007fe9b77b4740(0000) GS:ffff88881b200000(0000) knlGS:0000000000000000 [ 1136.350278] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1136.356766] CR2: 000055dec9867ba0 CR3: 00000001094a0000 CR4: 00000000007506f0 [ 1136.364640] PKRU: 55555554 [ 1136.368172] Call Trace: [ 1136.371373] <TASK> [ 1136.374275] ? __warn+0xc8/0x2f0 [ 1136.378230] ? dma_buf_unmap_attachment+0x267/0x320 [ 1136.383855] ? report_bug+0x326/0x3c0 [ 1136.388324] ? handle_bug+0x3c/0x70 [ 1136.392549] ? exc_invalid_op+0x14/0x50 [ 1136.397126] ? asm_exc_invalid_op+0x16/0x20 [ 1136.402126] ? dma_buf_unmap_attachment+0x267/0x320 [ 1136.407727] ? dma_buf_unmap_attachment+0x25f/0x320 [ 1136.413344] __net_devmem_dmabuf_binding_free+0x10a/0x220 [ 1136.419728] net_devmem_unbind_dmabuf+0x349/0x440 [ 1136.425146] ? __pfx_lock_release+0x10/0x10 [ 1136.430080] ? __pfx_net_devmem_unbind_dmabuf+0x10/0x10 [ 1136.436106] netdev_nl_sock_priv_destroy+0x72/0xc0 [ 1136.441611] genl_release+0xed/0x190 [ 1136.445921] ? __pfx_genl_release+0x10/0x10 [ 1136.450823] ? mark_held_locks+0xa5/0xf0 [ 1136.455490] ? __local_bh_enable_ip+0xa5/0x120 [ 1136.460790] ? __pfx_genl_release+0x10/0x10 [ 1136.465703] netlink_release+0x839/0x18f0 [ 1135.709313] WARNING: CPU: 3 PID: 1644 at net/core/netdev_rx_queue.c:18 netdev_rx_queue_restart+0x3f4/0x5a0 [ 1135.719686] Modules linked in: 8021q garp mrp xt_nat xt_tcpudp veth xt_conntrack nft_chain_nat xt_MASQUERADE nf_nat nf _conntrack_netlink nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xfrm_user xt_addrtype nft_compat nf_tables br_netfilter bri dge stp llc qrtr crct10dif_pclmul overlay crc32_generic crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 sha256 _ssse3 sha1_ssse3 xts amdgpu cts wmi_bmof aesni_intel amdxcp i2c_algo_bit crypto_simd drm_ttm_helper cryptd ttm drm_exec bnxt_en ionic gpu_sched drm_suballoc_helper drm_buddy ptp video drm_display_helper drm_kms_helper wmi cfg80211 drm drm_pa nel_orientation_quirks backlight nfnetlink bpf_preload ip_tables x_tables [ 1135.779526] CPU: 3 PID: 1644 Comm: ncdevmem Tainted: G W 6.10.0-rc5+ #43 6e089cf25edb5a71cabb8ab97c9df bf7e96b1a3a [ 1135.791882] Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021 [ 1135.800781] RIP: 0010:netdev_rx_queue_restart+0x3f4/0x5a0 [ 1135.806905] Code: d0 0f 1f 00 48 89 df e8 9a ce a9 fe 4c 89 f7 e8 92 ce a9 fe 48 83 c4 08 44 89 e0 5b 5d 41 5c 41 5d 4 1 5e 41 5f c3 cc cc cc cc <0f> 0b e9 05 fd ff ff 44 89 fe 48 c7 c7 80 70 fc 86 e8 46 7c 38 fe [ 1135.826382] RSP: 0018:ffff888224c2fbb0 EFLAGS: 00010246 [ 1135.832339] RAX: 0000000000000000 RBX: ffffffffc0bb5c80 RCX: ffffffff842d81c3 [ 1135.840185] RDX: 1ffffffff1e6fc44 RSI: 0000000000000008 RDI: ffffffff8f37e220 [ 1135.848028] RBP: ffff88814b864000 R08: 0000000000000000 R09: fffffbfff1e6fc44 [ 1135.855875] R10: ffffffff8f37e227 R11: 0000000000000000 R12: ffff888224c2fc28 [ 1135.863716] R13: ffff88814b864be0 R14: ffffed102265e337 R15: 0000000000000001 [ 1135.871560] FS: 00007fe9b77b4740(0000) GS:ffff88881ba00000(0000) knlGS:0000000000000000 [ 1135.880364] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1135.886824] CR2: 00007f5f22a21f50 CR3: 00000001094a0000 CR4: 00000000007506f0 [ 1135.894670] PKRU: 55555554 [ 1135.898090] Call Trace: [ 1135.901255] <TASK> [ 1135.904073] ? __warn+0xc8/0x2f0 [ 1135.908020] ? netdev_rx_queue_restart+0x3f4/0x5a0 [ 1135.913524] ? report_bug+0x326/0x3c0 [ 1135.917908] ? handle_bug+0x3c/0x70 [ 1135.922112] ? exc_invalid_op+0x14/0x50 [ 1135.926671] ? asm_exc_invalid_op+0x16/0x20 [ 1135.931591] ? mutex_is_locked+0x13/0x50 [ 1135.936238] ? netdev_rx_queue_restart+0x3f4/0x5a0 [ 1135.941748] net_devmem_unbind_dmabuf+0x2a3/0x440 [ 1135.947179] ? __pfx_lock_release+0x10/0x10 [ 1135.952081] ? __pfx_net_devmem_unbind_dmabuf+0x10/0x10 [ 1135.958040] netdev_nl_sock_priv_destroy+0x72/0xc0 [ 1135.963561] genl_release+0xed/0x190 [ 1135.967851] ? __pfx_genl_release+0x10/0x10 [ 1135.972755] ? mark_held_locks+0xa5/0xf0 [ 1135.977392] ? __local_bh_enable_ip+0xa5/0x120 [ 1135.982561] ? __pfx_genl_release+0x10/0x10 [ 1135.987464] netlink_release+0x839/0x18f0 Thanks! Taehee Yoo
On Thu, Jul 4, 2024 at 10:57 AM Taehee Yoo <ap420073@gmail.com> wrote: > > I found several locking warnings while testing. > Thanks for Testing Taehee! And sorry for the late reply. I was off for a couple of days. With some minor tweaks to my test setup I was able to reproduce and fix all 3 warnings. > [ 1135.125874] WARNING: CPU: 1 PID: 1644 at > drivers/dma-buf/dma-buf.c:1123 dma_buf_map_attachment+0x164/0x2f0 ... > [ 1136.178258] WARNING: CPU: 1 PID: 1644 at > drivers/dma-buf/dma-buf.c:1226 dma_buf_unmap_attachment+0x267/0x320 Both of these are warnings that dma->resv is not locked when calling dma_buf_[un]map_attachment(). As far as I can tell so far, this can be resolved by using the unlocked versions: dma_buf_[un]map_attachment_unlocked() which is correct here for this static importer. ... > [ 1135.709313] WARNING: CPU: 3 PID: 1644 at > net/core/netdev_rx_queue.c:18 netdev_rx_queue_restart+0x3f4/0x5a0 This is due to rtnl_lock() actually not being acquired in the unbind path, when the netlink socket is closed. Sorry about that. This is fixed by obtaining rtnl_lock() in the unbind path. With the fixes below all the warnings disappear. I'm planning to squash them to the next version. Let me know if those don't work for you. Thanks! diff --git a/net/core/devmem.c b/net/core/devmem.c index e52bca1a55c7c..a6ef1485b80f2 100644 --- a/net/core/devmem.c +++ b/net/core/devmem.c @@ -46,8 +46,8 @@ void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding) size, avail)) gen_pool_destroy(binding->chunk_pool); - dma_buf_unmap_attachment(binding->attachment, binding->sgt, - DMA_FROM_DEVICE); + dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt, + DMA_FROM_DEVICE); dma_buf_detach(binding->dmabuf, binding->attachment); dma_buf_put(binding->dmabuf); xa_destroy(&binding->bound_rxqs); @@ -157,8 +157,8 @@ struct net_devmem_dmabuf_binding *net_devmem_bind_dmabuf(struct net_device *dev, goto err_free_id; } - binding->sgt = - dma_buf_map_attachment(binding->attachment, DMA_FROM_DEVICE); + binding->sgt = dma_buf_map_attachment_unlocked(binding->attachment, + DMA_FROM_DEVICE); if (IS_ERR(binding->sgt)) { err = PTR_ERR(binding->sgt); goto err_detach; @@ -225,8 +225,8 @@ struct net_devmem_dmabuf_binding *net_devmem_bind_dmabuf(struct net_device *dev, net_devmem_dmabuf_free_chunk_owner, NULL); gen_pool_destroy(binding->chunk_pool); err_unmap: - dma_buf_unmap_attachment(binding->attachment, binding->sgt, - DMA_FROM_DEVICE); + dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt, + DMA_FROM_DEVICE); err_detach: dma_buf_detach(dmabuf, binding->attachment); err_free_id: diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index 4b16b3ad2ec5b..33bb20c143997 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -861,6 +861,9 @@ void netdev_nl_sock_priv_destroy(struct list_head *priv) struct net_devmem_dmabuf_binding *binding; struct net_devmem_dmabuf_binding *temp; - list_for_each_entry_safe(binding, temp, priv, list) + list_for_each_entry_safe(binding, temp, priv, list) { + rtnl_lock(); net_devmem_unbind_dmabuf(binding); + rtnl_unlock(); + } } -- Thanks, Mina
On Tue, Jul 9, 2024 at 5:08 AM Mina Almasry <almasrymina@google.com> wrote: > Hi Mina, Thanks a lot for your reply! > On Thu, Jul 4, 2024 at 10:57 AM Taehee Yoo <ap420073@gmail.com> wrote: > > > > I found several locking warnings while testing. > > > > Thanks for Testing Taehee! And sorry for the late reply. I was off for > a couple of days. With some minor tweaks to my test setup I was able > to reproduce and fix all 3 warnings. > > > [ 1135.125874] WARNING: CPU: 1 PID: 1644 at > > drivers/dma-buf/dma-buf.c:1123 dma_buf_map_attachment+0x164/0x2f0 > ... > > [ 1136.178258] WARNING: CPU: 1 PID: 1644 at > > drivers/dma-buf/dma-buf.c:1226 dma_buf_unmap_attachment+0x267/0x320 > > Both of these are warnings that dma->resv is not locked when calling > dma_buf_[un]map_attachment(). As far as I can tell so far, this can be > resolved by using the unlocked versions: > dma_buf_[un]map_attachment_unlocked() which is correct here for this > static importer. > > ... > > > [ 1135.709313] WARNING: CPU: 3 PID: 1644 at > > net/core/netdev_rx_queue.c:18 netdev_rx_queue_restart+0x3f4/0x5a0 > > This is due to rtnl_lock() actually not being acquired in the unbind > path, when the netlink socket is closed. Sorry about that. This is > fixed by obtaining rtnl_lock() in the unbind path. > > With the fixes below all the warnings disappear. I'm planning to > squash them to the next version. Let me know if those don't work for > you. Thanks! > > diff --git a/net/core/devmem.c b/net/core/devmem.c > index e52bca1a55c7c..a6ef1485b80f2 100644 > --- a/net/core/devmem.c > +++ b/net/core/devmem.c > @@ -46,8 +46,8 @@ void __net_devmem_dmabuf_binding_free(struct > net_devmem_dmabuf_binding *binding) > size, avail)) > gen_pool_destroy(binding->chunk_pool); > > - dma_buf_unmap_attachment(binding->attachment, binding->sgt, > - DMA_FROM_DEVICE); > + dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt, > + DMA_FROM_DEVICE); > dma_buf_detach(binding->dmabuf, binding->attachment); > dma_buf_put(binding->dmabuf); > xa_destroy(&binding->bound_rxqs); > @@ -157,8 +157,8 @@ struct net_devmem_dmabuf_binding > *net_devmem_bind_dmabuf(struct net_device *dev, > goto err_free_id; > } > > - binding->sgt = > - dma_buf_map_attachment(binding->attachment, DMA_FROM_DEVICE); > + binding->sgt = dma_buf_map_attachment_unlocked(binding->attachment, > + DMA_FROM_DEVICE); > if (IS_ERR(binding->sgt)) { > err = PTR_ERR(binding->sgt); > goto err_detach; > @@ -225,8 +225,8 @@ struct net_devmem_dmabuf_binding > *net_devmem_bind_dmabuf(struct net_device *dev, > net_devmem_dmabuf_free_chunk_owner, NULL); > gen_pool_destroy(binding->chunk_pool); > err_unmap: > - dma_buf_unmap_attachment(binding->attachment, binding->sgt, > - DMA_FROM_DEVICE); > + dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt, > + DMA_FROM_DEVICE); > err_detach: > dma_buf_detach(dmabuf, binding->attachment); > err_free_id: > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c > index 4b16b3ad2ec5b..33bb20c143997 100644 > --- a/net/core/netdev-genl.c > +++ b/net/core/netdev-genl.c > @@ -861,6 +861,9 @@ void netdev_nl_sock_priv_destroy(struct list_head *priv) > struct net_devmem_dmabuf_binding *binding; > struct net_devmem_dmabuf_binding *temp; > > - list_for_each_entry_safe(binding, temp, priv, list) > + list_for_each_entry_safe(binding, temp, priv, list) { > + rtnl_lock(); > net_devmem_unbind_dmabuf(binding); > + rtnl_unlock(); > + } > } > > > > -- > Thanks, > Mina I tested the above fix, it works well. And I found another bug. [ 236.625141] BUG: KASAN: slab-use-after-free in net_devmem_unbind_dmabuf+0x364/0x440 [ 236.633488] Read of size 8 at addr ffff8881490d00b0 by task ncdevmem/1480 [ 236.643137] CPU: 0 PID: 1480 Comm: ncdevmem Tainted: G W 6.10.0-rc5+ #50 8d4b0a557c4b34e2938739913129f 4523354121c [ 236.655443] Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021 [ 236.664307] Call Trace: [ 236.667443] <TASK> [ 236.670234] dump_stack_lvl+0x7e/0xc0 [ 236.674583] print_report+0xc1/0x5e0 [ 236.678850] ? __virt_addr_valid+0x1f5/0x3d0 [ 236.683803] ? net_devmem_unbind_dmabuf+0x364/0x440 [ 236.689362] kasan_report+0xb9/0xf0 [ 236.693536] ? net_devmem_unbind_dmabuf+0x364/0x440 [ 236.699094] net_devmem_unbind_dmabuf+0x364/0x440 [ 236.704487] ? __pfx_lock_release+0x10/0x10 [ 236.709352] ? __pfx_net_devmem_unbind_dmabuf+0x10/0x10 [ 236.715256] netdev_nl_sock_priv_destroy+0x77/0xd0 [ 236.720743] genl_release+0xed/0x190 [ 236.725004] ? __pfx_genl_release+0x10/0x10 [ 236.729870] ? rcu_is_watching+0x11/0xb0 [ 236.734476] ? netlink_release+0x7d8/0x18f0 [ 236.739343] ? trace_irq_enable.constprop.0+0xe4/0x130 [ 236.745168] ? __pfx_genl_release+0x10/0x10 [ 236.750034] netlink_release+0x839/0x18f0 [ 236.754727] ? netlink_release+0x1a9/0x18f0 [ 236.759594] ? __pfx_netlink_release+0x10/0x10 [ 236.764719] ? __pfx_down_write+0x10/0x10 [ 236.769413] ? __pfx_locks_remove_file+0x10/0x10 [ 236.774718] __sock_release+0xa3/0x260 [ 236.779153] sock_close+0x14/0x20 [ 236.783153] __fput+0x367/0xad0 [ 236.786982] ? trace_irq_enable.constprop.0+0xe4/0x130 [ 236.792801] task_work_run+0x12e/0x220 [ 236.797243] ? __pfx_task_work_run+0x10/0x10 [ 236.802193] ? do_raw_spin_unlock+0x54/0x220 [ 236.807149] do_exit+0x916/0x2570 ... [ 236.994294] Allocated by task 1503: [ 236.998470] kasan_save_stack+0x20/0x40 [ 237.002992] kasan_save_track+0x10/0x30 [ 237.007513] __kasan_slab_alloc+0x83/0x90 [ 237.012203] kmem_cache_alloc_node_noprof+0x154/0x380 [ 237.017936] kmalloc_reserve+0x140/0x240 [ 237.022541] __alloc_skb+0x10d/0x2d0 [ 237.026801] alloc_uevent_skb+0x79/0x210 [ 237.031408] kobject_uevent_env+0xd7c/0x10e0 [ 237.036362] __kobject_del+0x131/0x1d0 [ 237.040794] kobject_put+0x23e/0x3f0 [ 237.045056] net_rx_queue_update_kobjects+0x35d/0x470 [ 237.050789] netdev_unregister_kobject+0x139/0x250 [ 237.056266] unregister_netdevice_many_notify+0xf05/0x1900 [ 237.062429] unregister_netdevice_queue+0x29a/0x360 [ 237.067988] unregister_netdev+0x18/0x20 [ 237.072594] 0xffffffffc09de73c [ 237.076422] pci_device_remove+0xa7/0x1d0 [ 237.081140] device_release_driver_internal+0x36d/0x530 [ 237.087044] driver_detach+0xc1/0x180 [ 237.091392] bus_remove_driver+0x11a/0x2a0 [ 237.096173] pci_unregister_driver+0x26/0x250 [ 237.101210] 0xffffffffc0a47b3c [ 237.105038] __do_sys_delete_module.constprop.0+0x2ff/0x4b0 [ 237.111289] do_syscall_64+0x64/0x140 [ 237.115637] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 237.123553] Freed by task 279: [ 237.127296] kasan_save_stack+0x20/0x40 [ 237.131816] kasan_save_track+0x10/0x30 [ 237.136337] kasan_save_free_info+0x37/0x60 [ 237.141203] poison_slab_object+0xee/0x170 [ 237.145983] __kasan_slab_free+0x2f/0x50 [ 237.150589] kmem_cache_free+0x12e/0x470 [ 237.155197] skb_release_data+0x51f/0x790 [ 237.159891] consume_skb+0xa7/0x110 [ 237.164063] netlink_recvmsg+0x4f9/0xc80 [ 237.168672] ____sys_recvmsg+0x5fc/0x860 [ 237.173278] ___sys_recvmsg+0xd3/0x150 [ 237.177712] __sys_recvmsg+0xc6/0x160 [ 237.182060] do_syscall_64+0x64/0x140 [ 237.186405] entry_SYSCALL_64_after_hwframe+0x76/0x7e [ 237.194320] The buggy address belongs to the object at ffff8881490d0040 which belongs to the cache skbuff_small_head of size 640 [ 237.208701] The buggy address is located 112 bytes inside of freed 640-byte region [ffff8881490d0040, ffff8881490d02c0) [ 237.224514] The buggy address belongs to the physical page: [ 237.230763] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1490d0 [ 237.239433] head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0 [ 237.247755] flags: 0x200000000000040(head|node=0|zone=2) [ 237.253746] page_type: 0xffffefff(slab) [ 237.258265] raw: 0200000000000040 ffff8881050cadc0 ffffea0004153b10 ffffea00044ddb10 [ 237.266677] raw: 0000000000000000 0000000000120012 00000001ffffefff 0000000000000000 [ 237.275096] head: 0200000000000040 ffff8881050cadc0 ffffea0004153b10 ffffea00044ddb10 [ 237.283599] head: 0000000000000000 0000000000120012 00000001ffffefff 0000000000000000 [ 237.292095] head: 0200000000000002 ffffea0005243401 ffffffffffffffff 0000000000000000 [ 237.300590] head: 0000000000000004 0000000000000000 00000000ffffffff 0000000000000000 [ 237.309088] page dumped because: kasan: bad access detected [ 237.317519] Memory state around the buggy address: [ 237.322993] ffff8881490cff80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 237.330887] ffff8881490d0000: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb [ 237.338776] >ffff8881490d0080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 237.346668] ^ [ 237.352139] ffff8881490d0100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 237.360032] ffff8881490d0180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 237.367921] ================================================================== Reproducer: ./ncdevmem -f <interface name> -l -p 5201 -v 7 -t 0 -q 2 & sleep 10 modprobe -rv bnxt_en killall ncdevmem I think it's a devmemTCP core bug so this issue would be reproduced with other drivers. Thanks! Taehee Yoo
On Tue, Jul 9, 2024 at 8:37 AM Taehee Yoo <ap420073@gmail.com> wrote: > ... > And I found another bug. > > [ 236.625141] BUG: KASAN: slab-use-after-free in > net_devmem_unbind_dmabuf+0x364/0x440 ... > Reproducer: > ./ncdevmem -f <interface name> -l -p 5201 -v 7 -t 0 -q 2 & > sleep 10 > modprobe -rv bnxt_en > killall ncdevmem > > I think it's a devmemTCP core bug so this issue would be reproduced > with other drivers. > Thanks again for testing Taehee. I haven't looked into reproducing yet but the issue seems obvious from the repro and the trace. What happens is that when we bind an rxq we add it to bound_rxq_list, and then when we unbind we access the rxq in the list, without checking if it's still alive. With your sequence, the rxq is freed before the unbind happens, I think, so we hit a use-after-free. The fix, I think, should be simple, we need to remember to remove the rxq from bound_rxq_list as it is deallocated so there is no access after free. Btw, I have all the rest of the feedback addressed (including netlink introspection) and I was in the process of rebasing and build-testing a new version, to try to get in before net-next closes if at all possible. I don't think I'll be able to fix this particular issue in time, but I should be able to submit a fix targeting the net tree during the merged window, if that's OK. If folks feel this issue is blocking, please let me know so I don't send another version before net-next reopens.
On Tue, Jul 9, 2024 at 8:37 AM Taehee Yoo <ap420073@gmail.com> wrote: ... > Reproducer: > ./ncdevmem -f <interface name> -l -p 5201 -v 7 -t 0 -q 2 & > sleep 10 > modprobe -rv bnxt_en > killall ncdevmem > > I think it's a devmemTCP core bug so this issue would be reproduced > with other drivers. Sorry for the late reply. I was out at netdev. I'm also having trouble reproducing this, not because the bug doesn't exist, but quirks with my test setup that I need to figure out. AFAICT this diff should fix the issue. If you have time to confirm, let me know if it doesn't work for you. It should apply on top of v16: commit 795b8ff01906d ("fix for release issue") Author: Mina Almasry <almasrymina@google.com> Date: Tue Jul 23 00:18:23 2024 +0000 fix for release issue Change-Id: Ib45a0aa6cba2918db5f7ba535414ffa860911fa4 diff --git a/include/net/devmem.h b/include/net/devmem.h index 51b25ba193c96..df52526bb516a 100644 --- a/include/net/devmem.h +++ b/include/net/devmem.h @@ -68,6 +68,9 @@ net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd); void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding); int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, struct net_devmem_dmabuf_binding *binding); + +void dev_dmabuf_uninstall(struct net_device *dev); + struct net_iov * net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding); void net_devmem_free_dmabuf(struct net_iov *ppiov); diff --git a/net/core/dev.c b/net/core/dev.c index 5882ddc3f8592..7be084e4936e4 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -11320,6 +11320,7 @@ void unregister_netdevice_many_notify(struct list_head *head, dev_tcx_uninstall(dev); dev_xdp_uninstall(dev); bpf_dev_bound_netdev_unregister(dev); + dev_dmabuf_uninstall(dev); netdev_offload_xstats_disable_all(dev); diff --git a/net/core/devmem.c b/net/core/devmem.c index e75057ecfa6de..227bcb1070ec0 100644 --- a/net/core/devmem.c +++ b/net/core/devmem.c @@ -362,4 +362,20 @@ bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem) return false; } +void dev_dmabuf_uninstall(struct net_device *dev) +{ + unsigned int i, count = dev->num_rx_queues; + struct net_devmem_dmabuf_binding *binding; + struct netdev_rx_queue *rxq; + unsigned long xa_idx; + + for (i = 0; i < count; i++) { + binding = dev->_rx[i].mp_params.mp_priv; + if (binding) + xa_for_each(&binding->bound_rxqs, xa_idx, rxq) + if (rxq == &dev->_rx[i]) + xa_erase(&binding->bound_rxqs, xa_idx); + } +} + #endif
On Wed, Jul 24, 2024 at 6:49 AM Mina Almasry <almasrymina@google.com> wrote: > > On Tue, Jul 9, 2024 at 8:37 AM Taehee Yoo <ap420073@gmail.com> wrote: > ... > > Reproducer: > > ./ncdevmem -f <interface name> -l -p 5201 -v 7 -t 0 -q 2 & > > sleep 10 > > modprobe -rv bnxt_en > > killall ncdevmem > > > > I think it's a devmemTCP core bug so this issue would be reproduced > > with other drivers. > > Sorry for the late reply. I was out at netdev. > > I'm also having trouble reproducing this, not because the bug doesn't > exist, but quirks with my test setup that I need to figure out. AFAICT > this diff should fix the issue. If you have time to confirm, let me > know if it doesn't work for you. It should apply on top of v16: > > commit 795b8ff01906d ("fix for release issue") > Author: Mina Almasry <almasrymina@google.com> > Date: Tue Jul 23 00:18:23 2024 +0000 > > fix for release issue > > Change-Id: Ib45a0aa6cba2918db5f7ba535414ffa860911fa4 > > > > diff --git a/include/net/devmem.h b/include/net/devmem.h > index 51b25ba193c96..df52526bb516a 100644 > --- a/include/net/devmem.h > +++ b/include/net/devmem.h > @@ -68,6 +68,9 @@ net_devmem_bind_dmabuf(struct net_device *dev, > unsigned int dmabuf_fd); > void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding); > int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, > struct net_devmem_dmabuf_binding *binding); > + > +void dev_dmabuf_uninstall(struct net_device *dev); > + > struct net_iov * > net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding); > void net_devmem_free_dmabuf(struct net_iov *ppiov); > diff --git a/net/core/dev.c b/net/core/dev.c > index 5882ddc3f8592..7be084e4936e4 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -11320,6 +11320,7 @@ void unregister_netdevice_many_notify(struct > list_head *head, > dev_tcx_uninstall(dev); > dev_xdp_uninstall(dev); > bpf_dev_bound_netdev_unregister(dev); > + dev_dmabuf_uninstall(dev); > > netdev_offload_xstats_disable_all(dev); > > diff --git a/net/core/devmem.c b/net/core/devmem.c > index e75057ecfa6de..227bcb1070ec0 100644 > --- a/net/core/devmem.c > +++ b/net/core/devmem.c > @@ -362,4 +362,20 @@ bool mp_dmabuf_devmem_release_page(struct > page_pool *pool, netmem_ref netmem) > return false; > } > > +void dev_dmabuf_uninstall(struct net_device *dev) > +{ > + unsigned int i, count = dev->num_rx_queues; > + struct net_devmem_dmabuf_binding *binding; > + struct netdev_rx_queue *rxq; > + unsigned long xa_idx; > + > + for (i = 0; i < count; i++) { > + binding = dev->_rx[i].mp_params.mp_priv; > + if (binding) > + xa_for_each(&binding->bound_rxqs, xa_idx, rxq) > + if (rxq == &dev->_rx[i]) > + xa_erase(&binding->bound_rxqs, xa_idx); > + } > +} > + > #endif > I tested this patch and it works well. Thanks a lot for this work! Thanks a lot! Taehee Yoo > -- > Thanks, > Mina
diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml index 899ac0882a098..d6d7cb01c145c 100644 --- a/Documentation/netlink/specs/netdev.yaml +++ b/Documentation/netlink/specs/netdev.yaml @@ -673,6 +673,10 @@ operations: - tx-packets - tx-bytes +kernel-family: + headers: [ "linux/list.h"] + sock-priv: struct list_head + mcast-groups: list: - diff --git a/include/net/devmem.h b/include/net/devmem.h new file mode 100644 index 0000000000000..eaf3fd965d7a8 --- /dev/null +++ b/include/net/devmem.h @@ -0,0 +1,111 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Device memory TCP support + * + * Authors: Mina Almasry <almasrymina@google.com> + * Willem de Bruijn <willemb@google.com> + * Kaiyuan Zhang <kaiyuanz@google.com> + * + */ +#ifndef _NET_DEVMEM_H +#define _NET_DEVMEM_H + +struct net_devmem_dmabuf_binding { + struct dma_buf *dmabuf; + struct dma_buf_attachment *attachment; + struct sg_table *sgt; + struct net_device *dev; + struct gen_pool *chunk_pool; + + /* The user holds a ref (via the netlink API) for as long as they want + * the binding to remain alive. Each page pool using this binding holds + * a ref to keep the binding alive. Each allocated net_iov holds a + * ref. + * + * The binding undos itself and unmaps the underlying dmabuf once all + * those refs are dropped and the binding is no longer desired or in + * use. + */ + refcount_t ref; + + /* The list of bindings currently active. Used for netlink to notify us + * of the user dropping the bind. + */ + struct list_head list; + + /* rxq's this binding is active on. */ + struct xarray bound_rxq_list; + + /* ID of this binding. Globally unique to all bindings currently + * active. + */ + u32 id; +}; + +/* Owner of the dma-buf chunks inserted into the gen pool. Each scatterlist + * entry from the dmabuf is inserted into the genpool as a chunk, and needs + * this owner struct to keep track of some metadata necessary to create + * allocations from this chunk. + */ +struct dmabuf_genpool_chunk_owner { + /* Offset into the dma-buf where this chunk starts. */ + unsigned long base_virtual; + + /* dma_addr of the start of the chunk. */ + dma_addr_t base_dma_addr; + + /* Array of net_iovs for this chunk. */ + struct net_iov *niovs; + size_t num_niovs; + + struct net_devmem_dmabuf_binding *binding; +}; + +#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR) +void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding); +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, + struct net_devmem_dmabuf_binding **out); +void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding); +int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, + struct net_devmem_dmabuf_binding *binding); +#else +static inline void +__net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding) +{ +} + +static inline int net_devmem_bind_dmabuf(struct net_device *dev, + unsigned int dmabuf_fd, + struct net_devmem_dmabuf_binding **out) +{ + return -EOPNOTSUPP; +} +static inline void +net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding) +{ +} + +static inline int +net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx, + struct net_devmem_dmabuf_binding *binding) +{ + return -EOPNOTSUPP; +} +#endif + +static inline void +net_devmem_dmabuf_binding_get(struct net_devmem_dmabuf_binding *binding) +{ + refcount_inc(&binding->ref); +} + +static inline void +net_devmem_dmabuf_binding_put(struct net_devmem_dmabuf_binding *binding) +{ + if (!refcount_dec_and_test(&binding->ref)) + return; + + __net_devmem_dmabuf_binding_free(binding); +} + +#endif /* _NET_DEVMEM_H */ diff --git a/include/net/netdev_rx_queue.h b/include/net/netdev_rx_queue.h index e78ca52d67fbf..ac34f5fb4f71d 100644 --- a/include/net/netdev_rx_queue.h +++ b/include/net/netdev_rx_queue.h @@ -6,6 +6,7 @@ #include <linux/netdevice.h> #include <linux/sysfs.h> #include <net/xdp.h> +#include <net/page_pool/types.h> /* This structure contains an instance of an RX queue. */ struct netdev_rx_queue { @@ -25,6 +26,7 @@ struct netdev_rx_queue { * Readers and writers must hold RTNL */ struct napi_struct *napi; + struct pp_memory_provider_params mp_params; } ____cacheline_aligned_in_smp; /* diff --git a/include/net/netmem.h b/include/net/netmem.h index d8b810245c1da..72e932a1a9489 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h @@ -8,6 +8,16 @@ #ifndef _NET_NETMEM_H #define _NET_NETMEM_H +#include <net/devmem.h> + +/* net_iov */ + +struct net_iov { + struct dmabuf_genpool_chunk_owner *owner; +}; + +/* netmem */ + /** * typedef netmem_ref - a nonexistent type marking a reference to generic * network memory. diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index 7e8477057f3d1..9f3c3ee2ee755 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -128,6 +128,10 @@ struct page_pool_stats { }; #endif +struct pp_memory_provider_params { + void *mp_priv; +}; + struct page_pool { struct page_pool_params_fast p; @@ -194,6 +198,8 @@ struct page_pool { */ struct ptr_ring ring; + void *mp_priv; + #ifdef CONFIG_PAGE_POOL_STATS /* recycle stats are per-cpu to avoid locking */ struct page_pool_recycle_stats __percpu *recycle_stats; diff --git a/net/core/Makefile b/net/core/Makefile index f82232b358a2c..6b43611fb4a43 100644 --- a/net/core/Makefile +++ b/net/core/Makefile @@ -13,7 +13,7 @@ obj-y += dev.o dev_addr_lists.o dst.o netevent.o \ neighbour.o rtnetlink.o utils.o link_watch.o filter.o \ sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \ fib_notifier.o xdp.o flow_offload.o gro.o \ - netdev-genl.o netdev-genl-gen.o gso.o + netdev-genl.o netdev-genl-gen.o gso.o devmem.o obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o diff --git a/net/core/dev.c b/net/core/dev.c index 0a23d7da7fbc6..5e5e2d266b83f 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -158,6 +158,9 @@ #include <net/page_pool/types.h> #include <net/page_pool/helpers.h> #include <net/rps.h> +#include <linux/genalloc.h> +#include <linux/dma-buf.h> +#include <net/devmem.h> #include "dev.h" #include "net-sysfs.h" diff --git a/net/core/devmem.c b/net/core/devmem.c new file mode 100644 index 0000000000000..cfb5a2f69dcd2 --- /dev/null +++ b/net/core/devmem.c @@ -0,0 +1,252 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Devmem TCP + * + * Authors: Mina Almasry <almasrymina@google.com> + * Willem de Bruijn <willemdebruijn.kernel@gmail.com> + * Kaiyuan Zhang <kaiyuanz@google.com + */ + +#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> + +/* Device memory support */ + +#if defined(CONFIG_DMA_SHARED_BUFFER) && defined(CONFIG_GENERIC_ALLOCATOR) +static void net_devmem_dmabuf_free_chunk_owner(struct gen_pool *genpool, + struct gen_pool_chunk *chunk, + void *not_used) +{ + struct dmabuf_genpool_chunk_owner *owner = chunk->owner; + + kvfree(owner->niovs); + kfree(owner); +} + +void __net_devmem_dmabuf_binding_free(struct net_devmem_dmabuf_binding *binding) +{ + size_t size, avail; + + gen_pool_for_each_chunk(binding->chunk_pool, + net_devmem_dmabuf_free_chunk_owner, NULL); + + size = gen_pool_size(binding->chunk_pool); + avail = gen_pool_avail(binding->chunk_pool); + + if (!WARN(size != avail, "can't destroy genpool. size=%zu, avail=%zu", + size, avail)) + gen_pool_destroy(binding->chunk_pool); + + dma_buf_unmap_attachment(binding->attachment, binding->sgt, + DMA_FROM_DEVICE); + dma_buf_detach(binding->dmabuf, binding->attachment); + dma_buf_put(binding->dmabuf); + xa_destroy(&binding->bound_rxq_list); + kfree(binding); +} + +/* Protected by rtnl_lock() */ +static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1); + +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_priv, NULL); + + rxq_idx = get_netdev_rx_queue_index(rxq); + + 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->num_rx_queues) + return -ERANGE; + + rxq = __netif_get_rx_queue(dev, rxq_idx); + if (rxq->mp_params.mp_priv) + return -EEXIST; + + err = xa_alloc(&binding->bound_rxq_list, &xa_idx, rxq, xa_limit_32b, + GFP_KERNEL); + if (err) + return err; + + /* 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 driver may read this config while it's creating its * rx-queues. + * WRITE_ONCE() here to match the READ_ONCE() in the driver. + */ + WRITE_ONCE(rxq->mp_params.mp_priv, binding); + + err = netdev_rx_queue_restart(dev, rxq_idx); + if (err) + goto err_xa_erase; + + return 0; + +err_xa_erase: + WRITE_ONCE(rxq->mp_params.mp_priv, NULL); + xa_erase(&binding->bound_rxq_list, xa_idx); + + return err; +} + +int net_devmem_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd, + struct net_devmem_dmabuf_binding **out) +{ + struct net_devmem_dmabuf_binding *binding; + static u32 id_alloc_next; + struct scatterlist *sg; + struct dma_buf *dmabuf; + unsigned int sg_idx, i; + unsigned long virtual; + int err; + + dmabuf = dma_buf_get(dmabuf_fd); + if (IS_ERR(dmabuf)) + return -EBADFD; + + binding = kzalloc_node(sizeof(*binding), GFP_KERNEL, + dev_to_node(&dev->dev)); + if (!binding) { + err = -ENOMEM; + goto err_put_dmabuf; + } + + binding->dev = dev; + + err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id, + binding, xa_limit_32b, &id_alloc_next, + GFP_KERNEL); + if (err < 0) + goto err_free_binding; + + xa_init_flags(&binding->bound_rxq_list, XA_FLAGS_ALLOC); + + refcount_set(&binding->ref, 1); + + binding->dmabuf = dmabuf; + + binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent); + if (IS_ERR(binding->attachment)) { + err = PTR_ERR(binding->attachment); + goto err_free_id; + } + + binding->sgt = + dma_buf_map_attachment(binding->attachment, DMA_FROM_DEVICE); + if (IS_ERR(binding->sgt)) { + err = PTR_ERR(binding->sgt); + goto err_detach; + } + + /* For simplicity we expect to make PAGE_SIZE allocations, but the + * binding can be much more flexible than that. We may be able to + * allocate MTU sized chunks here. Leave that for future work... + */ + binding->chunk_pool = + gen_pool_create(PAGE_SHIFT, dev_to_node(&dev->dev)); + if (!binding->chunk_pool) { + err = -ENOMEM; + goto err_unmap; + } + + virtual = 0; + for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) { + dma_addr_t dma_addr = sg_dma_address(sg); + struct dmabuf_genpool_chunk_owner *owner; + size_t len = sg_dma_len(sg); + struct net_iov *niov; + + owner = kzalloc_node(sizeof(*owner), GFP_KERNEL, + dev_to_node(&dev->dev)); + owner->base_virtual = virtual; + owner->base_dma_addr = dma_addr; + owner->num_niovs = len / PAGE_SIZE; + owner->binding = binding; + + err = gen_pool_add_owner(binding->chunk_pool, dma_addr, + dma_addr, len, dev_to_node(&dev->dev), + owner); + if (err) { + err = -EINVAL; + goto err_free_chunks; + } + + owner->niovs = kvmalloc_array(owner->num_niovs, + sizeof(*owner->niovs), + GFP_KERNEL); + if (!owner->niovs) { + err = -ENOMEM; + goto err_free_chunks; + } + + for (i = 0; i < owner->num_niovs; i++) { + niov = &owner->niovs[i]; + niov->owner = owner; + } + + virtual += len; + } + + *out = binding; + + return 0; + +err_free_chunks: + gen_pool_for_each_chunk(binding->chunk_pool, + net_devmem_dmabuf_free_chunk_owner, NULL); + gen_pool_destroy(binding->chunk_pool); +err_unmap: + dma_buf_unmap_attachment(binding->attachment, binding->sgt, + DMA_FROM_DEVICE); +err_detach: + dma_buf_detach(dmabuf, binding->attachment); +err_free_id: + xa_erase(&net_devmem_dmabuf_bindings, binding->id); +err_free_binding: + kfree(binding); +err_put_dmabuf: + dma_buf_put(dmabuf); + return err; +} +#endif diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c index 9acd0d893765a..3dcd25049e593 100644 --- a/net/core/netdev-genl-gen.c +++ b/net/core/netdev-genl-gen.c @@ -9,6 +9,7 @@ #include "netdev-genl-gen.h" #include <uapi/linux/netdev.h> +#include <linux/list.h> /* Integer value ranges */ static const struct netlink_range_validation netdev_a_page_pool_id_range = { @@ -187,4 +188,7 @@ struct genl_family netdev_nl_family __ro_after_init = { .n_split_ops = ARRAY_SIZE(netdev_nl_ops), .mcgrps = netdev_nl_mcgrps, .n_mcgrps = ARRAY_SIZE(netdev_nl_mcgrps), + .sock_priv_size = sizeof(struct list_head), + .sock_priv_init = (void *)netdev_nl_sock_priv_init, + .sock_priv_destroy = (void *)netdev_nl_sock_priv_destroy, }; diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h index ca5a0983f2834..2c431b7dcbc84 100644 --- a/net/core/netdev-genl-gen.h +++ b/net/core/netdev-genl-gen.h @@ -10,6 +10,7 @@ #include <net/genetlink.h> #include <uapi/linux/netdev.h> +#include <linux/list.h> /* Common nested types */ extern const struct nla_policy netdev_page_pool_info_nl_policy[NETDEV_A_PAGE_POOL_IFINDEX + 1]; @@ -40,4 +41,7 @@ enum { extern struct genl_family netdev_nl_family; +void netdev_nl_sock_priv_init(struct list_head *priv); +void netdev_nl_sock_priv_destroy(struct list_head *priv); + #endif /* _LINUX_NETDEV_GEN_H */ diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index 2d726e65211dd..133884eb13349 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 "netdev-genl-gen.h" #include "dev.h" @@ -721,10 +722,92 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb, return err; } -/* Stub */ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info) { - return 0; + struct nlattr *tb[ARRAY_SIZE(netdev_queue_dmabuf_nl_policy)]; + struct net_devmem_dmabuf_binding *out_binding; + struct list_head *sock_binding_list; + u32 ifindex, dmabuf_fd, rxq_idx; + struct net_device *netdev; + struct sk_buff *rsp; + struct nlattr *attr; + int rem, err = 0; + void *hdr; + + if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_DEV_IFINDEX) || + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_DMABUF_FD) || + GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_QUEUES)) + return -EINVAL; + + ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]); + dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_BIND_DMABUF_DMABUF_FD]); + + rtnl_lock(); + + netdev = __dev_get_by_index(genl_info_net(info), ifindex); + if (!netdev) { + err = -ENODEV; + goto err_unlock; + } + + err = net_devmem_bind_dmabuf(netdev, dmabuf_fd, &out_binding); + if (err) + goto err_unlock; + + nla_for_each_attr(attr, genlmsg_data(info->genlhdr), + genlmsg_len(info->genlhdr), rem) { + if (nla_type(attr) != NETDEV_A_BIND_DMABUF_QUEUES) + continue; + + err = nla_parse_nested( + tb, ARRAY_SIZE(netdev_queue_dmabuf_nl_policy) - 1, attr, + netdev_queue_dmabuf_nl_policy, info->extack); + if (err < 0) + goto err_unbind; + + rxq_idx = nla_get_u32(tb[NETDEV_A_QUEUE_DMABUF_IDX]); + + err = net_devmem_bind_dmabuf_to_queue(netdev, rxq_idx, + out_binding); + if (err) + goto err_unbind; + } + + sock_binding_list = genl_sk_priv_get(&netdev_nl_family, + NETLINK_CB(skb).sk); + if (IS_ERR(sock_binding_list)) { + err = PTR_ERR(sock_binding_list); + goto err_unbind; + } + + list_add(&out_binding->list, sock_binding_list); + + rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL); + if (!rsp) { + err = -ENOMEM; + goto err_unbind; + } + + hdr = genlmsg_iput(rsp, info); + if (!hdr) { + err = -EMSGSIZE; + goto err_genlmsg_free; + } + + nla_put_u32(rsp, NETDEV_A_BIND_DMABUF_DMABUF_ID, out_binding->id); + genlmsg_end(rsp, hdr); + + rtnl_unlock(); + + return genlmsg_reply(rsp, info); + +err_genlmsg_free: + nlmsg_free(rsp); +err_unbind: + net_devmem_unbind_dmabuf(out_binding); +err_unlock: + rtnl_unlock(); + return err; } static int netdev_genl_netdevice_event(struct notifier_block *nb, @@ -771,3 +854,17 @@ static int __init netdev_genl_init(void) } subsys_initcall(netdev_genl_init); + +void netdev_nl_sock_priv_init(struct list_head *priv) +{ + INIT_LIST_HEAD(priv); +} + +void netdev_nl_sock_priv_destroy(struct list_head *priv) +{ + struct net_devmem_dmabuf_binding *binding; + struct net_devmem_dmabuf_binding *temp; + + list_for_each_entry_safe(binding, temp, priv, list) + net_devmem_unbind_dmabuf(binding); +}