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

Loktionov, Aleksandr
2026-04-23T13:14:51Z

> -----Original Message-----
> From: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> Sent: Thursday, April 23, 2026 3:04 PM
> To: netdev@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; horms@kernel.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>; stable@vger.kernel.org
> 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: stable@vger.kernel.org
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
> 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-
> jtornosm@redhat.com/
>
> 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 <aleksandr.loktionov@intel.com>