[PATCH net v2 3/4] iavf: send MAC change request synchronously

[PATCH net v2 3/4] 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>
---
v2: Complete rewrite using synchronous polling approach instead of dropping
    the netdev lock. New approach:
    - Polls admin queue hardware directly (similar to iavf_reset_step)
    - Processes all virtchnl messages inline while holding netdev_lock
    - Introduced generic iavf_poll_virtchnl_response() for code reuse
    - No lock dropping, following accepted pattern from ndo_change_mtu fix
v1: https://lore.kernel.org/netdev/20260406112057.906685-4-jtornosm@redhat.com/

 drivers/net/ethernet/intel/iavf/iavf.h        |   2 +-
 drivers/net/ethernet/intel/iavf/iavf_main.c   | 118 +++++++++++++++---
 .../net/ethernet/intel/iavf/iavf_virtchnl.c   |  11 +-
 3 files changed, 110 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index e9fb0a0919e3..5bc23519fe9c 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);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 67aa14350b1b..2ef30b1ef35c 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1047,6 +1047,105 @@ static bool iavf_is_mac_set_handled(struct net_device *netdev,
 	return ret;
 }
 
+/**
+ * iavf_poll_virtchnl_response - Poll admin queue for virtchnl response
+ * @adapter: board private structure
+ * @condition: callback to check if desired response received
+ * @cond_data: context data passed to condition callback
+ * @timeout_ms: maximum time to wait in milliseconds
+ *
+ * Polls admin queue and processes all messages until condition returns true
+ * or timeout expires. Caller must hold netdev_lock. This can sleep for up to
+ * timeout_ms while polling hardware.
+ *
+ * Returns 0 on success (condition met), -EAGAIN on timeout or error
+ */
+static int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
+				       bool (*condition)(struct iavf_adapter *, void *),
+				       void *cond_data,
+				       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;
+	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);
+	while (time_before(jiffies, timeout)) {
+		if (condition(adapter, cond_data)) {
+			ret = 0;
+			goto out;
+		}
+
+		ret = iavf_clean_arq_element(hw, &event, NULL);
+		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);
+		}
+
+		usleep_range(1000, 2000);
+	}
+
+	ret = -EAGAIN;
+out:
+	kfree(event.msg_buf);
+	return ret;
+}
+
+/**
+ * iavf_mac_change_done - Check if MAC change completed
+ * @adapter: board private structure
+ * @data: MAC address being checked (as 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, 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 or error
+ */
+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,
+					   (void *)addr, 2500);
+}
+
 /**
  * iavf_set_mac - NDO callback to set port MAC address
  * @netdev: network interface device structure
@@ -1067,25 +1166,12 @@ 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)
-		return ret;
-
-	if (!ret)
-		return -EAGAIN;
+	ret = iavf_set_mac_sync(adapter, addr->sa_data);
+	if (ret)
+		return ret;
 
 	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..fdddf4b033ca 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -555,8 +555,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.
+ *
+ * Returns 0 on success or error
  **/
-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 +570,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 +582,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 +597,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 +619,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;
 }
 
 /**
-- 
2.53.0

Re: [Intel-wired-lan] [PATCH net v2 3/4] iavf: send MAC change request synchronously

[Przemek Kitszel]

On 4/7/26 18:52, 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 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>
> ---
> v2: Complete rewrite using synchronous polling approach instead of dropping
>      the netdev lock. New approach:
>      - Polls admin queue hardware directly (similar to iavf_reset_step)
>      - Processes all virtchnl messages inline while holding netdev_lock
>      - Introduced generic iavf_poll_virtchnl_response() for code reuse
>      - No lock dropping, following accepted pattern from ndo_change_mtu fix
> v1: https://lore.kernel.org/netdev/20260406112057.906685-4-jtornosm@redhat.com/
> 
>   drivers/net/ethernet/intel/iavf/iavf.h        |   2 +-
>   drivers/net/ethernet/intel/iavf/iavf_main.c   | 118 +++++++++++++++---
>   .../net/ethernet/intel/iavf/iavf_virtchnl.c   |  11 +-
>   3 files changed, 110 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
> index e9fb0a0919e3..5bc23519fe9c 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);
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 67aa14350b1b..2ef30b1ef35c 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1047,6 +1047,105 @@ static bool iavf_is_mac_set_handled(struct net_device *netdev,
>   	return ret;
>   }
>   
> +/**
> + * iavf_poll_virtchnl_response - Poll admin queue for virtchnl response
> + * @adapter: board private structure
> + * @condition: callback to check if desired response received
> + * @cond_data: context data passed to condition callback
> + * @timeout_ms: maximum time to wait in milliseconds
> + *
> + * Polls admin queue and processes all messages until condition returns true
> + * or timeout expires. Caller must hold netdev_lock. This can sleep for up to
> + * timeout_ms while polling hardware.
> + *
> + * Returns 0 on success (condition met), -EAGAIN on timeout or error
> + */
> +static int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
> +				       bool (*condition)(struct iavf_adapter *, void *),
> +				       void *cond_data,

