[PATCH net 0/3] Fix i40e/iavf VF bonding after netdev lock changes

[PATCH net 0/3] Fix i40e/iavf VF bonding after netdev lock changes

[Jose Ignacio Tornos Martinez]

This series fixes VF bonding failures introduced by commit ad7c7b2172c3
("net: hold netdev instance lock during sysfs operations").

The core issue is lock contention: iavf_set_mac() is now called with the
netdev lock held and waits for MAC change completion while holding it.
However, the watchdog task that processes the request also needs this lock,
creating a deadlock scenario where the watchdog cannot run, causing
timeouts.

Additionally, setting VF trust triggers an unnecessary ~10 second VF reset
that delays bonding setup, even though filter synchronization happens
naturally during normal VF operation.

This series:
1. Adds safety guard to avoid waiting with locks during reset
2. Eliminates unnecessary VF reset when setting trust (major performance
   win)
3. Fixes the lock contention by dropping the lock while waiting

Testing shows VF bonding now works reliably in ~5 seconds vs 15+ seconds
before, without timeouts or errors.

Tested on Intel 700-series dual-port NIC (i40e) with iavf driver.

Thanks to Jan Tluka <jtluka@redhat.com> for reporting the issue.

Jose Ignacio Tornos Martinez (3):
  iavf: return EBUSY if reset in progress during MAC change
  i40e: skip unnecessary VF reset when setting trust
  iavf: drop netdev lock while waiting for MAC change completion

 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 12 +++++++-----
 drivers/net/ethernet/intel/iavf/iavf_main.c        | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 5 deletions(-)

--
2.43.0

[PATCH net 1/3] iavf: return EBUSY if reset in progress during MAC change

[Jose Ignacio Tornos Martinez]

When bonding enslaves a VF shortly after creation/reset, the VF may still
be initializing. Instead of waiting (which holds locks), immediately return
-EBUSY if reset is in progress. This allows the caller to retry or handle
the busy state appropriately without blocking other operations.

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/ethernet/intel/iavf/iavf_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index dad001abc908..0ca4f9696a41 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1060,6 +1060,9 @@ static int iavf_set_mac(struct net_device *netdev, void *p)
 	struct sockaddr *addr = p;
 	int ret;
 
+	if (iavf_is_reset_in_progress(adapter))
+		return -EBUSY;
+
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
 
-- 
2.53.0

[PATCH net 2/3] i40e: skip unnecessary VF reset when setting trust

[Jose Ignacio Tornos Martinez]

When VF trust is changed, i40e_ndo_set_vf_trust() always calls
i40e_vc_reset_vf() to sync MAC/VLAN filters. However, this reset is
only necessary when trust is removed from a VF that has ADQ (advanced
queue) filters, which need to be deleted

In all other cases, the reset causes a ~10 second delay during which:
- VF must reinitialize completely
- Any in-progress operations (like bonding enslave) fail with timeouts
- VF is unavailable

The MAC/VLAN filter sync will happen naturally through the normal VF
operations and doesn't require a forced reset.

Fix by only resetting when actually needed: when removing trust from a
VF that has ADQ cloud filters. For all other trust changes, just update
the trust flag and let normal operation continue.

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index a26c3d47ec15..fea267af7afe 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4987,16 +4987,21 @@ int i40e_ndo_set_vf_trust(struct net_device *netdev, int vf_id, bool setting)
 	set_bit(__I40E_MACVLAN_SYNC_PENDING, pf->state);
 	pf->vsi[vf->lan_vsi_idx]->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
 
-	i40e_vc_reset_vf(vf, true);
 	dev_info(&pf->pdev->dev, "VF %u is now %strusted\n",
 		 vf_id, setting ? "" : "un");
 
+	/* Only reset VF if we're removing trust and it has ADQ cloud filters.
+	 * Cloud filters can only be added when trusted, so they must be
+	 * removed when trust is revoked. Other trust changes don't require
+	 * reset - MAC/VLAN filter sync happens through normal operation.
+	 */
 	if (vf->adq_enabled) {
 		if (!vf->trusted) {
 			dev_info(&pf->pdev->dev,
 				 "VF %u no longer Trusted, deleting all cloud filters\n",
 				 vf_id);
 			i40e_del_all_cloud_filters(vf);
+			i40e_vc_reset_vf(vf, true);
 		}
 	}
 
-- 
2.53.0

[PATCH net 3/3] iavf: drop netdev lock while waiting for MAC change completion

[Jose Ignacio Tornos Martinez]

