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

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Kwapulinski, Piotr
> Sent: Friday, June 26, 2026 5:04 PM
> To: Jose Ignacio Tornos Martinez <[email protected]>;
> [email protected]
> Cc: [email protected]; Kitszel, Przemyslaw
> <[email protected]>; Loktionov, Aleksandr
> <[email protected]>; Keller, Jacob E <[email protected]=
m>;
> [email protected]; Nguyen, Anthony L <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [Intel-wired-lan] [PATCH net v7 3/4] iavf: send MAC change r=
equest
> synchronously
>=20
> >-----Original Message-----
> >From: Intel-wired-lan <[email protected]> On Behalf Of
> >Jose Ignacio Tornos Martinez
> >Sent: Tuesday, June 23, 2026 12:18 PM
> >To: [email protected]
> >Cc: [email protected]; Kitszel, Przemyslaw
> ><[email protected]>; Loktionov, Aleksandr
> ><[email protected]>; Keller, Jacob E
> ><[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: [Intel-wired-lan] [PATCH net v7 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 alre=
ady held.
> >
> >The function queues a MAC address change request via
> >iavf_replace_primary_mac() and then waits for completion. However, in th=
e
> current flow, the actual virtchnl message is sent by the watchdog task, w=
hich also
> needs to acquire the netdev lock to run. Additionally, the adminq_task wh=
ich
> 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 se=
nt,
> 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 callbac=
k 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 i=
nstead
> of void, allowing callers to detect failures. Additionally, export
> iavf_mac_add_reject() to enable proper rollback on local failures (timeou=
ts, 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: [email protected]
> >Signed-off-by: Jose Ignacio Tornos Martinez <[email protected]>
> >---
> >v7: Rebase on current net tree
> >    Remove the multi-batch processing loop from version 6 according to P=
rzemek
> >    Kitszel review: the loop cannot work without polling between iterati=
ons
> >    since the second call would fail the current_op check. Multi-batch s=
cenario
> >    is extremely rare; send first batch and let watchdog handle remainde=
r as v5
> >    did
> >v6:
> >https://lore.kernel.org/all/[email protected]/
> >
> > drivers/net/ethernet/intel/iavf/iavf.h        | 11 ++-
> > drivers/net/ethernet/intel/iavf/iavf_main.c   | 85 ++++++++++++----
> > .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 99 +++++++++++++++++--
> > 3 files changed, 165 insertions(+), 30 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/intel/iavf/iavf.h
> >b/drivers/net/ethernet/intel/iavf/iavf.h
> >index 050f8241ef5e..5fcbfa0ca855 100644
> >--- a/drivers/net/ethernet/intel/iavf/iavf.h
> >+++ b/drivers/net/ethernet/intel/iavf/iavf.h
> >@@ -259,7 +259,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;
> >@@ -588,8 +587,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);
> >+int iavf_add_ether_addrs(struct iavf_adapter *adapter);
> > void iavf_del_ether_addrs(struct iavf_adapter *adapter);
> >+void iavf_mac_add_reject(struct iavf_adapter *adapter);
> > void iavf_add_vlans(struct iavf_adapter *adapter);  void iavf_del_vlans=
(struct
> iavf_adapter *adapter);  void iavf_set_promiscuous(struct iavf_adapter
> *adapter); @@ -606,6 +606,13 @@ void iavf_disable_vlan_stripping(struct
> iavf_adapter *adapter);  void iavf_virtchnl_completion(struct iavf_adapte=
r
> *adapter,
> > 			      enum virtchnl_ops v_opcode,
> > 			      enum iavf_status v_retval, u8 *msg, u16 msglen);
> >+int iavf_poll_virtchnl_response(struct iavf_adapter *adapter,
> >+				struct iavf_arq_event_info *event,
> >+				bool (*condition)(struct iavf_adapter *adapter,
> >+						  const void *data,
> >+						  enum virtchnl_ops v_op),
> >+				const void *cond_data,
> >+				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 630388e9d28c..3fa288e3798a 100644
> >--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> >+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> >@@ -1029,6 +1029,60 @@ static bool iavf_is_mac_set_handled(struct


Tested-by: Rafal Romanowski <[email protected]>