this could be const, then no cast on the callsite

> +				       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;
> +	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);
> +	while (time_before(jiffies, timeout)) {
> +		if (condition(adapter, cond_data)) {

if condition is met, but timed out, there should be no error

> +			ret = 0;
> +			goto out;
> +		}
> +
> +		ret = iavf_clean_arq_element(hw, &event, NULL);
> +		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);
> +		}
> +
> +		usleep_range(1000, 2000);

no sleep after message received (ok to do on empty queue)

> +	}
> +
> +	ret = -EAGAIN;
> +out:
> +	kfree(event.msg_buf);
> +	return ret;
> +}
> +
> +/**
> + * iavf_mac_change_done - Check if MAC change completed
> + * @adapter: board private structure
> + * @data: MAC address being checked (as 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, 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 or error
> + */
> +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,
> +					   (void *)addr, 2500);

this function looks elegant, thank you

I'm a little affraid that this model (if applied to other things than
setting MAC) will skip some of our "much needed" logic in the watchdog.

I have not thinked about much yet.

unrelated: callback looks elegant, but for virtchnl, it is almost always
the case that we wait for some VC OPCODE to come back, and this is just
a number. It could be easily coded as a callback too, passing wanted
value masked in pointer, but I would say that just passing a normal u32
param will be most clean

Re: [Intel-wired-lan] [PATCH net v2 3/4] iavf: send MAC change request synchronously

[Jose Ignacio Tornos Martinez]

Hello Przemek,

Thank you for your comments.
I will try to include them in a next version.

Best regards
Jose Ignacio

Re: [Intel-wired-lan] [PATCH net v2 3/4] iavf: send MAC change request synchronously

[Przemek Kitszel]

I believe this is a move in right direction,
please also find some more comments

>> +/**
>> + * iavf_poll_virtchnl_response - Poll admin queue for virtchnl response
>> + * @adapter: board private structure
>> + * @condition: callback to check if desired response received
>> + * @cond_data: context data passed to condition callback
>> + * @timeout_ms: maximum time to wait in milliseconds
>> + *
>> + * Polls admin queue and processes all messages until condition 
>> returns true
>> + * or timeout expires. Caller must hold netdev_lock. This can sleep 
>> for up to
>> + * timeout_ms while polling hardware.
>> + *
>> + * Returns 0 on success (condition met), -EAGAIN on timeout or error

kdoc requires "Return:"

>> + */
>> +static int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,

please move it to iavf_virtchnl.[ch], could be useful for other also,
not need to move it then and loose your authorship in default blame view

one more thing is that this could be perhaps integrated with existing
iavf_poll_virtchnl_msg(), which does not call
iavf_virtchnl_completion(), but likely could

>> +                       bool (*condition)(struct iavf_adapter *, void *),
>> +                       void *cond_data,
> 
> this could be const, then no cast on the callsite
> 
>> +                       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;
>> +    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);
>> +    while (time_before(jiffies, timeout)) {

please consider do-while (after the rest of changes could be better)

>> +        if (condition(adapter, cond_data)) {
> 
> if condition is met, but timed out, there should be no error
> 
>> +            ret = 0;
>> +            goto out;
>> +        }
>> +
>> +        ret = iavf_clean_arq_element(hw, &event, NULL);

instead of NULL pass "pending" param, if not zero you could omit
next sleep

>> +        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);
>> +        }
>> +
>> +        usleep_range(1000, 2000);

very old commit 9e3f23f44f32 ("i40e: reduce wait time for adminq command
completion") said that 50usec is right amount to sleep between checks

> 
> no sleep after message received (ok to do on empty queue)
> 
>> +    }
>> +
>> +    ret = -EAGAIN;
>> +out:
>> +    kfree(event.msg_buf);
>> +    return ret;
>> +}
>> +
>> +/**
>> + * iavf_mac_change_done - Check if MAC change completed
>> + * @adapter: board private structure
>> + * @data: MAC address being checked (as 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, 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 or error
>> + */
>> +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,
>> +                       (void *)addr, 2500);
> 
> this function looks elegant, thank you
> 
> I'm a little affraid that this model (if applied to other things than
> setting MAC) will skip some of our "much needed" logic in the watchdog.
> 
> I have not thinked about much yet.
> 
> unrelated: callback looks elegant, but for virtchnl, it is almost always
> the case that we wait for some VC OPCODE to come back, and this is just
> a number. It could be easily coded as a callback too, passing wanted
> value masked in pointer, but I would say that just passing a normal u32
> param will be most clean
> 
> 

Re: [Intel-wired-lan] [PATCH net v2 3/4] iavf: send MAC change request synchronously

[Jose Ignacio Tornos Martinez]

Hello Przemek,

Thank you again for your comments.
I will try to include the new ones too in the next version.
In addition, I will analyze the suggested integration with existing
iavf_poll_virtchnl_msg() and possibly iavf_virtchnl_completion, but maybe
better in a new patch of the series as a refactoring.

Best regards
Jose Ignacio