Re: [PATCH 1/1] qede: sync udp_tunnel ports outside qede_lock in the recovery path

[Jacob Keller <jacob.e.keller@intel.com>]

On 7/26/2026 3:43 AM, Denis V. Lunev wrote:
> 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: 8cd160a29415 ("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>

>  drivers/net/ethernet/qlogic/qede/qede_main.c | 44 ++++++++++----------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
> index cb0ae0650905..7ed17faced54 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> @@ -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);
> +

This still has rtnl_lock -> __qede_lock(), but since you unlock it
earlier than doing the udp_tunnel_nic_reset_ntf, you don't want to use
the helper anymore. Makes sense.