[PATCH net v3 0/5] Fix i40e/ice/iavf VF bonding after netdev lock changes

[PATCH net v3 0/5] Fix i40e/ice/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").

When adding VFs to a bond immediately after setting trust mode, MAC
address changes fail with -EAGAIN, preventing bonding setup. This
affects both i40e (700-series) and ice (800-series) Intel NICs.

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, both the watchdog task that sends the request and the adminq_task
that processes PF responses also need this lock, creating a deadlock where
neither can run, causing timeouts.

Additionally, setting VF trust triggers an unnecessary ~10 second VF reset
in i40e driver that delays bonding setup, even though filter
synchronization happens naturally during normal VF operation. For ice
driver, the delay is not so big, but in the same way the operation is not
necessary.

This series:
1. Adds safety guard to prevent MAC changes during reset or early
   initialization (before VF is ready)
2. Eliminates unnecessary VF reset when setting trust in i40e
3. Fixes lock contention by polling admin queue synchronously
4. Eliminates unnecessary VF reset when setting trust in ice
5. Refactors virtchnl polling to unify init-time and runtime code paths

The key fix (patch 3/5) implements a synchronous MAC change operation
similar to the approach used for ndo_change_mtu deadlock fix:
https://lore.kernel.org/intel-wired-lan/20260211191855.1532226-1-poros@redhat.com/ 
Instead of scheduling work and waiting, it:

- Sends the virtchnl message directly (not via watchdog)
- Polls the admin queue hardware directly for responses
- Processes all messages inline (including non-MAC messages)
- Returns when complete or times out

This allows the operation to complete synchronously while holding
netdev_lock, without relying on watchdog or adminq_task. A new generic
iavf_poll_virtchnl_response() function was introduced for this.

Patch 5 refactors the polling implementation based on Przemek Kitszel
feedback, unifying in a centralized polling way, the previously (with
patch 3) separate init-time (avf_poll_virtchnl_msg()) and runtime polling
(iavf_poll_virtchnl_response()) into the original polling function 
(iavf_poll_virtchnl_msg()) allowing both behaviors.
I have preferred to create a separate patch for the refactoring for the
sake of clarity in the solution, and I would prefer to include in the net
series because it is tightly coupled with patch 3.

The function can sleep for up to 2.5 seconds polling hardware, but this
is acceptable since netdev_lock is per-device and only serializes
operations on the same interface.

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

Tested on Intel 700-series (i40e) and 800-series (ice) dual-port NICs
with iavf driver.

Thanks to Jan Tluka <jtluka@redhat.com> and Yuying Ma <yuma@redhat.com> for
reporting the issues.

Jose Ignacio Tornos Martinez (5):
  iavf: return EBUSY if reset in progress or not ready during MAC change
  i40e: skip unnecessary VF reset when setting trust
  iavf: send MAC change request synchronously
  ice: skip unnecessary VF reset when setting trust
  iavf: refactor virtchnl polling to unify init and runtime paths

---
v3:
  - Complete patch 3 with the comments from Przemek Kitszel
  - Added patch 5: Refactor to unify polling into iavf_poll_virtchnl_msg()
    function (Przemek Kitszel suggestion). It processes messages through
    iavf_virtchnl_completion() when appropriate (runtime operations with
    timeout; init-time operations continue to return raw messages without
    completion processing).
  - No changes to patch 1,2 and 4 from v2
v2: https://lore.kernel.org/netdev/20260407165206.1121317-1-jtornosm@redhat.com/

 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |   7 ++++++-
 drivers/net/ethernet/intel/iavf/iavf.h             |   6 +++++-
 drivers/net/ethernet/intel/iavf/iavf_main.c        |  69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
 drivers/net/ethernet/intel/iavf/iavf_virtchnl.c    | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------
 drivers/net/ethernet/intel/ice/ice_sriov.c         |  13 +++++++++----
 5 files changed, 193 insertions(+), 64 deletions(-)
--
2.43.0

[PATCH net v3 1/5] iavf: return EBUSY if reset in progress or not ready during MAC change

[Jose Ignacio Tornos Martinez]

When a MAC address change is requested while the VF is resetting or still
initializing, return -EBUSY immediately instead of attempting the
operation.

Additionally, during early initialization states (before __IAVF_DOWN),
the PF may be slow to respond to MAC change requests, causing long
delays. Only allow MAC changes once the VF reaches __IAVF_DOWN state or
later, when the watchdog is running and the VF is ready for operations.

After commit ad7c7b2172c3 ("net: hold netdev instance lock
during sysfs operations"), MAC changes are called with the netdev lock
held, so we should not wait with the lock held during reset or
initialization. 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..67aa14350b1b 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) || adapter->state < __IAVF_DOWN)
+		return -EBUSY;
+
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
 
-- 
2.53.0

[PATCH net v3 2/5] 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 v3 3/5] iavf: send MAC change request synchronously

