mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-27 19:53:15 -04:00
qede: sync udp_tunnel ports outside qede_lock in the recovery path
A TX timeout on a qede NIC that has VXLAN/GENEVE tunnel ports
configured wedges the rtnetlink control plane of the whole machine:
NETDEV WATCHDOG: ens6f1 (qede): transmit queue 2 timed out 10226 ms
[qede_tx_timeout:586(ens6f1)]TX timeout on queue 2!
[qede_recovery_handler:2665(ens6f0)]Starting a recovery process
The recovery path deadlocks on the driver's own mutex:
qede_sp_task
rtnl_lock()
mutex_lock(&edev->qede_lock) <- taken
qede_recovery_handler
qede_load
udp_tunnel_nic_reset_ntf
__udp_tunnel_nic_device_sync
info->sync_table == qede_udp_tunnel_sync
mutex_lock(&edev->qede_lock) <- same task: deadlock
The mutex is not recursive, so the kworker blocks on itself with
rtnl_lock held, and neither lock is ever released. Every task that
calls rtnl_lock() afterwards (ip, ovs-vswitchd, lldpad, IPv6
addrconf, sshd) blocks forever while the node still answers ping.
In a vmcore from an affected production node rtnl_mutex.owner
decodes to the very kworker blocked at the innermost mutex_lock()
above.
Re-sync the tunnel ports from qede_sp_task() after the internal lock
is dropped, still under rtnl_lock as the udp_tunnel API requires.
This mirrors qede_open(), which calls udp_tunnel_nic_reset_ntf()
under rtnl without the internal lock.
qede_recovery_handler() now returns whether it has successfully
reloaded an open device, and the caller re-syncs the ports only in
that case. This keeps the old gating exactly: a device that was down
or a failed recovery returns false, as those paths never reached the
udp_tunnel_nic_reset_ntf() call before either.
This was the only user of the qede_lock()/qede_unlock() helpers, so
remove them.
Fixes: 8cd160a294 ("qede: convert to new udp_tunnel_nic infra")
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Andrew Lunn <andrew+netdev@lunn.ch>
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260726104311.1782900-1-den@openvz.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
committed by
Paolo Abeni
parent
c92922c02c
commit
451c9075d6
@@ -107,7 +107,7 @@ static void qede_remove(struct pci_dev *pdev);
|
||||
static void qede_shutdown(struct pci_dev *pdev);
|
||||
static void qede_link_update(void *dev, struct qed_link_output *link);
|
||||
static void qede_schedule_recovery_handler(void *dev);
|
||||
static void qede_recovery_handler(struct qede_dev *edev);
|
||||
static bool qede_recovery_handler(struct qede_dev *edev);
|
||||
static void qede_schedule_hw_err_handler(void *dev,
|
||||
enum qed_hw_err_type err_type);
|
||||
static void qede_get_eth_tlv_data(void *edev, void *data);
|
||||
@@ -1043,21 +1043,6 @@ void __qede_unlock(struct qede_dev *edev)
|
||||
mutex_unlock(&edev->qede_lock);
|
||||
}
|
||||
|
||||
/* This version of the lock should be used when acquiring the RTNL lock is also
|
||||
* needed in addition to the internal qede lock.
|
||||
*/
|
||||
static void qede_lock(struct qede_dev *edev)
|
||||
{
|
||||
rtnl_lock();
|
||||
__qede_lock(edev);
|
||||
}
|
||||
|
||||
static void qede_unlock(struct qede_dev *edev)
|
||||
{
|
||||
__qede_unlock(edev);
|
||||
rtnl_unlock();
|
||||
}
|
||||
|
||||
static void qede_periodic_task(struct work_struct *work)
|
||||
{
|
||||
struct qede_dev *edev = container_of(work, struct qede_dev,
|
||||
@@ -1094,6 +1079,8 @@ static void qede_sp_task(struct work_struct *work)
|
||||
*/
|
||||
|
||||
if (test_and_clear_bit(QEDE_SP_RECOVERY, &edev->sp_flags)) {
|
||||
bool reloaded;
|
||||
|
||||
cancel_delayed_work_sync(&edev->periodic_task);
|
||||
#ifdef CONFIG_QED_SRIOV
|
||||
/* SRIOV must be disabled outside the lock to avoid a deadlock.
|
||||
@@ -1102,9 +1089,17 @@ static void qede_sp_task(struct work_struct *work)
|
||||
if (pci_num_vf(edev->pdev))
|
||||
qede_sriov_configure(edev->pdev, 0);
|
||||
#endif
|
||||
qede_lock(edev);
|
||||
qede_recovery_handler(edev);
|
||||
qede_unlock(edev);
|
||||
rtnl_lock();
|
||||
__qede_lock(edev);
|
||||
reloaded = qede_recovery_handler(edev);
|
||||
__qede_unlock(edev);
|
||||
|
||||
/* The udp_tunnel core synchronously calls back into
|
||||
* qede_udp_tunnel_sync(), which takes the qede lock.
|
||||
*/
|
||||
if (reloaded)
|
||||
udp_tunnel_nic_reset_ntf(edev->ndev);
|
||||
rtnl_unlock();
|
||||
}
|
||||
|
||||
__qede_lock(edev);
|
||||
@@ -2645,9 +2640,13 @@ static void qede_recovery_failed(struct qede_dev *edev)
|
||||
edev->ops->common->set_power_state(edev->cdev, PCI_D3hot);
|
||||
}
|
||||
|
||||
static void qede_recovery_handler(struct qede_dev *edev)
|
||||
/* Returns true if an open device was successfully reloaded and its
|
||||
* udp_tunnel ports need to be re-synced by the caller.
|
||||
*/
|
||||
static bool qede_recovery_handler(struct qede_dev *edev)
|
||||
{
|
||||
u32 curr_state = edev->state;
|
||||
bool reloaded = false;
|
||||
int rc;
|
||||
|
||||
DP_NOTICE(edev, "Starting a recovery process\n");
|
||||
@@ -2677,17 +2676,18 @@ static void qede_recovery_handler(struct qede_dev *edev)
|
||||
goto err;
|
||||
|
||||
qede_config_rx_mode(edev->ndev);
|
||||
udp_tunnel_nic_reset_ntf(edev->ndev);
|
||||
reloaded = true;
|
||||
}
|
||||
|
||||
edev->state = curr_state;
|
||||
|
||||
DP_NOTICE(edev, "Recovery handling is done\n");
|
||||
|
||||
return;
|
||||
return reloaded;
|
||||
|
||||
err:
|
||||
qede_recovery_failed(edev);
|
||||
return false;
|
||||
}
|
||||
|
||||
static void qede_atomic_hw_err_handler(struct qede_dev *edev)
|
||||
|
||||
Reference in New Issue
Block a user