[Intel-wired-lan] [PATCH net v5 3/4] iavf: send MAC change request synchronously
Loktionov, Aleksandr
aleksandr.loktionov at intel.com
Wed Apr 29 11:28:25 UTC 2026
> -----Original Message-----
> From: Jose Ignacio Tornos Martinez <jtornosm at redhat.com>
> Sent: Wednesday, April 29, 2026 12:24 PM
> To: netdev at vger.kernel.org
> Cc: intel-wired-lan at lists.osuosl.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel at intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov at intel.com>; Keller, Jacob E
> <jacob.e.keller at intel.com>; horms at kernel.org;
> jesse.brandeburg at intel.com; Nguyen, Anthony L
> <anthony.l.nguyen at intel.com>; davem at davemloft.net;
> edumazet at google.com; kuba at kernel.org; pabeni at redhat.com; Jose Ignacio
> Tornos Martinez <jtornosm at redhat.com>; stable at vger.kernel.org
> Subject: [PATCH net v5 3/4] iavf: send MAC change request
> synchronously
>
> 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. Additionally,
> export iavf_mac_add_reject() to enable proper rollback on local
> failures (timeouts, send errors) - PF rejections are already handled
> automatically by iavf_virtchnl_completion().
>
> Remove vc_waitqueue entirely because iavf_set_mac was the only waiter
> on this waitqueue and after the changes it is not needed.
>
> Fixes: ad7c7b2172c3 ("net: hold netdev instance lock during sysfs
> operations")
> cc: stable at vger.kernel.org
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm at redhat.com>
> ---
> v5: Address the comments from Przemek Kitszel:
> - Add note in commit message about vc_waitqueue removal.
> - Change kdoc to use "Return:" instead of "Returns"
> - kdoc should end with '*/' not '**/' (new functions or with
> changes in the
> prototypes)
> - Sort lines from longest to shortest
> (iavf_poll_virtchnl_response)
> - Avoid "sleep then check time" (iavf_poll_virtchnl_response)
> Address AI review (sashiko.dev) from Simon Horman:
> - Restore adapter->hw.mac.addr on local failure (complete rollback
> in iavf_set_mac)
> - Remove timeout current_op clearing to prevent overlapping
> command
> race, the status can be controlled from outside and better to
> not
> corrupt it (iavf_poll_virtchnl_response) (as in v3).
> v4: https://lore.kernel.org/all/20260423130405.139568-4-
> jtornosm at redhat.com/
>
> drivers/net/ethernet/intel/iavf/iavf.h | 10 +-
> drivers/net/ethernet/intel/iavf/iavf_main.c | 71 +++++++++----
> .../net/ethernet/intel/iavf/iavf_virtchnl.c | 100 ++++++++++++++++-
> -
> 3 files changed, 151 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h
> b/drivers/net/ethernet/intel/iavf/iavf.h
> index e9fb0a0919e3..78fa3df06e11 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -260,7 +260,6 @@ struct iavf_adapter {
> struct work_struct adminq_task;
> struct work_struct finish_config;
> wait_queue_head_t down_waitqueue;
> - wait_queue_head_t vc_waitqueue;
> struct iavf_q_vector *q_vectors;
> struct list_head vlan_filter_list;
> int num_vlan_filters;
> @@ -589,8 +588,9 @@ 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);
...
> +/**
> + * iavf_poll_virtchnl_response - Poll admin queue for virtchnl
> response
> + * @adapter: adapter 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 the admin queue and processes all incoming virtchnl
> messages.
> + * After processing each valid message, calls the condition callback
> to
> +check
> + * if the expected response has been received. The callback receives
> +the opcode
> + * of the processed message to identify which response was received.
> +Continues
> + * polling until the callback returns true or timeout expires.
> + * 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
> +code */ int iavf_poll_virtchnl_response(struct iavf_adapter
> *adapter,
> + bool (*condition)(struct iavf_adapter
> *adapter,
> + const void *data,
> + enum virtchnl_ops v_op),
> + const void *cond_data,
> + unsigned int timeout_ms)
> +{
> + struct iavf_hw *hw = &adapter->hw;
> + struct iavf_arq_event_info event;
> + enum virtchnl_ops received_op;
> + unsigned long timeout;
> + int ret = -EAGAIN;
> + u16 pending = 0;
> + u32 v_retval;
> +
> + 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 (!pending)
> + usleep_range(50, 75);
> +
> + if (iavf_clean_arq_element(hw, &event, &pending) ==
> IAVF_SUCCESS) {
> + received_op = (enum
> virtchnl_ops)le32_to_cpu(event.desc.cookie_high);
> + if (received_op != VIRTCHNL_OP_UNKNOWN) {
> + v_retval =
> le32_to_cpu(event.desc.cookie_low);
> +
> + iavf_virtchnl_completion(adapter,
> received_op,
> + (enum
> iavf_status)v_retval,
> + event.msg_buf,
> event.msg_len);
> +
> + if (condition(adapter, cond_data,
> received_op)) {
> + ret = 0;
> + break;
> + }
> + }
> +
> + memset(event.msg_buf, 0, IAVF_MAX_AQ_BUF_SIZE);
> +
> + if (pending)
> + continue;
I think continue at the end of the cycle is redundant.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov at intel.com>
> + }
> + } while (time_before(jiffies, timeout));
> +
> + kfree(event.msg_buf);
> + return ret;
> +}
> --
> 2.53.0
More information about the Intel-wired-lan
mailing list