[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 via
iavf_replace_primary_mac() and then waits for completion. However, in
the current flow, the actual virtchnl message is sent by the watchdog
task, which also needs to acquire the netdev lock to run. Additionally,
the adminq_task which processes virtchnl responses also needs the netdev
lock.

This creates a deadlock scenario:
1. iavf_set_mac() holds netdev lock and waits for MAC change
2. Watchdog needs netdev lock to send the request -> blocked
3. Even if request is sent, adminq_task needs netdev lock to process
   PF response -> blocked
4. MAC change times out after 2.5 seconds
5. iavf_set_mac() returns -EAGAIN

This particularly affects VFs during bonding setup when multiple VFs are
enslaved in quick succession.

Fix by implementing a synchronous MAC change operation similar to the
approach used in commit fdadbf6e84c4 ("iavf: fix incorrect reset handling
in callbacks").

The solution:
1. Send the virtchnl ADD_ETH_ADDR message directly (not via watchdog)
2. Poll the admin queue hardware directly for responses
3. Process all received messages (including non-MAC messages)
4. Return when MAC change completes or times out

A new generic function iavf_poll_virtchnl_response() is introduced that
can be reused for any future synchronous virtchnl operations. It takes a
callback to check completion, allowing flexible condition checking.

This allows the operation to complete synchronously while holding
netdev_lock, without relying on watchdog or adminq_task. The function
can sleep for up to 2.5 seconds polling hardware, but this is acceptable
since netdev_lock is per-device and only serializes operations on the
same interface.

To support this, change iavf_add_ether_addrs() to return an error code
instead of void, allowing callers to detect failures.

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>
---
v3: Complete with Przemek Kitszel comments:                                                                                                                                           
    - Moved iavf_poll_virtchnl_response() to iavf_virtchnl.c for reusability                                                                                                                                               
    - Changed kdoc to use "Return:" instead of "Returns"                                                                                                                                                                   
    - Changed to do-while loop structure                                                                                                                                                                                   
    - Added pending parameter to skip sleep when more messages queued                                                                                                                                                      
    - Reduced sleep time to 50-75 usec (from 1000-2000, per commit 9e3f23f44f32)                                                                                                                                           
    - Added v_opcode parameter for standard completion checking                                                                                                                                                            
    - Callback parameter takes priority over opcode check                                                                                                                                                                  
    - Made cond_data parameter const                                                                                                                                                                                       
    - Final condition check after timeout before returning -EAGAIN                                                                                                                                                         
v2: https://lore.kernel.org/netdev/20260407165206.1121317-4-jtornosm@redhat.com/

 drivers/net/ethernet/intel/iavf/iavf.h        |   7 +-
 drivers/net/ethernet/intel/iavf/iavf_main.c   |  57 ++++++---
 .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 111 +++++++++++++++++-
 3 files changed, 155 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index e9fb0a0919e3..b012a91b0252 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -589,7 +589,7 @@ void iavf_configure_queues(struct iavf_adapter *adapter);
 void iavf_enable_queues(struct iavf_adapter *adapter);
 void iavf_disable_queues(struct iavf_adapter *adapter);
 void iavf_map_queues(struct iavf_adapter *adapter);
-void iavf_add_ether_addrs(struct iavf_adapter *adapter);
+int iavf_add_ether_addrs(struct iavf_adapter *adapter);
 void iavf_del_ether_addrs(struct iavf_adapter *adapter);
 void iavf_add_vlans(struct iavf_adapter *adapter);
 void iavf_del_vlans(struct iavf_adapter *adapter);
@@ -607,6 +607,11 @@ void iavf_disable_vlan_stripping(struct iavf_adapter *adapter);
 void iavf_virtchnl_completion(struct iavf_adapter *adapter,
 			      enum virtchnl_ops v_opcode,
 			      enum iavf_status v_retval, u8 *msg, u16 msglen);
+int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
+				bool (*condition)(struct iavf_adapter *, const void *),
+				const void *cond_data,
+				enum virtchnl_ops v_opcode,
+				unsigned int timeout_ms);
 int iavf_config_rss(struct iavf_adapter *adapter);
 void iavf_cfg_queues_bw(struct iavf_adapter *adapter);
 void iavf_cfg_queues_quanta_size(struct iavf_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 67aa14350b1b..80277d495a8d 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1047,6 +1047,46 @@ static bool iavf_is_mac_set_handled(struct net_device *netdev,
 	return ret;
 }
 
+/**
+ * iavf_mac_change_done - Check if MAC change completed
+ * @adapter: board private structure
+ * @data: MAC address being checked (as const void *)
+ *
+ * Callback for iavf_poll_virtchnl_response() to check if MAC change completed.
+ *
+ * Returns true if MAC change completed, false otherwise
+ */
+static bool iavf_mac_change_done(struct iavf_adapter *adapter, const void *data)
+{
+	const u8 *addr = data;
+
+	return iavf_is_mac_set_handled(adapter->netdev, addr);
+}
+
+/**
+ * iavf_set_mac_sync - Synchronously change MAC address
+ * @adapter: board private structure
+ * @addr: MAC address to set
+ *
+ * Sends MAC change request to PF and polls admin queue for response.
+ * Caller must hold netdev_lock. This can sleep for up to 2.5 seconds.
+ *
+ * Returns 0 on success, negative on failure
+ */
+static int iavf_set_mac_sync(struct iavf_adapter *adapter, const u8 *addr)
+{
+	int ret;
+
+	netdev_assert_locked(adapter->netdev);
+
+	ret = iavf_add_ether_addrs(adapter);
+	if (ret)
+		return ret;
+
+	return iavf_poll_virtchnl_response(adapter, iavf_mac_change_done, addr,
+					   VIRTCHNL_OP_UNKNOWN, 2500);
+}
+
 /**
  * iavf_set_mac - NDO callback to set port MAC address
  * @netdev: network interface device structure
@@ -1067,26 +1107,13 @@ static int iavf_set_mac(struct net_device *netdev, void *p)
 		return -EADDRNOTAVAIL;
 
 	ret = iavf_replace_primary_mac(adapter, addr->sa_data);
-
 	if (ret)
 		return ret;
 
-	ret = wait_event_interruptible_timeout(adapter->vc_waitqueue,
-					       iavf_is_mac_set_handled(netdev, addr->sa_data),
-					       msecs_to_jiffies(2500));
-
-	/* 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,
-	 * check if netdev MAC was updated to requested MAC,
-	 * if yes then set MAC succeeded otherwise it failed return -EACCES
-	 */
-	if (ret < 0)
+	ret = iavf_set_mac_sync(adapter, addr->sa_data);
+	if (ret)
 		return ret;
 
-	if (!ret)
-		return -EAGAIN;
-
 	if (!ether_addr_equal(netdev->dev_addr, addr->sa_data))
 		return -EACCES;
 
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index a52c100dcbc5..df124f840ddb 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -2,6 +2,7 @@
 /* Copyright(c) 2013 - 2018 Intel Corporation. */
 
 #include <linux/net/intel/libie/rx.h>
+#include <net/netdev_lock.h>
 
 #include "iavf.h"
 #include "iavf_ptp.h"
@@ -555,8 +556,10 @@ iavf_set_mac_addr_type(struct virtchnl_ether_addr *virtchnl_ether_addr,
  * @adapter: adapter structure
  *
  * Request that the PF add one or more addresses to our filters.
+ *
+ * Return: 0 on success, negative on failure
  **/
-void iavf_add_ether_addrs(struct iavf_adapter *adapter)
+int iavf_add_ether_addrs(struct iavf_adapter *adapter)
 {
 	struct virtchnl_ether_addr_list *veal;
 	struct iavf_mac_filter *f;
@@ -568,7 +571,7 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter)
 		/* bail because we already have a command pending */
 		dev_err(&adapter->pdev->dev, "Cannot add filters, command %d pending\n",
 			adapter->current_op);
-		return;
+		return -EBUSY;
 	}
 
 	spin_lock_bh(&adapter->mac_vlan_list_lock);
@@ -580,7 +583,7 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter)
 	if (!count) {
 		adapter->aq_required &= ~IAVF_FLAG_AQ_ADD_MAC_FILTER;
 		spin_unlock_bh(&adapter->mac_vlan_list_lock);
-		return;
+		return 0;
 	}
 	adapter->current_op = VIRTCHNL_OP_ADD_ETH_ADDR;
 
@@ -595,7 +598,7 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter)
 	veal = kzalloc(len, GFP_ATOMIC);
 	if (!veal) {
 		spin_unlock_bh(&adapter->mac_vlan_list_lock);
-		return;
+		return -ENOMEM;
 	}
 
 	veal->vsi_id = adapter->vsi_res->vsi_id;
@@ -617,6 +620,7 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter)
 
 	iavf_send_pf_msg(adapter, VIRTCHNL_OP_ADD_ETH_ADDR, (u8 *)veal, len);
 	kfree(veal);
+	return 0;
 }
 
 /**
@@ -2956,3 +2960,102 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
 	} /* switch v_opcode */
 	adapter->current_op = VIRTCHNL_OP_UNKNOWN;
 }
