> -----Original Message-----
> From: Jose Ignacio Tornos Martinez <[email protected]>
> Sent: Thursday, April 23, 2026 3:04 PM
> To: [email protected]
> Cc: [email protected]; Kitszel, Przemyslaw
> <[email protected]>; Loktionov, Aleksandr
> <[email protected]>; Keller, Jacob E
> <[email protected]>; [email protected];
> [email protected]; Nguyen, Anthony L
> <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Jose Ignacio
> Tornos Martinez <[email protected]>; [email protected]
> Subject: [PATCH net v4 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().
> 
> Fixes: ad7c7b2172c3 ("net: hold netdev instance lock during sysfs
> operations")
> cc: [email protected]
> Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> ---
> v4: Complete with Przemek Kitszel comments:
>     - Remove vc_waitqueue entirely (not needed any more)
>     - Add named parameters to callback function pointer declaration
> for
>       clarity
>     - Simplify callback signature: add v_op parameter so callback
>       receives the opcode from the processed message to identify which
>       response was received
>     - Optimize polling loop to single condition check per iteration
>       instead of checking both before and after message processing
>     Address AI review (sashiko.dev) from Simon Horman:
>     - Complete iavf_add_ether_addrs() error handling
>     - Skip non-virtchnl hardware events
> (received_op=VIRTCHNL_OP_UNKNOWN),
>       these can cause false completion detection
>     - Complete rollback for local failures (not PF rejection) reusing
>       iavf_mac_add_reject() to restore the old primary filter
> v3: https://lore.kernel.org/netdev/20260414110006.124286-4-
> [email protected]/
> 
>  drivers/net/ethernet/intel/iavf/iavf.h        | 10 +-
>  drivers/net/ethernet/intel/iavf/iavf_main.c   | 70 +++++++++----
>  .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 99 +++++++++++++++++-
> -
>  3 files changed, 151 insertions(+), 28 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;

...

> --
> 2.53.0


Reviewed-by: Aleksandr Loktionov <[email protected]>

Reply via email to