Skip to content

Commit

Permalink
net: move mp dev config validation to __net_mp_open_rxq()
Browse files Browse the repository at this point in the history
devmem code performs a number of safety checks to avoid having
to reimplement all of them in the drivers. Move those to
__net_mp_open_rxq() and reuse that function for binding to make
sure that io_uring ZC also benefits from them.

While at it rename the queue ID variable to rxq_idx in
__net_mp_open_rxq(), we touch most of the relevant lines.

The XArray insertion is reordered after the netdev_rx_queue_restart()
call, otherwise we'd need to duplicate the queue index check
or risk inserting an invalid pointer. The XArray allocation
failures should be extremely rare.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Fixes: 6e18ed9 ("net: add helpers for setting a memory provider on an rx queue")
Link: https://patch.msgid.link/20250403013405.2827250-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Apr 4, 2025
1 parent 053f3ff commit ec304b7
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 57 deletions.
6 changes: 6 additions & 0 deletions include/net/page_pool/memory_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <net/page_pool/types.h>

struct netdev_rx_queue;
struct netlink_ext_ack;
struct sk_buff;

struct memory_provider_ops {
Expand All @@ -24,8 +25,13 @@ void net_mp_niov_clear_page_pool(struct net_iov *niov);

int net_mp_open_rxq(struct net_device *dev, unsigned ifq_idx,
struct pp_memory_provider_params *p);
int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
const struct pp_memory_provider_params *p,
struct netlink_ext_ack *extack);
void net_mp_close_rxq(struct net_device *dev, unsigned ifq_idx,
struct pp_memory_provider_params *old_p);
void __net_mp_close_rxq(struct net_device *dev, unsigned int rxq_idx,
const struct pp_memory_provider_params *old_p);