+
+/**
+ * iavf_virtchnl_done - Check if virtchnl operation completed
+ * @adapter: board private structure
+ * @condition: optional callback for custom completion check
+ *   (takes priority)
+ * @cond_data: context data for callback
+ * @v_opcode: virtchnl opcode value we're waiting for if no condition
+ *   configured (typically VIRTCHNL_OP_UNKNOWN), if condition not used
+ *
+ * Checks completion status. Callback takes priority if provided. Otherwise
+ * waits for current_op to reach v_opcode (typically VIRTCHNL_OP_UNKNOWN
+ * after completion).
+ *
+ * Return: true if operation completed
+ */
+static inline bool iavf_virtchnl_done(struct iavf_adapter *adapter,
+				      bool (*condition)(struct iavf_adapter *, const void *),
+				      const void *cond_data,
+				      enum virtchnl_ops v_opcode)
+{
+	if (condition)
+		return condition(adapter, cond_data);
+
+	return adapter->current_op == v_opcode;
+}
+
+/**
+ * iavf_poll_virtchnl_response - Poll admin queue for virtchnl response
+ * @adapter: board private structure
+ * @condition: optional callback to check if desired response received
+ *   (takes priority)
+ * @cond_data: context data passed to condition callback
+ * @v_opcode: virtchnl opcode value to wait for if no condition configured
+ *   (typically VIRTCHNL_OP_UNKNOWN), if condition, not used
+ * @timeout_ms: maximum time to wait in milliseconds
+ *
+ * Polls admin queue and processes all messages until condition returns true
+ * or timeout expires. If condition is NULL, waits for current_op to become
+ * v_opcode (typically VIRTCHNL_OP_UNKNOWN after operation completes).
+ * Caller must hold netdev_lock. This can sleep for up to timeout_ms while
+ * polling hardware.
+ *
+ * Return: 0 on success (condition met), -EAGAIN on timeout or error
+ */
+int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
+				bool (*condition)(struct iavf_adapter *, const void *),
+				const void *cond_data,
+				enum virtchnl_ops v_opcode,
+				unsigned int timeout_ms)
+{
+	struct iavf_hw *hw = &adapter->hw;
+	struct iavf_arq_event_info event;
+	enum virtchnl_ops v_op;
+	enum iavf_status v_ret;
+	unsigned long timeout;
+	u16 pending;
+	int ret;
+
+	netdev_assert_locked(adapter->netdev);
+
+	event.buf_len = IAVF_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
+	if (!event.msg_buf)
+		return -ENOMEM;
+
+	timeout = jiffies + msecs_to_jiffies(timeout_ms);
+	do {
+		if (iavf_virtchnl_done(adapter, condition, cond_data, v_opcode)) {
+			ret = 0;
+			goto out;
+		}
+
+		ret = iavf_clean_arq_element(hw, &event, &pending);
+		if (!ret) {
+			v_op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);
+			v_ret = (enum iavf_status)le32_to_cpu(event.desc.cookie_low);
+
+			iavf_virtchnl_completion(adapter, v_op, v_ret,
+						 event.msg_buf, event.msg_len);
+
+			memset(event.msg_buf, 0, IAVF_MAX_AQ_BUF_SIZE);
+
+			if (pending)
+				continue;
+		}
+
+		usleep_range(50, 75);
+	} while (time_before(jiffies, timeout));
+
+	if (iavf_virtchnl_done(adapter, condition, cond_data, v_opcode))
+		ret = 0;
+	else
+		ret = -EAGAIN;
+
+out:
+	kfree(event.msg_buf);
+	return ret;
+}
-- 
2.53.0

[PATCH net v3 4/5] ice: skip unnecessary VF reset when setting trust

[Jose Ignacio Tornos Martinez]

Similar to the i40e fix, ice_set_vf_trust() unconditionally calls
ice_reset_vf() when the trust setting changes.

The ice driver already has logic to clean up MAC LLDP filters when
removing trust, which is the only operation that requires filter
synchronization. After this cleanup, the VF reset is only necessary if
there were actually filters to remove.

For all other trust state changes (setting trust, or removing trust
when no filters exist), the reset is unnecessary as filter
synchronization happens naturally through normal VF operations.

Fix by only triggering the VF reset when removing trust AND filters
were actually cleaned up (num_mac_lldp was non-zero).

This saves some time and eliminates unnecessary service disruption when
changing VF trust settings if not necessary.

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/ethernet/intel/ice/ice_sriov.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 7e00e091756d..23f692b1e86c 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -1399,14 +1399,19 @@ int ice_set_vf_trust(struct net_device *netdev, int vf_id, bool trusted)
 
 	mutex_lock(&vf->cfg_lock);
 
-	while (!trusted && vf->num_mac_lldp)
-		ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), false);
-
 	vf->trusted = trusted;
-	ice_reset_vf(vf, ICE_VF_RESET_NOTIFY);
 	dev_info(ice_pf_to_dev(pf), "VF %u is now %strusted\n",
 		 vf_id, trusted ? "" : "un");
 
+	/* Only reset VF if removing trust and there are MAC LLDP filters
+	 * to clean up. Reset is needed to ensure filter removal completes.
+	 */
+	if (!trusted && vf->num_mac_lldp) {
+		while (vf->num_mac_lldp)
+			ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), false);
+		ice_reset_vf(vf, ICE_VF_RESET_NOTIFY);
+	}
+
 	mutex_unlock(&vf->cfg_lock);
 
 out_put_vf:
-- 
2.53.0

[PATCH net v3 5/5] iavf: refactor virtchnl polling into single function

[Jose Ignacio Tornos Martinez]

At this moment, the driver has two separate functions for polling virtchnl
messages from the admin queue:
- iavf_poll_virtchnl_msg() for init-time (no timeout, no completion
  handler)
- iavf_poll_virtchnl_response() for runtime (with timeout, calls
  completion)

Refactor by enhancing iavf_poll_virtchnl_msg() to handle both use cases:
1. Init-time mode (timeout_ms=0):
  - Polls until matching opcode found or queue empty
  - Returns raw message data without processing through completion handler
  - Exits immediately on empty queue (no sleep/retry)
