> -----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]>; > [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 > request > synchronously > > >-----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 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: [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 > > Przemek > > Kitszel review: the loop cannot work without polling between iterations > > since the second call would fail the current_op check. Multi-batch > > scenario > > is extremely rare; send first batch and let watchdog handle remainder 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_adapter > *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]>