/**
* net_mp_netmem_place_in_cache() - give a netmem to a page pool
Expand Down
50 changes: 10 additions & 40 deletions net/core/devmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/

#include <linux/dma-buf.h>
#include <linux/ethtool_netlink.h>
#include <linux/genalloc.h>
#include <linux/mm.h>
#include <linux/netdevice.h>
Expand Down Expand Up @@ -143,57 +142,28 @@ int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
struct net_devmem_dmabuf_binding *binding,
struct netlink_ext_ack *extack)
{
struct pp_memory_provider_params mp_params = {
.mp_priv = binding,
.mp_ops = &dmabuf_devmem_ops,
};
struct netdev_rx_queue *rxq;
u32 xa_idx;
int err;

if (rxq_idx >= dev->real_num_rx_queues) {
NL_SET_ERR_MSG(extack, "rx queue index out of range");
return -ERANGE;
}

if (dev->cfg->hds_config != ETHTOOL_TCP_DATA_SPLIT_ENABLED) {
NL_SET_ERR_MSG(extack, "tcp-data-split is disabled");
return -EINVAL;
}

if (dev->cfg->hds_thresh) {
NL_SET_ERR_MSG(extack, "hds-thresh is not zero");
return -EINVAL;
}
err = __net_mp_open_rxq(dev, rxq_idx, &mp_params, extack);
if (err)
return err;

rxq = __netif_get_rx_queue(dev, rxq_idx);
if (rxq->mp_params.mp_ops) {
NL_SET_ERR_MSG(extack, "designated queue already memory provider bound");
return -EEXIST;
}

#ifdef CONFIG_XDP_SOCKETS
if (rxq->pool) {
NL_SET_ERR_MSG(extack, "designated queue already in use by AF_XDP");
return -EBUSY;
}
#endif

err = xa_alloc(&binding->bound_rxqs, &xa_idx, rxq, xa_limit_32b,
GFP_KERNEL);
if (err)
return err;

rxq->mp_params.mp_priv = binding;
rxq->mp_params.mp_ops = &dmabuf_devmem_ops;

err = netdev_rx_queue_restart(dev, rxq_idx);
if (err)
goto err_xa_erase;
goto err_close_rxq;

return 0;

err_xa_erase:
rxq->mp_params.mp_priv = NULL;
rxq->mp_params.mp_ops = NULL;
xa_erase(&binding->bound_rxqs, xa_idx);

err_close_rxq:
__net_mp_close_rxq(dev, rxq_idx, &mp_params);
return err;
}

Expand Down
6 changes: 0 additions & 6 deletions net/core/netdev-genl.c
Original file line number Diff line number Diff line change
Expand Up @@ -874,12 +874,6 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
goto err_unlock;
}

if (dev_xdp_prog_count(netdev)) {
NL_SET_ERR_MSG(info->extack, "unable to bind dmabuf to device with XDP program attached");
err = -EEXIST;
goto err_unlock;
}

binding = net_devmem_bind_dmabuf(netdev, dmabuf_fd, info->extack);
if (IS_ERR(binding)) {
err = PTR_ERR(binding);
Expand Down
49 changes: 38 additions & 11 deletions net/core/netdev_rx_queue.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later

#include <linux/ethtool_netlink.h>
#include <linux/netdevice.h>
#include <net/netdev_lock.h>
#include <net/netdev_queues.h>
Expand Down Expand Up @@ -86,45 +87,71 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
}
EXPORT_SYMBOL_NS_GPL(netdev_rx_queue_restart, "NETDEV_INTERNAL");

static int __net_mp_open_rxq(struct net_device *dev, unsigned ifq_idx,
struct pp_memory_provider_params *p)
int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
const struct pp_memory_provider_params *p,
struct netlink_ext_ack *extack)
{
struct netdev_rx_queue *rxq;
int ret;

if (!netdev_need_ops_lock(dev))
return -EOPNOTSUPP;

if (ifq_idx >= dev->real_num_rx_queues)
if (rxq_idx >= dev->real_num_rx_queues)
return -EINVAL;
ifq_idx = array_index_nospec(ifq_idx, dev->real_num_rx_queues);
rxq_idx = array_index_nospec(rxq_idx, dev->real_num_rx_queues);

rxq = __netif_get_rx_queue(dev, ifq_idx);
if (rxq->mp_params.mp_ops)
if (rxq_idx >= dev->real_num_rx_queues) {
NL_SET_ERR_MSG(extack, "rx queue index out of range");
return -ERANGE;
}
if (dev->cfg->hds_config != ETHTOOL_TCP_DATA_SPLIT_ENABLED) {
NL_SET_ERR_MSG(extack, "tcp-data-split is disabled");
return -EINVAL;
}
if (dev->cfg->hds_thresh) {
NL_SET_ERR_MSG(extack, "hds-thresh is not zero");
return -EINVAL;
}
if (dev_xdp_prog_count(dev)) {
NL_SET_ERR_MSG(extack, "unable to custom memory provider to device with XDP program attached");
return -EEXIST;
}

rxq = __netif_get_rx_queue(dev, rxq_idx);
if (rxq->mp_params.mp_ops) {
NL_SET_ERR_MSG(extack, "designated queue already memory provider bound");
return -EEXIST;
}
#ifdef CONFIG_XDP_SOCKETS
if (rxq->pool) {
NL_SET_ERR_MSG(extack, "designated queue already in use by AF_XDP");
return -EBUSY;
}
#endif

rxq->mp_params = *p;
ret = netdev_rx_queue_restart(dev, ifq_idx);
ret = netdev_rx_queue_restart(dev, rxq_idx);
if (ret) {
rxq->mp_params.mp_ops = NULL;
rxq->mp_params.mp_priv = NULL;
}
return ret;
}

int net_mp_open_rxq(struct net_device *dev, unsigned ifq_idx,
int net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
struct pp_memory_provider_params *p)
{
int ret;

netdev_lock(dev);
ret = __net_mp_open_rxq(dev, ifq_idx, p);
ret = __net_mp_open_rxq(dev, rxq_idx, p, NULL);
netdev_unlock(dev);
return ret;
}

static void __net_mp_close_rxq(struct net_device *dev, unsigned ifq_idx,
struct pp_memory_provider_params *old_p)
void __net_mp_close_rxq(struct net_device *dev, unsigned int ifq_idx,
const struct pp_memory_provider_params *old_p)
{
struct netdev_rx_queue *rxq;

Expand Down

0 comments on commit ec304b7

Please sign in to comment.