2. Runtime mode (timeout_ms>0):
  - Polls with timeout using condition callback or opcode check
  - Processes all messages through iavf_virtchnl_completion()
  - Supports custom completion callback (takes priority) or falls back
    to checking adapter->current_op against expected opcode
  - Uses pending parameter to skip sleep when more messages queued
  - Uses 50-75 usec sleep (due to commit 9e3f23f44f32 ("i40e: reduce wait
    time for adminq command completion"))

By unifying message handling, both init-time and runtime messages can be
processed through the completion handler when appropriate, ensuring
consistent state updates and maintaining backward compatibility with all
existing call sites.

Suggested-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/ethernet/intel/iavf/iavf.h        |   9 +-
 drivers/net/ethernet/intel/iavf/iavf_main.c   |  13 +-
 .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 247 ++++++++----------
 3 files changed, 125 insertions(+), 144 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index b012a91b0252..9b25c5a65d2a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -607,11 +607,10 @@ void iavf_disable_vlan_stripping(struct iavf_adapter *adapter);
 void iavf_virtchnl_completion(struct iavf_adapter *adapter,
 			      enum virtchnl_ops v_opcode,
 			      enum iavf_status v_retval, u8 *msg, u16 msglen);
-int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
-				bool (*condition)(struct iavf_adapter *, const void *),
-				const void *cond_data,
-				enum virtchnl_ops v_opcode,
-				unsigned int timeout_ms);
+int iavf_poll_virtchnl_msg(struct iavf_hw *hw, struct iavf_arq_event_info *event,
+			   enum virtchnl_ops op_to_poll, unsigned int timeout_ms,
+			   bool (*condition)(struct iavf_adapter *, const void *),
+			   const void *cond_data);
 int iavf_config_rss(struct iavf_adapter *adapter);
 void iavf_cfg_queues_bw(struct iavf_adapter *adapter);
 void iavf_cfg_queues_quanta_size(struct iavf_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 80277d495a8d..b0db15fd8ddb 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1075,6 +1075,7 @@ static bool iavf_mac_change_done(struct iavf_adapter *adapter, const void *data)
  */
 static int iavf_set_mac_sync(struct iavf_adapter *adapter, const u8 *addr)
 {
+	struct iavf_arq_event_info event;
 	int ret;
 
 	netdev_assert_locked(adapter->netdev);
@@ -1083,8 +1084,16 @@ static int iavf_set_mac_sync(struct iavf_adapter *adapter, const u8 *addr)
 	if (ret)
 		return ret;
 
-	return iavf_poll_virtchnl_response(adapter, iavf_mac_change_done, addr,
-					   VIRTCHNL_OP_UNKNOWN, 2500);
+	event.buf_len = IAVF_MAX_AQ_BUF_SIZE;
+	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
+	if (!event.msg_buf)
+		return -ENOMEM;
+
+	ret = iavf_poll_virtchnl_msg(&adapter->hw, &event, VIRTCHNL_OP_UNKNOWN,
+				     2500, iavf_mac_change_done, addr);
+
+	kfree(event.msg_buf);
+	return ret;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index df124f840ddb..ef9a251060d9 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -54,55 +54,121 @@ int iavf_send_api_ver(struct iavf_adapter *adapter)
 }
 
 /**
- * iavf_poll_virtchnl_msg
+ * iavf_virtchnl_completion_done - Check if virtchnl operation completed
+ * @adapter: adapter structure
+ * @condition: optional callback for custom completion check
+ * @cond_data: context data for callback
+ * @op_to_poll: opcode to check against current_op (if no callback)
+ *
+ * Checks if operation is complete. Callback takes priority if provided,
+ * otherwise checks if current_op matches op_to_poll.
+ *
+ * Return: true if operation completed
+ */
+static inline bool
+iavf_virtchnl_completion_done(struct iavf_adapter *adapter,
+			      bool (*condition)(struct iavf_adapter *, const void *),
+			      const void *cond_data,
+			      enum virtchnl_ops op_to_poll)
+{
+	if (condition)
+		return condition(adapter, cond_data);
+
+	return adapter->current_op == op_to_poll;
+}
+
+/**
+ * iavf_poll_virtchnl_msg - Poll admin queue for virtchnl message
  * @hw: HW configuration structure
  * @event: event to populate on success
- * @op_to_poll: requested virtchnl op to poll for
+ * @op_to_poll: virtchnl opcode to poll for (used for init-time and runtime
+ *              without callback)
+ * @timeout_ms: timeout in milliseconds (0 = no timeout, exit on empty queue)
+ * @condition: optional callback to check custom completion (runtime use,
+ *             takes priority over op_to_poll check)
+ * @cond_data: context data for condition callback
+ *
+ * Enhanced polling function that handles both init-time and runtime use cases:
+ * - Init-time: Set op_to_poll, timeout_ms=0, condition=NULL
+ *   Polls until matching opcode found or queue empty
+ * - Runtime with callback: Set timeout_ms>0, condition callback, cond_data
+ *   Polls with timeout until condition returns true (op_to_poll not used)
+ * - Runtime without callback: Set op_to_poll, timeout_ms>0, condition=NULL
+ *   Polls with timeout until adapter->current_op == op_to_poll
+ *
+ * Runtime messages are processed through iavf_virtchnl_completion().
+ * For init-time use, returns 0 with raw message data in event buffer.
+ * For runtime use, returns 0 when completion condition is met.
  *
- * Initialize poll for virtchnl msg matching the requested_op. Returns 0
- * if a message of the correct opcode is in the queue or an error code
- * if no message matching the op code is waiting and other failures.
+ * Return: 0 on success, -EAGAIN on timeout, or error code
  */
-static int
-iavf_poll_virtchnl_msg(struct iavf_hw *hw, struct iavf_arq_event_info *event,
-		       enum virtchnl_ops op_to_poll)
+int iavf_poll_virtchnl_msg(struct iavf_hw *hw, struct iavf_arq_event_info *event,
+			   enum virtchnl_ops op_to_poll, unsigned int timeout_ms,
+			   bool (*condition)(struct iavf_adapter *, const void *),
+			   const void *cond_data)
 {
+	struct iavf_adapter *adapter = hw->back;
+	unsigned long timeout = timeout_ms ? jiffies + msecs_to_jiffies(timeout_ms) : 0;
 	enum virtchnl_ops received_op;
 	enum iavf_status status;
-	u32 v_retval;
+	u32 v_retval = 0;
+	u16 pending;
 
-	while (1) {
-		/* When the AQ is empty, iavf_clean_arq_element will return
-		 * nonzero and this loop will terminate.
-		 */
-		status = iavf_clean_arq_element(hw, event, NULL);
-		if (status != IAVF_SUCCESS)
-			return iavf_status_to_errno(status);
-		received_op =
-		    (enum virtchnl_ops)le32_to_cpu(event->desc.cookie_high);
+	do {
+		if (timeout_ms && iavf_virtchnl_completion_done(adapter, condition,
+								cond_data, op_to_poll))
+			return 0;
 
-		if (received_op == VIRTCHNL_OP_EVENT) {
-			struct iavf_adapter *adapter = hw->back;
-			struct virtchnl_pf_event *vpe =
-				(struct virtchnl_pf_event *)event->msg_buf;
+		status = iavf_clean_arq_element(hw, event, &pending);
+		if (status == IAVF_SUCCESS) {
+			received_op = (enum virtchnl_ops)le32_to_cpu(event->desc.cookie_high);
 
-			if (vpe->event != VIRTCHNL_EVENT_RESET_IMPENDING)
-				continue;
+			/* Handle reset events specially */
+			if (received_op == VIRTCHNL_OP_EVENT) {
+				struct virtchnl_pf_event *vpe =
+					(struct virtchnl_pf_event *)event->msg_buf;
 
-			dev_info(&adapter->pdev->dev, "Reset indication received from the PF\n");
-			if (!(adapter->flags & IAVF_FLAG_RESET_PENDING))
-				iavf_schedule_reset(adapter,
-						    IAVF_FLAG_RESET_PENDING);
+				if (vpe->event != VIRTCHNL_EVENT_RESET_IMPENDING)
+					continue;
+
+				dev_info(&adapter->pdev->dev,
+					 "Reset indication received from the PF\n");
+				if (!(adapter->flags & IAVF_FLAG_RESET_PENDING))
+					iavf_schedule_reset(adapter,
+							    IAVF_FLAG_RESET_PENDING);
+
+				return -EIO;
+			}
+
+			v_retval = le32_to_cpu(event->desc.cookie_low);
+
+			if (!timeout_ms) {
+				if (received_op == op_to_poll)
+					return virtchnl_status_to_errno((enum virtchnl_status_code)
+							v_retval);
+			} else {
+				iavf_virtchnl_completion(adapter, received_op,
+							 (enum iavf_status)v_retval,
+							 event->msg_buf, event->msg_len);
+			}
+
+			if (pending)
+				continue;
+		} else if (!timeout_ms) {
+			return iavf_status_to_errno(status);
+		}
 
-			return -EIO;
+		if (timeout_ms) {
+			memset(event->msg_buf, 0, IAVF_MAX_AQ_BUF_SIZE);
+			usleep_range(50, 75);
 		}
 
-		if (op_to_poll == received_op)
-			break;
-	}
+	} while (!timeout_ms || time_before(jiffies, timeout));
+
+	if (iavf_virtchnl_completion_done(adapter, condition, cond_data, op_to_poll))
+		return 0;
 
-	v_retval = le32_to_cpu(event->desc.cookie_low);
-	return virtchnl_status_to_errno((enum virtchnl_status_code)v_retval);
+	return -EAGAIN;
 }
 
 /**
@@ -124,7 +190,8 @@ int iavf_verify_api_ver(struct iavf_adapter *adapter)
 	if (!event.msg_buf)
 		return -ENOMEM;
 
-	err = iavf_poll_virtchnl_msg(&adapter->hw, &event, VIRTCHNL_OP_VERSION);
+	err = iavf_poll_virtchnl_msg(&adapter->hw, &event, VIRTCHNL_OP_VERSION,
+				     0, NULL, NULL);
 	if (!err) {
 		struct virtchnl_version_info *pf_vvi =
 			(struct virtchnl_version_info *)event.msg_buf;
@@ -294,7 +361,8 @@ int iavf_get_vf_config(struct iavf_adapter *adapter)
 	if (!event.msg_buf)
 		return -ENOMEM;
 
-	err = iavf_poll_virtchnl_msg(hw, &event, VIRTCHNL_OP_GET_VF_RESOURCES);
+	err = iavf_poll_virtchnl_msg(hw, &event, VIRTCHNL_OP_GET_VF_RESOURCES,
+				     0, NULL, NULL);
 	memcpy(adapter->vf_res, event.msg_buf, min(event.msg_len, len));
 
 	/* some PFs send more queues than we should have so validate that
@@ -322,7 +390,8 @@ int iavf_get_vf_vlan_v2_caps(struct iavf_adapter *adapter)
 		return -ENOMEM;
 
 	err = iavf_poll_virtchnl_msg(&adapter->hw, &event,
-				     VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS);
+				     VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS,
+				     0, NULL, NULL);
 	if (!err)
 		memcpy(&adapter->vlan_v2_caps, event.msg_buf,
 		       min(event.msg_len, len));
@@ -342,7 +411,8 @@ int iavf_get_vf_supported_rxdids(struct iavf_adapter *adapter)
 	event.buf_len = sizeof(rxdids);
 
 	err = iavf_poll_virtchnl_msg(&adapter->hw, &event,
-				     VIRTCHNL_OP_GET_SUPPORTED_RXDIDS);
+				     VIRTCHNL_OP_GET_SUPPORTED_RXDIDS,
+				     0, NULL, NULL);
 	if (!err)
 		adapter->supp_rxdids = rxdids;
 
@@ -359,7 +429,8 @@ int iavf_get_vf_ptp_caps(struct iavf_adapter *adapter)
 	event.buf_len = sizeof(caps);
 
 	err = iavf_poll_virtchnl_msg(&adapter->hw, &event,
-				     VIRTCHNL_OP_1588_PTP_GET_CAPS);
+				     VIRTCHNL_OP_1588_PTP_GET_CAPS,
+				     0, NULL, NULL);
 	if (!err)
 		adapter->ptp.hw_caps = caps;
 
@@ -2961,101 +3032,3 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
 	adapter->current_op = VIRTCHNL_OP_UNKNOWN;
 }
 
-/**
- * iavf_virtchnl_done - Check if virtchnl operation completed
- * @adapter: board private structure
- * @condition: optional callback for custom completion check
- *   (takes priority)
- * @cond_data: context data for callback
- * @v_opcode: virtchnl opcode value we're waiting for if no condition
- *   configured (typically VIRTCHNL_OP_UNKNOWN), if condition not used
- *
- * Checks completion status. Callback takes priority if provided. Otherwise
- * waits for current_op to reach v_opcode (typically VIRTCHNL_OP_UNKNOWN
- * after completion).
- *
- * Return: true if operation completed
- */
-static inline bool iavf_virtchnl_done(struct iavf_adapter *adapter,
-				      bool (*condition)(struct iavf_adapter *, const void *),
-				      const void *cond_data,
-				      enum virtchnl_ops v_opcode)
-{
-	if (condition)
-		return condition(adapter, cond_data);
-
-	return adapter->current_op == v_opcode;
-}
-
-/**
- * iavf_poll_virtchnl_response - Poll admin queue for virtchnl response
- * @adapter: board private structure
- * @condition: optional callback to check if desired response received
- *   (takes priority)
- * @cond_data: context data passed to condition callback
- * @v_opcode: virtchnl opcode value to wait for if no condition configured
- *   (typically VIRTCHNL_OP_UNKNOWN), if condition, not used
- * @timeout_ms: maximum time to wait in milliseconds
- *
- * Polls admin queue and processes all messages until condition returns true
- * or timeout expires. If condition is NULL, waits for current_op to become
- * v_opcode (typically VIRTCHNL_OP_UNKNOWN after operation completes).
- * Caller must hold netdev_lock. This can sleep for up to timeout_ms while
- * polling hardware.
- *
- * Return: 0 on success (condition met), -EAGAIN on timeout or error
- */
-int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
-				bool (*condition)(struct iavf_adapter *, const void *),
-				const void *cond_data,
-				enum virtchnl_ops v_opcode,
-				unsigned int timeout_ms)
-{
-	struct iavf_hw *hw = &adapter->hw;
-	struct iavf_arq_event_info event;
-	enum virtchnl_ops v_op;
-	enum iavf_status v_ret;
-	unsigned long timeout;
-	u16 pending;
-	int ret;
-
-	netdev_assert_locked(adapter->netdev);
-
-	event.buf_len = IAVF_MAX_AQ_BUF_SIZE;
-	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
-	if (!event.msg_buf)
-		return -ENOMEM;
-
-	timeout = jiffies + msecs_to_jiffies(timeout_ms);
-	do {
-		if (iavf_virtchnl_done(adapter, condition, cond_data, v_opcode)) {
-			ret = 0;
-			goto out;
-		}
-
-		ret = iavf_clean_arq_element(hw, &event, &pending);
-		if (!ret) {
-			v_op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);
-			v_ret = (enum iavf_status)le32_to_cpu(event.desc.cookie_low);
-
-			iavf_virtchnl_completion(adapter, v_op, v_ret,
-						 event.msg_buf, event.msg_len);
-
-			memset(event.msg_buf, 0, IAVF_MAX_AQ_BUF_SIZE);
-
-			if (pending)
-				continue;
-		}
-
-		usleep_range(50, 75);
-	} while (time_before(jiffies, timeout));
-
-	if (iavf_virtchnl_done(adapter, condition, cond_data, v_opcode))
-		ret = 0;
-	else
-		ret = -EAGAIN;
-
-out:
-	kfree(event.msg_buf);
-	return ret;
-}
-- 
2.53.0

RE: [Intel-wired-lan] [PATCH net v3 2/5] i40e: skip unnecessary VF reset when setting trust

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Jose Ignacio Tornos Martinez
> Sent: Tuesday, April 14, 2026 1:00 PM
> To: netdev@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org; jesse.brandeburg@intel.com;
> Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Jose Ignacio
> Tornos Martinez <jtornosm@redhat.com>
> Subject: [Intel-wired-lan] [PATCH net v3 2/5] i40e: skip unnecessary
> VF reset when setting trust
> 
> 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

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [Intel-wired-lan] [PATCH net v3 5/5] iavf: refactor virtchnl polling into single function

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Jose Ignacio Tornos Martinez
> Sent: Tuesday, April 14, 2026 1:00 PM
> To: netdev@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org; jesse.brandeburg@intel.com;
> Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Jose Ignacio
> Tornos Martinez <jtornosm@redhat.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH net v3 5/5] iavf: refactor virtchnl
> polling into single function