After commit ad7c7b2172c3 ("net: hold netdev instance lock during sysfs
operations"), iavf_set_mac() is called with the netdev instance lock
already held.

The function queues a MAC address change request and then waits for
completion while holding this lock. However, the watchdog task that
processes admin queue commands (including MAC changes) also needs to
acquire the netdev lock to run.

This creates a lock contention scenario:
1. iavf_set_mac() holds netdev lock and waits for MAC change
2. Watchdog needs netdev lock to process the MAC change request
3. Watchdog blocks waiting for lock
4. MAC change times out after 2.5 seconds
5. iavf_set_mac() returns -EAGAIN

This particularly affects VFs during initialization when enslaved to a
bond. The first VF typically succeeds as it's already fully initialized,
but subsequent VFs fail as they're still progressing through their state
machine and need the watchdog to advance.

Fix by temporarily dropping the netdev lock before waiting for MAC change
completion, allowing the watchdog to run and process the request, then
re-acquiring the lock before returning.

This is safe because:
- The MAC change request is already queued before we drop the lock
- iavf_is_mac_set_handled() just checks filter state, doesn't modify it
- We re-acquire the lock before checking results and returning

Fixes: ad7c7b2172c3 ("net: hold netdev instance lock during sysfs operations")
cc: stable@vger.kernel.org
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index dad001abc908..6281858e6f3c 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1068,10 +1068,14 @@ static int iavf_set_mac(struct net_device *netdev, void *p)
 	if (ret)
 		return ret;
 
+	netdev_unlock(netdev);
+
 	ret = wait_event_interruptible_timeout(adapter->vc_waitqueue,
 					       iavf_is_mac_set_handled(netdev, addr->sa_data),
 					       msecs_to_jiffies(2500));
 
+	netdev_lock(netdev);
+
 	/* If ret < 0 then it means wait was interrupted.
 	 * If ret == 0 then it means we got a timeout.
 	 * else it means we got response for set MAC from PF,
-- 
2.53.0

Re: [PATCH net 3/3] iavf: drop netdev lock while waiting for MAC change completion

[Kohei Enju]

On 04/06 13:20, Jose Ignacio Tornos Martinez wrote:
> After commit ad7c7b2172c3 ("net: hold netdev instance lock during sysfs
> operations"), iavf_set_mac() is called with the netdev instance lock
> already held.
> 
> The function queues a MAC address change request and then waits for
> completion while holding this lock. However, the watchdog task that
> processes admin queue commands (including MAC changes) also needs to
> acquire the netdev lock to run.
> 
> This creates a lock contention scenario:
> 1. iavf_set_mac() holds netdev lock and waits for MAC change
> 2. Watchdog needs netdev lock to process the MAC change request
> 3. Watchdog blocks waiting for lock
> 4. MAC change times out after 2.5 seconds
> 5. iavf_set_mac() returns -EAGAIN
> 
> This particularly affects VFs during initialization when enslaved to a
> bond. The first VF typically succeeds as it's already fully initialized,
> but subsequent VFs fail as they're still progressing through their state
> machine and need the watchdog to advance.
> 
> Fix by temporarily dropping the netdev lock before waiting for MAC change
> completion, allowing the watchdog to run and process the request, then
> re-acquiring the lock before returning.
> 
> This is safe because:
> - The MAC change request is already queued before we drop the lock
> - iavf_is_mac_set_handled() just checks filter state, doesn't modify it
> - We re-acquire the lock before checking results and returning
> 
> Fixes: ad7c7b2172c3 ("net: hold netdev instance lock during sysfs operations")
> cc: stable@vger.kernel.org
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index dad001abc908..6281858e6f3c 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1068,10 +1068,14 @@ static int iavf_set_mac(struct net_device *netdev, void *p)
>  	if (ret)
>  		return ret;
>  
> +	netdev_unlock(netdev);
> +
>  	ret = wait_event_interruptible_timeout(adapter->vc_waitqueue,
>  					       iavf_is_mac_set_handled(netdev, addr->sa_data),
>  					       msecs_to_jiffies(2500));
>  
> +	netdev_lock(netdev);
> +

Hi Jose, thank you for the fix and detailed explanation.

I don't have a great solution for this issue, but dropping the netdev
lock taken by the networking core in the driver callback might not look
acceptable.

FYI, Petr reported the same type of locking issue in
ndo_change_mtu(), and the v1 approach was really similar to this one.
https://lore.kernel.org/intel-wired-lan/20260202155813.3f8fbc27@kernel.org/

IIUC, the issue was eventually fixed by completing the reset
synchronously in the same context as ndo_change_mtu(), instead of
dropping the netdev lock and waiting for reset_task.
https://lore.kernel.org/intel-wired-lan/20260211191855.1532226-1-poros@redhat.com/

If that applies here as well, maybe iavf_set_mac() needs a similar
approach, e.g. progressing the relevant virtchnl request/completion
synchronously with the netdev lock held, rather than dropping the lock
here?

>  	/* If ret < 0 then it means wait was interrupted.
>  	 * If ret == 0 then it means we got a timeout.
>  	 * else it means we got response for set MAC from PF,
> -- 
> 2.53.0
>