| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aWQOO0xhXxNebmXF@mini-arch>
Date: Sun, 11 Jan 2026 12:55:23 -0800
From: Stanislav Fomichev <stfomichev@...il.com>
To: Daniel Borkmann <daniel@...earbox.net>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org, kuba@...nel.org,
davem@...emloft.net, razor@...ckwall.org, pabeni@...hat.com,
willemb@...gle.com, sdf@...ichev.me, john.fastabend@...il.com,
martin.lau@...nel.org, jordan@...fe.io,
maciej.fijalkowski@...el.com, magnus.karlsson@...el.com,
dw@...idwei.uk, toke@...hat.com, yangzhenze@...edance.com,
wangdongdong.6@...edance.com
Subject: Re: [PATCH net-next v5 02/16] net: Implement
netdev_nl_queue_create_doit
On 01/09, Daniel Borkmann wrote:
> Implement netdev_nl_queue_create_doit which creates a new rx queue in a
> virtual netdev and then leases it to a rx queue in a physical netdev.
>
> Example with ynl client:
>
> # ./pyynl/cli.py \
> --spec ~/netlink/specs/netdev.yaml \
> --do queue-create \
> --json '{"ifindex": 8, "type": "rx", "lease": {"ifindex": 4, "queue": {"type": "rx", "id": 15}}}'
> {'id': 1}
>
> Note that the netdevice locking order is always from the virtual to
> the physical device.
>
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> Co-developed-by: David Wei <dw@...idwei.uk>
> Signed-off-by: David Wei <dw@...idwei.uk>
> ---
> include/net/netdev_queues.h | 19 ++++-
> include/net/netdev_rx_queue.h | 9 ++-
> include/net/xdp_sock_drv.h | 2 +-
> net/core/dev.c | 7 ++
> net/core/dev.h | 2 +
> net/core/netdev-genl.c | 146 +++++++++++++++++++++++++++++++++-
> net/core/netdev_queues.c | 57 +++++++++++++
> net/core/netdev_rx_queue.c | 50 +++++++++++-
> net/xdp/xsk.c | 2 +-
> 9 files changed, 282 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
> index cd00e0406cf4..f65319a6fb87 100644
> --- a/include/net/netdev_queues.h
> +++ b/include/net/netdev_queues.h
> @@ -130,6 +130,11 @@ void netdev_stat_queue_sum(struct net_device *netdev,
> * @ndo_queue_get_dma_dev: Get dma device for zero-copy operations to be used
> * for this queue. Return NULL on error.
> *
> + * @ndo_queue_create: Create a new RX queue which can be leased to another queue.
> + * Ops on this queue are redirected to the leased queue e.g.
> + * when opening a memory provider. Return the new queue id on
> + * success. Return negative error code on failure.
> + *
> * Note that @ndo_queue_mem_alloc and @ndo_queue_mem_free may be called while
> * the interface is closed. @ndo_queue_start and @ndo_queue_stop will only
> * be called for an interface which is open.
> @@ -149,9 +154,12 @@ struct netdev_queue_mgmt_ops {
> int idx);
> struct device * (*ndo_queue_get_dma_dev)(struct net_device *dev,
> int idx);
> + int (*ndo_queue_create)(struct net_device *dev);
> };
>
> -bool netif_rxq_has_unreadable_mp(struct net_device *dev, int idx);
> +bool netif_rxq_has_unreadable_mp(struct net_device *dev, unsigned int rxq_idx);
> +bool netif_rxq_has_mp(struct net_device *dev, unsigned int rxq_idx);
> +bool netif_rxq_is_leased(struct net_device *dev, unsigned int rxq_idx);
>
> /**
> * DOC: Lockless queue stopping / waking helpers.
> @@ -329,5 +337,10 @@ static inline void netif_subqueue_sent(const struct net_device *dev,
> })
>
> struct device *netdev_queue_get_dma_dev(struct net_device *dev, int idx);
> -
> -#endif
> +bool netdev_can_create_queue(const struct net_device *dev,
> + struct netlink_ext_ack *extack);
> +bool netdev_can_lease_queue(const struct net_device *dev,
> + struct netlink_ext_ack *extack);
> +bool netdev_queue_busy(struct net_device *dev, int idx,
> + struct netlink_ext_ack *extack);
> +#endif /* _LINUX_NET_QUEUES_H */
> diff --git a/include/net/netdev_rx_queue.h b/include/net/netdev_rx_queue.h
> index 8cdcd138b33f..1cacc2451516 100644
> --- a/include/net/netdev_rx_queue.h
> +++ b/include/net/netdev_rx_queue.h
> @@ -28,6 +28,8 @@ struct netdev_rx_queue {
> #endif
> struct napi_struct *napi;
> struct pp_memory_provider_params mp_params;
> + struct netdev_rx_queue *lease;
> + netdevice_tracker lease_tracker;
> } ____cacheline_aligned_in_smp;
>
> /*
> @@ -57,5 +59,8 @@ get_netdev_rx_queue_index(struct netdev_rx_queue *queue)
> }
>
> int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq);
> -
> -#endif
> +void netdev_rx_queue_lease(struct netdev_rx_queue *rxq_dst,
> + struct netdev_rx_queue *rxq_src);
> +void netdev_rx_queue_unlease(struct netdev_rx_queue *rxq_dst,
> + struct netdev_rx_queue *rxq_src);
> +#endif /* _LINUX_NETDEV_RX_QUEUE_H */
> diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
> index 242e34f771cc..c07cfb431eac 100644
> --- a/include/net/xdp_sock_drv.h
> +++ b/include/net/xdp_sock_drv.h
> @@ -28,7 +28,7 @@ void xsk_tx_completed(struct xsk_buff_pool *pool, u32 nb_entries);
> bool xsk_tx_peek_desc(struct xsk_buff_pool *pool, struct xdp_desc *desc);
> u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 max);
> void xsk_tx_release(struct xsk_buff_pool *pool);
> -struct xsk_buff_pool *xsk_get_pool_from_qid(struct net_device *dev,
> +struct xsk_buff_pool *xsk_get_pool_from_qid(const struct net_device *dev,
> u16 queue_id);
> void xsk_set_rx_need_wakeup(struct xsk_buff_pool *pool);
> void xsk_set_tx_need_wakeup(struct xsk_buff_pool *pool);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 36dc5199037e..c2b64ffeb9e6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1102,6 +1102,13 @@ netdev_get_by_index_lock_ops_compat(struct net *net, int ifindex)
> return __netdev_put_lock_ops_compat(dev, net);
> }
>
> +struct net_device *
> +netdev_put_lock(struct net_device *dev, netdevice_tracker *tracker)
> +{
> + netdev_tracker_free(dev, tracker);
> + return __netdev_put_lock(dev, dev_net(dev));
> +}
> +
> struct net_device *
> netdev_xa_find_lock(struct net *net, struct net_device *dev,
> unsigned long *index)
> diff --git a/net/core/dev.h b/net/core/dev.h
> index da18536cbd35..9bcb76b325d0 100644
> --- a/net/core/dev.h
> +++ b/net/core/dev.h
> @@ -30,6 +30,8 @@ netdev_napi_by_id_lock(struct net *net, unsigned int napi_id);
> struct net_device *dev_get_by_napi_id(unsigned int napi_id);
>
> struct net_device *__netdev_put_lock(struct net_device *dev, struct net *net);
> +struct net_device *netdev_put_lock(struct net_device *dev,
> + netdevice_tracker *tracker);
> struct net_device *
> netdev_xa_find_lock(struct net *net, struct net_device *dev,
> unsigned long *index);
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index aae75431858d..cd4dc4eef029 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -1122,7 +1122,151 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
>
> int netdev_nl_queue_create_doit(struct sk_buff *skb, struct genl_info *info)
> {
> - return -EOPNOTSUPP;
> + const int qmaxtype = ARRAY_SIZE(netdev_queue_id_nl_policy) - 1;
> + const int lmaxtype = ARRAY_SIZE(netdev_lease_nl_policy) - 1;
> + int err, ifindex, ifindex_lease, queue_id, queue_id_lease;
> + struct nlattr *qtb[ARRAY_SIZE(netdev_queue_id_nl_policy)];
> + struct nlattr *ltb[ARRAY_SIZE(netdev_lease_nl_policy)];
> + struct netdev_rx_queue *rxq, *rxq_lease;
> + struct net_device *dev, *dev_lease;
> + netdevice_tracker dev_tracker;
> + struct nlattr *nest;
> + struct sk_buff *rsp;
> + void *hdr;
> +
> + if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_LEASE))
> + return -EINVAL;
> + if (nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]) !=
> + NETDEV_QUEUE_TYPE_RX) {
> + NL_SET_BAD_ATTR(info->extack, info->attrs[NETDEV_A_QUEUE_TYPE]);
> + return -EINVAL;
> + }
> +
> + ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
> +
> + nest = info->attrs[NETDEV_A_QUEUE_LEASE];
> + err = nla_parse_nested(ltb, lmaxtype, nest,
> + netdev_lease_nl_policy, info->extack);
> + if (err < 0)
> + return err;
> + if (NL_REQ_ATTR_CHECK(info->extack, nest, ltb, NETDEV_A_LEASE_IFINDEX) ||
> + NL_REQ_ATTR_CHECK(info->extack, nest, ltb, NETDEV_A_LEASE_QUEUE))
> + return -EINVAL;
> + if (ltb[NETDEV_A_LEASE_NETNS_ID]) {
> + NL_SET_BAD_ATTR(info->extack, ltb[NETDEV_A_LEASE_NETNS_ID]);
> + return -EINVAL;
> + }
> +
> + ifindex_lease = nla_get_u32(ltb[NETDEV_A_LEASE_IFINDEX]);
> +
> + nest = ltb[NETDEV_A_LEASE_QUEUE];
> + err = nla_parse_nested(qtb, qmaxtype, nest,
> + netdev_queue_id_nl_policy, info->extack);
> + if (err < 0)
> + return err;
> + if (NL_REQ_ATTR_CHECK(info->extack, nest, qtb, NETDEV_A_QUEUE_ID) ||
> + NL_REQ_ATTR_CHECK(info->extack, nest, qtb, NETDEV_A_QUEUE_TYPE))
> + return -EINVAL;
> + if (nla_get_u32(qtb[NETDEV_A_QUEUE_TYPE]) != NETDEV_QUEUE_TYPE_RX) {
> + NL_SET_BAD_ATTR(info->extack, qtb[NETDEV_A_QUEUE_TYPE]);
> + return -EINVAL;
> + }
> + if (ifindex == ifindex_lease) {
> + NL_SET_ERR_MSG(info->extack,
> + "Lease ifindex cannot be the same as queue creation ifindex");
> + return -EINVAL;
> + }
> +
> + queue_id_lease = nla_get_u32(qtb[NETDEV_A_QUEUE_ID]);
> +
> + rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!rsp)
> + return -ENOMEM;
> +
> + hdr = genlmsg_iput(rsp, info);
> + if (!hdr) {
> + err = -EMSGSIZE;
> + goto err_genlmsg_free;
> + }
> +
> + /* Locking order is always from the virtual to the physical device
> + * since this is also the same order when applications open the
> + * memory provider later on.
> + */
> + dev = netdev_get_by_index_lock(genl_info_net(info), ifindex);
> + if (!dev) {
> + err = -ENODEV;
> + goto err_genlmsg_free;
> + }
> + if (!netdev_can_create_queue(dev, info->extack)) {
> + err = -EINVAL;
> + goto err_unlock_dev;
> + }
> +
> + dev_lease = netdev_get_by_index(genl_info_net(info), ifindex_lease,
> + &dev_tracker, GFP_KERNEL);
> + if (!dev_lease) {
> + err = -ENODEV;
> + goto err_unlock_dev;
> + }
> + if (!netdev_can_lease_queue(dev_lease, info->extack)) {
> + netdev_put(dev_lease, &dev_tracker);
> + err = -EINVAL;
> + goto err_unlock_dev;
> + }
> +
> + dev_lease = netdev_put_lock(dev_lease, &dev_tracker);
> + if (!dev_lease) {
> + err = -ENODEV;
> + goto err_unlock_dev;
> + }
> + if (queue_id_lease >= dev_lease->real_num_rx_queues) {
> + err = -ERANGE;
> + NL_SET_BAD_ATTR(info->extack, qtb[NETDEV_A_QUEUE_ID]);
> + goto err_unlock_dev_lease;
> + }
> + if (netdev_queue_busy(dev_lease, queue_id_lease, info->extack)) {
> + err = -EBUSY;
> + goto err_unlock_dev_lease;
> + }
> +
> + rxq_lease = __netif_get_rx_queue(dev_lease, queue_id_lease);
> + rxq = __netif_get_rx_queue(dev, dev->real_num_rx_queues - 1);
> +
> + if (rxq->lease && rxq->lease->dev != dev_lease) {
> + err = -EOPNOTSUPP;
> + NL_SET_ERR_MSG(info->extack,
> + "Leasing multiple queues from different devices not supported");
> + goto err_unlock_dev_lease;
> + }
> +
> + err = queue_id = dev->queue_mgmt_ops->ndo_queue_create(dev);
> + if (err < 0) {
> + NL_SET_ERR_MSG(info->extack,
> + "Device is unable to create a new queue");
> + goto err_unlock_dev_lease;
> + }
> +
> + rxq = __netif_get_rx_queue(dev, queue_id);
> + netdev_rx_queue_lease(rxq, rxq_lease);
> +
> + nla_put_u32(rsp, NETDEV_A_QUEUE_ID, queue_id);
> + genlmsg_end(rsp, hdr);
> +
> + netdev_unlock(dev_lease);
> + netdev_unlock(dev);
> +
> + return genlmsg_reply(rsp, info);
> +
> +err_unlock_dev_lease:
> + netdev_unlock(dev_lease);
> +err_unlock_dev:
> + netdev_unlock(dev);
> +err_genlmsg_free:
> + nlmsg_free(rsp);
> + return err;
> }
>
> void netdev_nl_sock_priv_init(struct netdev_nl_sock *priv)
> diff --git a/net/core/netdev_queues.c b/net/core/netdev_queues.c
> index 251f27a8307f..fae92ee090c4 100644
> --- a/net/core/netdev_queues.c
> +++ b/net/core/netdev_queues.c
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
>
> #include <net/netdev_queues.h>
> +#include <net/netdev_rx_queue.h>
> +#include <net/xdp_sock_drv.h>
>
> /**
> * netdev_queue_get_dma_dev() - get dma device for zero-copy operations
> @@ -25,3 +27,58 @@ struct device *netdev_queue_get_dma_dev(struct net_device *dev, int idx)
> return dma_dev && dma_dev->dma_mask ? dma_dev : NULL;
> }
>
> +bool netdev_can_create_queue(const struct net_device *dev,
> + struct netlink_ext_ack *extack)
> +{
> + if (dev->dev.parent) {
> + NL_SET_ERR_MSG(extack, "Device is not a virtual device");
> + return false;
> + }
> + if (!dev->queue_mgmt_ops ||
> + !dev->queue_mgmt_ops->ndo_queue_create) {
> + NL_SET_ERR_MSG(extack, "Device does not support queue creation");
> + return false;
> + }
> + if (dev->real_num_rx_queues < 1 ||
> + dev->real_num_tx_queues < 1) {
> + NL_SET_ERR_MSG(extack, "Device must have at least one real queue");
> + return false;
> + }
> + return true;
> +}
> +
> +bool netdev_can_lease_queue(const struct net_device *dev,
> + struct netlink_ext_ack *extack)
> +{
> + if (!dev->dev.parent) {
> + NL_SET_ERR_MSG(extack, "Lease device is a virtual device");
> + return false;
> + }
> + if (!netif_device_present(dev)) {
> + NL_SET_ERR_MSG(extack, "Lease device has been removed from the system");
> + return false;
> + }
> + if (!dev->queue_mgmt_ops) {
> + NL_SET_ERR_MSG(extack, "Lease device does not support queue management operations");
> + return false;
> + }
> + return true;
> +}
> +
> +bool netdev_queue_busy(struct net_device *dev, int idx,
> + struct netlink_ext_ack *extack)
> +{
> + if (netif_rxq_is_leased(dev, idx)) {
> + NL_SET_ERR_MSG(extack, "Lease device queue is already leased");
> + return true;
> + }
> + if (xsk_get_pool_from_qid(dev, idx)) {
> + NL_SET_ERR_MSG(extack, "Lease device queue in use by AF_XDP");
> + return true;
> + }
> + if (netif_rxq_has_mp(dev, idx)) {
> + NL_SET_ERR_MSG(extack, "Lease device queue in use by memory provider");
> + return true;
> + }
> + return false;
> +}
> diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
> index c7d9341b7630..ed85dfb434a0 100644
> --- a/net/core/netdev_rx_queue.c
> +++ b/net/core/netdev_rx_queue.c
> @@ -9,15 +9,57 @@
>
> #include "page_pool_priv.h"
>
> -/* See also page_pool_is_unreadable() */
> -bool netif_rxq_has_unreadable_mp(struct net_device *dev, int idx)
> +void netdev_rx_queue_lease(struct netdev_rx_queue *rxq_dst,
> + struct netdev_rx_queue *rxq_src)
> +{
> + netdev_assert_locked(rxq_src->dev);
> + netdev_assert_locked(rxq_dst->dev);
> +
> + WARN_ON_ONCE(READ_ONCE(rxq_src->dev->reg_state) == NETREG_UNREGISTERING);
I might have missed some of your discussions with Jakub, but what is
this WARN_ON_ONCE above trying to catch? And why not handle it explicitly
(via returning an error from netdev_rx_queue_lease)?
Powered by blists - more mailing lists