For me it looks it should go to net-next as a refactoring.

> 
> At this moment, the driver has two separate functions for polling
> virtchnl messages from the admin queue:
> - iavf_poll_virtchnl_msg() for init-time (no timeout, no completion
>   handler)
> - iavf_poll_virtchnl_response() for runtime (with timeout, calls
>   completion)
> 
> Refactor by enhancing iavf_poll_virtchnl_msg() to handle both use
> cases:
> 1. Init-time mode (timeout_ms=0):
>   - Polls until matching opcode found or queue empty
>   - Returns raw message data without processing through completion
> handler
>   - Exits immediately on empty queue (no sleep/retry) 2. Runtime mode
> (timeout_ms>0):
>   - Polls with timeout using condition callback or opcode check
>   - Processes all messages through iavf_virtchnl_completion()
>   - Supports custom completion callback (takes priority) or falls back
>     to checking adapter->current_op against expected opcode
>   - Uses pending parameter to skip sleep when more messages queued
>   - Uses 50-75 usec sleep (due to commit 9e3f23f44f32 ("i40e: reduce
> wait
>     time for adminq command completion"))
> 
> By unifying message handling, both init-time and runtime messages can
> be processed through the completion handler when appropriate, ensuring
> consistent state updates and maintaining backward compatibility with
> all existing call sites.
> 
> Suggested-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf.h        |   9 +-
>  drivers/net/ethernet/intel/iavf/iavf_main.c   |  13 +-
>  .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 247 ++++++++---------

...

> --
> 2.53.0

RE: [Intel-wired-lan] [PATCH net v3 5/5] iavf: refactor virtchnl polling into single function

[Jose Ignacio Tornos Martinez]

Hello Aleksandr,

Thank you for your comments.
I wanted to link this patch in some way with patch 3/5, but you are right,
perhaps as a refactoring, better for net-next.
Anyway, I am going to wait for Przemek and
"iavf: add iavf_poll_virtchnl_response()" merge, after that I will rebase
and I will create another version of the series, dropping this for now.

Best regards
Jose Ignacio

Re: [PATCH net v3 4/5] ice: skip unnecessary VF reset when setting trust

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ice: skip unnecessary VF reset when setting trust

Similar to the i40e fix, ice_set_vf_trust() unconditionally calls
ice_reset_vf() when the trust setting changes. The ice driver already
has logic to clean up MAC LLDP filters when removing trust, which is
the only operation that requires filter synchronization. After this
cleanup, the VF reset is only necessary if there were actually filters
to remove.

> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -1399,14 +1399,19 @@ int ice_set_vf_trust(struct net_device *netdev, int vf_id, bool trusted)
>
>  	mutex_lock(&vf->cfg_lock);
>
> -	while (!trusted && vf->num_mac_lldp)
> -		ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), false);
> -
>  	vf->trusted = trusted;
> -	ice_reset_vf(vf, ICE_VF_RESET_NOTIFY);
>  	dev_info(ice_pf_to_dev(pf), "VF %u is now %strusted\n",
>  		 vf_id, trusted ? "" : "un");
>
> +	/* Only reset VF if removing trust and there are MAC LLDP filters
> +	 * to clean up. Reset is needed to ensure filter removal completes.
> +	 */
> +	if (!trusted && vf->num_mac_lldp) {
> +		while (vf->num_mac_lldp)
> +			ice_vf_update_mac_lldp_num(vf, ice_get_vf_vsi(vf), false);
> +		ice_reset_vf(vf, ICE_VF_RESET_NOTIFY);
> +	}
> +

