[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <IA3PR11MB89866E59AAA33A502D596830E565A@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Mon, 9 Feb 2026 07:15:22 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: "Oros, Petr" <poros@...hat.com>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>
CC: "Vecera, Ivan" <ivecera@...hat.com>, "shaojijie@...wei.com"
<shaojijie@...wei.com>, "Oros, Petr" <poros@...hat.com>, "Keller, Jacob E"
<jacob.e.keller@...el.com>, "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
"Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>, Andrew Lunn
<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, "Eric
Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Stanislav Fomichev <sdf@...ichev.me>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net v2] iavf: fix deadlock in reset handling
> -----Original Message-----
> From: Petr Oros <poros@...hat.com>
> Sent: Saturday, February 7, 2026 11:23 AM
> To: netdev@...r.kernel.org
> Cc: Vecera, Ivan <ivecera@...hat.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@...el.com>; shaojijie@...wei.com; Oros, Petr
> <poros@...hat.com>; Keller, Jacob E <jacob.e.keller@...el.com>;
> Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@...el.com>; Andrew Lunn <andrew+netdev@...n.ch>;
> David S. Miller <davem@...emloft.net>; Eric Dumazet
> <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>; Paolo Abeni
> <pabeni@...hat.com>; Stanislav Fomichev <sdf@...ichev.me>; intel-
> wired-lan@...ts.osuosl.org; linux-kernel@...r.kernel.org
> Subject: [PATCH net v2] iavf: fix deadlock in reset handling
>
> Three driver callbacks schedule a reset and wait for its completion:
> ndo_change_mtu(), ethtool set_ringparam(), and ethtool set_channels().
>
> Waiting for reset in ndo_change_mtu() and set_ringparam() was added by
> commit c2ed2403f12c ("iavf: Wait for reset in callbacks which trigger
> it") to fix a race condition where adding an interface to bonding
> immediately after MTU or ring parameter change failed because the
> interface was still in __RESETTING state. The same commit also added
> waiting in iavf_set_priv_flags(), which was later removed by commit
> 53844673d555 ("iavf: kill "legacy-rx" for good").
>
> Waiting in set_channels() was introduced earlier by commit
> 4e5e6b5d9d13
> ("iavf: Fix return of set the new channel count") to ensure the PF has
> enough time to complete the VF reset when changing channel count, and
> to return correct error codes to userspace.
>
> Commit ef490bbb2267 ("iavf: Add net_shaper_ops support") added
> net_shaper_ops to iavf, which required reset_task to use _locked NAPI
> variants (napi_enable_locked, napi_disable_locked) that need the
> netdev instance lock.
>
> Later, commit 7e4d784f5810 ("net: hold netdev instance lock during
> rtnetlink operations") and commit 2bcf4772e45a ("net: ethtool: try to
> protect all callback with netdev instance lock") started holding the
> netdev instance lock during ndo and ethtool callbacks for drivers with
> net_shaper_ops.
>
> Finally, commit 120f28a6f314 ("iavf: get rid of the crit lock")
> replaced the driver's crit_lock with netdev_lock in reset_task, making
> the deadlock manifest: the callback holds netdev_lock and waits for
> reset_task, but reset_task needs the same lock:
>
> Thread 1 (callback) Thread 2 (reset_task)
> ------------------- ---------------------
> netdev_lock() [blocked on workqueue]
> ndo_change_mtu() or ethtool op
> iavf_schedule_reset()
> iavf_wait_for_reset() iavf_reset_task()
> waiting... netdev_lock() <- DEADLOCK
>
> Fix this by extracting the reset logic from iavf_reset_task() into a
> new
> iavf_reset_step() function that expects netdev_lock to be already
> held.
> The three callbacks now call iavf_reset_step() directly instead of
> scheduling the work and waiting, performing the reset synchronously in
> the caller's context which already holds netdev_lock. This eliminates
> both the deadlock and the need for iavf_wait_for_reset(), which is
> removed.
>
> The workqueue-based iavf_reset_task() becomes a thin wrapper that
> acquires netdev_lock and calls iavf_reset_step(), preserving its use
> for PF-initiated resets.
>
> The callbacks may block for several seconds while iavf_reset_step()
> polls hardware registers, but this is acceptable since netdev_lock is
> a per-device mutex and only serializes operations on the same
> interface.
>
> Fixes: 120f28a6f314 ("iavf: get rid of the crit lock")
> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> Signed-off-by: Petr Oros <poros@...hat.com>
> ---
> drivers/net/ethernet/intel/iavf/iavf.h | 2 +-
> .../net/ethernet/intel/iavf/iavf_ethtool.c | 21 +++---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 72 +++++++-----------
> -
> 3 files changed, 33 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h
> b/drivers/net/ethernet/intel/iavf/iavf.h
> index d552f912e8a947..0c3844b3ff1c86 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -625,5 +625,5 @@ void iavf_add_adv_rss_cfg(struct iavf_adapter
> *adapter); void iavf_del_adv_rss_cfg(struct iavf_adapter *adapter);
> struct iavf_mac_filter *iavf_add_filter(struct iavf_adapter *adapter,
> const u8 *macaddr);
...
> /**
> --
> 2.52.0
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
Powered by blists - more mailing lists