mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-10-26 10:57:38 +08:00
Commitb6f74dff6d("net: use two lockdep classes for the netdev instance lock") put every device without a parent in the virtual class. It also restricted the locking order for the virtual class, because queue leasing has hard requirements on the exact order. This bites us back on bond, which is "virtual" and needs to be taken before taking the locks of the lowers. NIPA hit the following on the new test I recently posted for XDP+bond: WARNING: possible circular locking dependency detected ------------------------------------------------------ python3/18635 is trying to acquire lock: ff11000120b8ce30 (&dev->lock){+.+.}-{4:4}, at: netdev_put_lock+0x2d/0x1a0 but task is already holding lock: ff110001ef77ae30 (&netdev_virt_instance_lock_key){+.+.}-{4:4}, at: netdev_put_lock+0x2d/0x1a0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&netdev_virt_instance_lock_key){+.+.}-{4:4}: __mutex_lock+0x1ae/0x1f10 xdp_set_features_flag+0x2b/0x50 bond_xdp_set_features+0x1eb/0x360 bond_netdev_event+0x13f/0x300 notifier_call_chain+0xae/0x300 call_netdevice_notifiers+0x70/0xa0 bnxt_xdp_set+0x2f6/0x620 netif_xdp_propagate+0x503/0xc60 dev_xdp_propagate+0xa1/0x230 bond_xdp_set+0x234/0x700 dev_xdp_install+0x592/0xd70 dev_xdp_attach+0x355/0xf50 dev_change_xdp_fd+0x176/0x210 do_setlink.isra.0+0x220d/0x2b20 rtnl_newlink+0x9f1/0x11b0 -> #0 (&dev->lock){+.+.}-{4:4}: __mutex_lock+0x1ae/0x1f10 netdev_put_lock+0x2d/0x1a0 netdev_nl_queue_create_doit+0x801/0x1a70 genl_family_rcv_msg_doit+0x206/0x300 Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&netdev_virt_instance_lock_key); lock(&dev->lock); lock(&netdev_virt_instance_lock_key); lock(&dev->lock); Let's narrow down the "virtual" class to only the devices which can actually create a queue. More LoC and complexity, but that is what we actually care about here. The rest needs to nest under rtnl_lock, which bond does (famous last words?) Take the instance locks in two passes, first the netkits then the rest (matching the queue leasing order). An alternative would be to make sure the close list is sorted correctly from the start (queue head/tail appropriately in unregister_netdevice_queue()). I think it works but feels a little more fragile. Happy to change, tho. netdev_can_create_queue() will now be used on paths where we genuinely handle non-netkit, so we can't always set the extack. Unfortunately, the (recently) added tracepoint in extack fires even when extack is NULL. Fixes:b6f74dff6d("net: use two lockdep classes for the netdev instance lock") Signed-off-by: Jakub Kicinski <kuba@kernel.org> Acked-by: Stanislav Fomichev <sdf@fomichev.me> Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org> Acked-by: Daniel Borkmann <daniel@iogearbox.net> Link: https://patch.msgid.link/20260929191543.3295633-1-kuba@kernel.org Signed-off-by: Paolo Abeni <pabeni@redhat.com>
122 lines
3.3 KiB
C
122 lines
3.3 KiB
C
// 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>
|
|
|
|
#include "dev.h"
|
|
|
|
static struct device *
|
|
__netdev_queue_get_dma_dev(struct net_device *dev, unsigned int idx)
|
|
{
|
|
const struct netdev_queue_mgmt_ops *queue_ops = dev->queue_mgmt_ops;
|
|
struct device *dma_dev;
|
|
|
|
if (queue_ops && queue_ops->ndo_queue_get_dma_dev)
|
|
dma_dev = queue_ops->ndo_queue_get_dma_dev(dev, idx);
|
|
else
|
|
dma_dev = dev->dev.parent;
|
|
|
|
return dma_dev && dma_dev->dma_mask ? dma_dev : NULL;
|
|
}
|
|
|
|
/**
|
|
* netdev_queue_get_dma_dev() - get dma device for zero-copy operations
|
|
* @dev: net_device
|
|
* @idx: queue index
|
|
* @type: queue type (RX or TX)
|
|
*
|
|
* Get dma device for zero-copy operations to be used for this queue. If
|
|
* the queue is an RX queue leased from a physical queue, we retrieve the
|
|
* physical queue's dma device. When the dma device is not available or
|
|
* valid, the function will return NULL.
|
|
*
|
|
* Return: Device or NULL on error
|
|
*/
|
|
struct device *netdev_queue_get_dma_dev(struct net_device *dev,
|
|
unsigned int idx,
|
|
enum netdev_queue_type type)
|
|
{
|
|
struct netdev_rx_queue *hw_rxq;
|
|
struct device *dma_dev;
|
|
|
|
netdev_assert_locked(dev);
|
|
|
|
/* Only RX side supports queue leasing today. */
|
|
if (type != NETDEV_QUEUE_TYPE_RX || !netif_rxq_is_leased(dev, idx))
|
|
return __netdev_queue_get_dma_dev(dev, idx);
|
|
if (!netif_is_queue_leasee(dev))
|
|
return NULL;
|
|
|
|
hw_rxq = __netif_get_rx_queue(dev, idx)->lease;
|
|
|
|
netdev_lock(hw_rxq->dev);
|
|
idx = get_netdev_rx_queue_index(hw_rxq);
|
|
dma_dev = __netdev_queue_get_dma_dev(hw_rxq->dev, idx);
|
|
netdev_unlock(hw_rxq->dev);
|
|
|
|
return dma_dev;
|
|
}
|
|
|
|
bool netdev_can_create_queue(const struct net_device *dev,
|
|
struct netlink_ext_ack *extack)
|
|
{
|
|
const char *msg = NULL;
|
|
|
|
if (dev->real_num_rx_queues < 1 ||
|
|
dev->real_num_tx_queues < 1)
|
|
msg = "Device must have at least one real queue";
|
|
if (!dev->queue_mgmt_ops ||
|
|
!dev->queue_mgmt_ops->ndo_queue_create)
|
|
msg = "Device does not support queue creation";
|
|
if (dev->dev.parent)
|
|
msg = "Device is not a virtual device";
|
|
|
|
/* callers with extack=NULL are not from uAPI and don't want to trigger
|
|
* the extack tracepoint.
|
|
*/
|
|
if (msg && extack)
|
|
NL_SET_ERR_MSG_FMT(extack, "%s", msg);
|
|
|
|
return !msg;
|
|
}
|
|
|
|
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, unsigned int idx,
|
|
enum netdev_queue_type type,
|
|
struct netlink_ext_ack *extack)
|
|
{
|
|
if (xsk_get_pool_from_qid(dev, idx)) {
|
|
NL_SET_ERR_MSG(extack, "Device queue in use by AF_XDP");
|
|
return true;
|
|
}
|
|
if (type == NETDEV_QUEUE_TYPE_TX)
|
|
return false;
|
|
if (netif_rxq_is_leased(dev, idx)) {
|
|
NL_SET_ERR_MSG(extack, "Device queue in use due to queue leasing");
|
|
return true;
|
|
}
|
|
if (netif_rxq_has_mp(dev, idx)) {
|
|
NL_SET_ERR_MSG(extack, "Device queue in use by memory provider");
|
|
return true;
|
|
}
|
|
return false;
|
|
}
|