Does setting vf->trusted before the cleanup loop break the filter
removal? Looking at the cleanup mechanism, ice_vf_update_mac_lldp_num()
calls ice_vf_is_lldp_ena() which checks both num_mac_lldp AND
vf->trusted:

ice_vf_lib.h:ice_vf_is_lldp_ena():
    return vf->num_mac_lldp && vf->trusted;

With the new ordering where vf->trusted is set to false before the loop:

ice_vf_update_mac_lldp_num() logic:
    was_ena = ice_vf_is_lldp_ena(vf);  // evaluates to (num > 0 && FALSE) = FALSE
    vf->num_mac_lldp--;
    is_ena = ice_vf_is_lldp_ena(vf);   // evaluates to FALSE

    if (was_ena != is_ena)              // FALSE != FALSE is FALSE
        ice_vsi_cfg_sw_lldp(vsi, false, false);  // NEVER called

But in the old code, vf->trusted was still TRUE during the loop:

    was_ena = (num > 0 && TRUE) = TRUE
    vf->num_mac_lldp--;
    is_ena = (0 && TRUE) = FALSE      // when last filter removed

    if (was_ena != is_ena)            // TRUE != FALSE is TRUE
        ice_vsi_cfg_sw_lldp(...)      // called to remove LLDP filters

So the ETH_P_LLDP packet filters are not removed from the hardware
switch, leaving stale filter rules. Should vf->trusted be set after the
cleanup loop instead?

>  	mutex_unlock(&vf->cfg_lock);
>
>  out_put_vf:

Re: [PATCH net v3 2/5] i40e: skip unnecessary VF reset when setting trust

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
i40e: skip unnecessary VF reset when setting trust

This commit skips the VF reset when setting VF trust, unless trust is
being removed from a VF with ADQ cloud filters. The goal is to avoid
a 10-second delay caused by the reset during trust changes.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index a26c3d47ec156..fea267af7afe6 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);

By skipping the VF reset here, does the VF's privilege capability flag
become permanently desynchronized from its trust state?

The underlying capability flag I40E_VIRTCHNL_VF_CAP_PRIVILEGE is used by
Virtchnl handlers like i40e_check_vf_permission() to authorize privileged
actions. This flag is exclusively updated in i40e_alloc_vf_res(), which is
only invoked during a VF reset.

Without the reset, it appears that granting trust fails to give the VF
actual privileged capabilities, and revoking trust leaves the privilege
bit set indefinitely, allowing an untrusted VF to perform unauthorized
privileged operations.

Additionally, could revoking trust fail to scrub the VF's actively
configured privileged hardware state, such as promiscuous mode and excess
MAC/VLAN filters?

Trusted VFs can explicitly enable promiscuous mode and configure MAC/VLAN
filters beyond strict limits. In the original code, i40e_vc_reset_vf()
dismantled these states by calling i40e_cleanup_reset_vf(), which
explicitly invokes i40e_config_vf_promiscuous_mode(..., false, false) and
destroys the VSI.

By skipping the reset, an untrusted VF retains its previously configured
promiscuous mode, allowing it to continue intercepting unauthorized traffic.

>  	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);

Is it possible that reordering i40e_del_all_cloud_filters() before
i40e_vc_reset_vf() creates a race condition on vf->cloud_filter_list?

i40e_del_all_cloud_filters() performs lockless modifications on
vf->cloud_filter_list while running from the netlink context.

In the original code, i40e_vc_reset_vf() ran first, which immediately
cleared I40E_VF_STATE_ACTIVE. This prevented concurrent Virtchnl requests
like VIRTCHNL_OP_ADD_CLOUD_FILTER from modifying the list, because their
handlers in i40e_vc_process_vf_msg() abort if I40E_VF_STATE_ACTIVE is
not set.

By calling i40e_del_all_cloud_filters() first, the VF is left in the
active state. The PF service task could concurrently process an
ADD_CLOUD_FILTER message, executing hlist_add_head() simultaneously with
hlist_for_each_entry_safe(), which might cause list corruption.

>  		}
>  	}
>

Re: [Intel-wired-lan] [PATCH net v3 5/5] iavf: refactor virtchnl polling into single function

[Jacob Keller]

On 4/15/2026 10:51 PM, Jose Ignacio Tornos Martinez wrote:
> Hello Aleksandr,
> 
> Thank you for your comments.
> I wanted to link this patch in some way with patch 3/5, but you are right,
> perhaps as a refactoring, better for net-next.

The cleanup makes sense as next material, but the other patches fix bugs
that could (should?) still target net, right?

> Anyway, I am going to wait for Przemek and
> "iavf: add iavf_poll_virtchnl_response()" merge, after that I will rebase
> and I will create another version of the series, dropping this for now.
> 
> Best regards
> Jose Ignacio
> 

I'll drop this version from the Intel Wired LAN patchwork then.

Thanks,
Jake

Re: [PATCH net v3 3/5] iavf: send MAC change request synchronously

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
iavf: send MAC change request synchronously

This commit attempts to fix a deadlock scenario during MAC address
changes by sending the ADD_ETH_ADDR message directly and polling the
admin queue synchronously, avoiding reliance on the watchdog task.

> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
[ ... ]
> -void iavf_add_ether_addrs(struct iavf_adapter *adapter)
> +int iavf_add_ether_addrs(struct iavf_adapter *adapter)
>  {
[ ... ]
>  	adapter->current_op = VIRTCHNL_OP_ADD_ETH_ADDR;
>  
>  	len = virtchnl_struct_size(veal, list, count);
[ ... ]
>  	veal = kzalloc(len, GFP_ATOMIC);
>  	if (!veal) {
>  		spin_unlock_bh(&adapter->mac_vlan_list_lock);
> -		return;
> +		return -ENOMEM;
>  	}

If kzalloc() fails here, the function returns -ENOMEM but leaves 
adapter->current_op set to VIRTCHNL_OP_ADD_ETH_ADDR. Does this permanently
prevent any future virtchnl operations from running, as they will all
immediately fail with -EBUSY checks?

> +int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
[ ... ]
> +	do {
[ ... ]
> +		ret = iavf_clean_arq_element(hw, &event, &pending);
> +		if (!ret) {
> +			v_op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);
> +			v_ret = (enum iavf_status)le32_to_cpu(event.desc.cookie_low);
> +
> +			iavf_virtchnl_completion(adapter, v_op, v_ret,
> +						 event.msg_buf, event.msg_len);

Unlike the legacy iavf_adminq_task() which explicitly checked for v_op == 0
before processing, this code appears to pass a 0 opcode directly to
iavf_virtchnl_completion() when fetching non-virtchnl admin queue events.

When v_op is 0, the completion handler hits the default switch case and
unconditionally clears adapter->current_op. Could this corrupt the state
machine by clearing the current operation state while a valid operation is
still pending?

[ ... ]
> +		usleep_range(50, 75);
> +	} while (time_before(jiffies, timeout));
> +
> +	if (iavf_virtchnl_done(adapter, condition, cond_data, v_opcode))
> +		ret = 0;
> +	else
> +		ret = -EAGAIN;
> +
> +out:
> +	kfree(event.msg_buf);
> +	return ret;
> +}

Similarly, if this polling loop times out, it returns -EAGAIN but does not
clear adapter->current_op. Will this also permanently block the control path
for the VF?

> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1047,6 +1047,46 @@ static bool iavf_is_mac_set_handled(struct net_device *netdev,
[ ... ]
> +static bool iavf_mac_change_done(struct iavf_adapter *adapter, const void *data)
> +{
> +	const u8 *addr = data;
> +
> +	return iavf_is_mac_set_handled(adapter->netdev, addr);
> +}

When promoting an existing secondary MAC to be the primary MAC,
iavf_replace_primary_mac() retrieves the existing filter which already has
f->add_handled set to true.

iavf_add_ether_addrs() sets f->add = false before dispatching. Because
iavf_is_mac_set_handled() checks (!f->add && f->add_handled), will this
cause iavf_mac_change_done() to immediately return true before the hardware
actually responds?

If so, this would cause iavf_set_mac() to prematurely evaluate the 
ether_addr_equal() check and fail with -EACCES.

> +static int iavf_set_mac_sync(struct iavf_adapter *adapter, const u8 *addr)
> +{
> +	int ret;
> +
> +	netdev_assert_locked(adapter->netdev);
> +
> +	ret = iavf_add_ether_addrs(adapter);
> +	if (ret)
> +		return ret;
> +
> +	return iavf_poll_virtchnl_response(adapter, iavf_mac_change_done, addr,
> +					   VIRTCHNL_OP_UNKNOWN, 2500);
> +}

If the MAC filter list has many pending filters, iavf_add_ether_addrs()
stops at IAVF_MAX_AQ_BUF_SIZE and sets "more = true", meaning the newly
requested MAC at the tail of the list might not be sent in the first chunk.

Since this function holds the netdev lock, the watchdog task cannot run to
send the remaining chunks. Does this cause the polling loop to wait
2.5 seconds and time out waiting for a MAC address that was never sent?

Additionally, this function exclusively dispatches iavf_add_ether_addrs().
If changing the MAC requires deleting the old one first, such as in
environments with a strict 1-MAC limit, the deletion is left for the
asynchronous watchdog. Does this cause the synchronous addition to fail
because the hardware rejects the new MAC before the old one is deleted?

> @@ -1067,26 +1107,13 @@ static int iavf_set_mac(struct net_device *netdev, void *p)
[ ... ]
>  	ret = iavf_replace_primary_mac(adapter, addr->sa_data);
> -
>  	if (ret)
>  		return ret;
>  
> -	ret = wait_event_interruptible_timeout(adapter->vc_waitqueue,
[ ... ]
> +	ret = iavf_set_mac_sync(adapter, addr->sa_data);
> +	if (ret)
>  		return ret;

If a background watchdog operation is active, iavf_set_mac_sync() will
immediately return -EBUSY because current_op is not VIRTCHNL_OP_UNKNOWN.

This causes iavf_set_mac() to return -EBUSY to the user. However,
iavf_replace_primary_mac() already successfully updated the software filter
lists. Won't the watchdog task eventually wake up and asynchronously change
the MAC address on the hardware, leaving the system in an inconsistent state
where a failed syscall still executes in the background?

Re: [Intel-wired-lan] [PATCH net v3 5/5] iavf: refactor virtchnl polling into single function

[Jose Ignacio Tornos Martinez]

Hello Jacob,

> The cleanup makes sense as next material, but the other patches fix bugs
> that could (should?) still target net, right?
Yes, I think so and that is my intention.
Unless Przemek has another opinion and/or better idea and since
"iavf: add iavf_poll_virtchnl_response()" has not yet landed in any public
repository, I can include "iavf: add iavf_poll_virtchnl_response()" patch
like it is in my next version (to drop when the commented one is landed)
and in the meantime allow to progress the rest of the patches for net.
I will take into account the interesting comments from Simon and his
AI-generated review too in my next version.
If it is ok, later on I can proceed with this patch for the refactoring.

> I'll drop this version from the Intel Wired LAN patchwork then.
Ok

Thanks

Best regards
Jose Ignacio

Re: [PATCH net v3 5/5] iavf: refactor virtchnl polling into single function

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
iavf: refactor virtchnl polling into single function

This commit consolidates the init-time and runtime virtchnl message polling
logic into a single enhanced iavf_poll_virtchnl_msg() function.

diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
[ ... ]
> +int iavf_poll_virtchnl_msg(struct iavf_hw *hw, struct iavf_arq_event_info *event,
> +			   enum virtchnl_ops op_to_poll, unsigned int timeout_ms,
> +			   bool (*condition)(struct iavf_adapter *, const void *),
> +			   const void *cond_data)
>  {
[ ... ]
> +	do {
> +		if (timeout_ms && iavf_virtchnl_completion_done(adapter, condition,
> +								cond_data, op_to_poll))
> +			return 0;
>  
> +		status = iavf_clean_arq_element(hw, event, &pending);
> +		if (status == IAVF_SUCCESS) {
> +			received_op = (enum virtchnl_ops)le32_to_cpu(event->desc.cookie_high);
>  
> +			/* Handle reset events specially */
> +			if (received_op == VIRTCHNL_OP_EVENT) {
> +				struct virtchnl_pf_event *vpe =
> +					(struct virtchnl_pf_event *)event->msg_buf;
>  
> +				if (vpe->event != VIRTCHNL_EVENT_RESET_IMPENDING)
> +					continue;

If received_op is VIRTCHNL_OP_EVENT but not VIRTCHNL_EVENT_RESET_IMPENDING,
the loop executes continue. Does this mean that during runtime polling (when
timeout_ms > 0), other asynchronous events like VIRTCHNL_EVENT_LINK_CHANGE
bypass iavf_virtchnl_completion() entirely and get discarded?

> +
> +				dev_info(&adapter->pdev->dev,
> +					 "Reset indication received from the PF\n");
> +				if (!(adapter->flags & IAVF_FLAG_RESET_PENDING))
> +					iavf_schedule_reset(adapter,
> +							    IAVF_FLAG_RESET_PENDING);
> +
> +				return -EIO;
> +			}
> +
> +			v_retval = le32_to_cpu(event->desc.cookie_low);
> +
> +			if (!timeout_ms) {
> +				if (received_op == op_to_poll)
> +					return virtchnl_status_to_errno((enum virtchnl_status_code)
> +							v_retval);
> +			} else {
> +				iavf_virtchnl_completion(adapter, received_op,
> +							 (enum iavf_status)v_retval,
> +							 event->msg_buf, event->msg_len);
> +			}
> +
> +			if (pending)
> +				continue;

When pending is true, the loop proceeds to the next iteration before executing
the memset buffer clearing code below. Might this leave stale data in
event->msg_buf for the next iavf_clean_arq_element() call if the subsequent
message is smaller than the previous one? The original implementation zeroed
the buffer before checking pending.

> +		} else if (!timeout_ms) {
> +			return iavf_status_to_errno(status);
> +		}
>  
> +		if (timeout_ms) {
> +			memset(event->msg_buf, 0, IAVF_MAX_AQ_BUF_SIZE);

The size passed to memset is hardcoded to IAVF_MAX_AQ_BUF_SIZE. If a caller
ever provides a timeout but passes a smaller buffer in the event structure,
could this write out of bounds?

For example, other callers like iavf_get_vf_supported_rxdids() use an 8-byte
stack variable for event->msg_buf with event->buf_len set to 8. Would it be
safer to use event->buf_len here instead of the hardcoded macro?

> +			usleep_range(50, 75);
>  		}
>  
> +	} while (!timeout_ms || time_before(jiffies, timeout));

Re: [PATCH net v3 3/5] iavf: send MAC change request synchronously

[Przemek Kitszel]

[-Jesse]

Thank you very much for working on this!
I see that we are going in good direction.
Please find some feedback inline.

> @@ -1067,26 +1107,13 @@ static int iavf_set_mac(struct net_device *netdev, void *p)
>   		return -EADDRNOTAVAIL;
>   
>   	ret = iavf_replace_primary_mac(adapter, addr->sa_data);
> -
>   	if (ret)
>   		return ret;
>   
> -	ret = wait_event_interruptible_timeout(adapter->vc_waitqueue,

this was the only waiter on this waitqueue, please remove it entriely

> -					       iavf_is_mac_set_handled(netdev, addr->sa_data),
> -					       msecs_to_jiffies(2500));
> -
> -	/* 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,
> -	 * check if netdev MAC was updated to requested MAC,
> -	 * if yes then set MAC succeeded otherwise it failed return -EACCES
> -	 */
> -	if (ret < 0)
> +	ret = iavf_set_mac_sync(adapter, addr->sa_data);
> +	if (ret)
>   		return ret;
>   
> -	if (!ret)
> -		return -EAGAIN;
> -
>   	if (!ether_addr_equal(netdev->dev_addr, addr->sa_data))
>   		return -EACCES;
>   

[..]

> +/**
> + * iavf_virtchnl_done - Check if virtchnl operation completed
> + * @adapter: board private structure
> + * @condition: optional callback for custom completion check
> + *   (takes priority)
> + * @cond_data: context data for callback
> + * @v_opcode: virtchnl opcode value we're waiting for if no condition
> + *   configured (typically VIRTCHNL_OP_UNKNOWN), if condition not used
> + *
> + * Checks completion status. Callback takes priority if provided. Otherwise
> + * waits for current_op to reach v_opcode (typically VIRTCHNL_OP_UNKNOWN
> + * after completion).
> + *
> + * Return: true if operation completed
> + */
> +static inline bool iavf_virtchnl_done(struct iavf_adapter *adapter,
> +				      bool (*condition)(struct iavf_adapter *, const void *),
> +				      const void *cond_data,
> +				      enum virtchnl_ops v_opcode)
> +{
> +	if (condition)
> +		return condition(adapter, cond_data);
> +
> +	return adapter->current_op == v_opcode;
> +}

after seeing this and patch 5, I think that the changes to combine the
two polling functions together are too big for "a preparation for fix"
type of change - so I agree with others that this should be scoped out
off this series

that stands for iavf_virtchnl_done() too - there is no caller that wants
"some opcode" in patches 1-4

and it will be possible to just pass "wanted_opcode" as the current 
param "const void *" of condition()

> +
> +/**
> + * iavf_poll_virtchnl_response - Poll admin queue for virtchnl response
> + * @adapter: board private structure
> + * @condition: optional callback to check if desired response received
> + *   (takes priority)
> + * @cond_data: context data passed to condition callback
> + * @v_opcode: virtchnl opcode value to wait for if no condition configured
> + *   (typically VIRTCHNL_OP_UNKNOWN), if condition, not used
> + * @timeout_ms: maximum time to wait in milliseconds
> + *
> + * Polls admin queue and processes all messages until condition returns true
> + * or timeout expires. If condition is NULL, waits for current_op to become
> + * v_opcode (typically VIRTCHNL_OP_UNKNOWN after operation completes).
> + * Caller must hold netdev_lock. This can sleep for up to timeout_ms while
> + * polling hardware.
> + *
> + * Return: 0 on success (condition met), -EAGAIN on timeout or error
> + */
> +int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
> +				bool (*condition)(struct iavf_adapter *, const void *),

please add v_op from below as a param

nit: I would also name the params instead of using plain types, not sure
how easy it will be for kdoc... (so no pressure for that)

> +				const void *cond_data,
> +				enum virtchnl_ops v_opcode,
> +				unsigned int timeout_ms)
> +{
> +	struct iavf_hw *hw = &adapter->hw;
> +	struct iavf_arq_event_info event;
> +	enum virtchnl_ops v_op;
> +	enum iavf_status v_ret;
> +	unsigned long timeout;
> +	u16 pending;
> +	int ret;
> +
> +	netdev_assert_locked(adapter->netdev);
> +
> +	event.buf_len = IAVF_MAX_AQ_BUF_SIZE;
> +	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
> +	if (!event.msg_buf)
> +		return -ENOMEM;
> +
> +	timeout = jiffies + msecs_to_jiffies(timeout_ms);
> +	do {
> +		if (iavf_virtchnl_done(adapter, condition, cond_data, v_opcode)) {
> +			ret = 0;
> +			goto out;
> +		}
> +
> +		ret = iavf_clean_arq_element(hw, &event, &pending);
> +		if (!ret) {
> +			v_op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);

comment about condition() signature:
I believe that condition() should take this @v_op

sidenote for patch5:
...@v_op instead of looking at adapter->current_op

> +			v_ret = (enum iavf_status)le32_to_cpu(event.desc.cookie_low);
> +
> +			iavf_virtchnl_completion(adapter, v_op, v_ret,
> +						 event.msg_buf, event.msg_len);
> +
> +			memset(event.msg_buf, 0, IAVF_MAX_AQ_BUF_SIZE);
> +
> +			if (pending)
> +				continue;

please incorporate the condition() check with iavf_clean_arq_element()
response (to avoid processing all subsequent VC messages if condition()
was met already)

it's fine to pass 0 as "v_op" to condition() when there is no VC msg yet

> +		}
> +
> +		usleep_range(50, 75);
> +	} while (time_before(jiffies, timeout));
> +
> +	if (iavf_virtchnl_done(adapter, condition, cond_data, v_opcode))
> +		ret = 0;
> +	else
> +		ret = -EAGAIN;

please change into just one call to condition(), and don't sleep between
the call and time_before() check (that will resolve my v2 concern)

> +
> +out:
> +	kfree(event.msg_buf);
> +	return ret;
> +}

Re: [Intel-wired-lan] [PATCH net v3 1/5] iavf: return EBUSY if reset in progress or not ready during MAC change

[Przemek Kitszel]

On 4/14/26 13:00, Jose Ignacio Tornos Martinez wrote:
> When a MAC address change is requested while the VF is resetting or still
> initializing, return -EBUSY immediately instead of attempting the
> operation.
> 
> Additionally, during early initialization states (before __IAVF_DOWN),
> the PF may be slow to respond to MAC change requests, causing long
> delays. Only allow MAC changes once the VF reaches __IAVF_DOWN state or
> later, when the watchdog is running and the VF is ready for operations.
> 
> After commit ad7c7b2172c3 ("net: hold netdev instance lock
> during sysfs operations"), MAC changes are called with the netdev lock
> held, so we should not wait with the lock held during reset or
> initialization. This allows the caller to retry or handle the busy state
> appropriately without blocking other operations.

that makes sense, but that could break user scripts, OTOH, with netdev
lock taken here, user could be blocked forever, so I think this is a net
positive change,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

> 
> 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..67aa14350b1b 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) || adapter->state < __IAVF_DOWN)
> +		return -EBUSY;
> +
>   	if (!is_valid_ether_addr(addr->sa_data))
>   		return -EADDRNOTAVAIL;
>