From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B63E43EF0A0 for ; Thu, 23 Apr 2026 13:04:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776949488; cv=none; b=IQNCIsoQ36gI4qm5K6G16Q9JUFQAzvNmHLyoi6SUtNAhmCMz7P8AoTudC/CQooKWrzT3+wvDi46KbZmmVNRn0CcBaPcAeWRWFhE68NZnErXe6LYuUSiAJ6AFUCn2fUTZX4qA6QbvDShB7glh+vk66ybNg9RJG0T44JajoWYu2T4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776949488; c=relaxed/simple; bh=BSzO8xFcLyj57rG632x2OhPUV28zebLZxidCYJpcGk8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=OPcEQ9Dec8IbItF5M0/Thq9XfuT8OjK92FKrsc75TtICIP/dV1X1M1W+iL1FwZVKvXKWZdGxz6TkPyt0Pq7+2TzOFJ4Vvlu0Rp/DmLuXQg/teRUqPMwNEPTDRUtnvDxMsns16RXoqhgqndQ44o6Kv2MU3wWh69n8hsT3Qy8jOgM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=g4JP5yHB; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="g4JP5yHB" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1776949483; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vjdtH9kLkJw7JMDDM4w0dM9rLKPqBsQXKMj1MJx953E=; b=g4JP5yHBM3HzjAzlZ3PXYQ6+NDk0JVxhAUXQAjKFO1NBA96+UHN0ZefzgnOOKN9hNxc/aY mjWm5Aq8Wp2zUNrgItBVMDh1g15YU7n4aYduuw2PyV8VaspjCcLVbprQrvObRJEH5Dhvwc 2y2naAvfIFVx6vMP9kmPrHEOuCDiYUk= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-677-HdyM2jtlOQ6CL-txtnKIqA-1; Thu, 23 Apr 2026 09:04:40 -0400 X-MC-Unique: HdyM2jtlOQ6CL-txtnKIqA-1 X-Mimecast-MFC-AGG-ID: HdyM2jtlOQ6CL-txtnKIqA_1776949476 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 85AD61955F0B; Thu, 23 Apr 2026 13:04:36 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.44.32.35]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id AE8D819560AB; Thu, 23 Apr 2026 13:04:31 +0000 (UTC) From: Jose Ignacio Tornos Martinez To: netdev@vger.kernel.org Cc: intel-wired-lan@lists.osuosl.org, przemyslaw.kitszel@intel.com, aleksandr.loktionov@intel.com, jacob.e.keller@intel.com, horms@kernel.org, jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, Jose Ignacio Tornos Martinez , stable@vger.kernel.org Subject: [PATCH net v4 3/4] iavf: send MAC change request synchronously Date: Thu, 23 Apr 2026 15:04:04 +0200 Message-ID: <20260423130405.139568-4-jtornosm@redhat.com> In-Reply-To: <20260423130405.139568-1-jtornosm@redhat.com> References: <20260423130405.139568-1-jtornosm@redhat.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 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 --- 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; - 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); +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); @@ -607,6 +607,12 @@ 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, + 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 67aa14350b1b..bc5994bf2cd9 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -1047,6 +1047,48 @@ static bool iavf_is_mac_set_handled(struct net_device *netdev, return ret; } +/** + * iavf_mac_change_done - Check if MAC change completed + * @adapter: board private structure + * @data: MAC address being checked (as const void *) + * @v_op: virtchnl opcode from processed message + * + * Callback for iavf_poll_virtchnl_response() to check if MAC change completed. + * + * Returns true if MAC change completed, false otherwise + */ +static bool iavf_mac_change_done(struct iavf_adapter *adapter, + const void *data, enum virtchnl_ops v_op) +{ + const u8 *addr = data; + + return iavf_is_mac_set_handled(adapter->netdev, addr); +} + +/** + * iavf_set_mac_sync - Synchronously change MAC address + * @adapter: board private structure + * @addr: MAC address to set + * + * Sends MAC change request to PF and polls admin queue for response. + * Caller must hold netdev_lock. This can sleep for up to 2.5 seconds. + * + * Returns 0 on success, negative on failure + */ +static int iavf_set_mac_sync(struct iavf_adapter *adapter, const u8 *addr) +{ + int ret; + + netdev_assert_locked(adapter->netdev); + + ret = iavf_add_ether_addrs(adapter); + if (ret) + return ret; + + return iavf_poll_virtchnl_response(adapter, iavf_mac_change_done, + addr, 2500); +} + /** * iavf_set_mac - NDO callback to set port MAC address * @netdev: network interface device structure @@ -1067,25 +1109,20 @@ static int iavf_set_mac(struct net_device *netdev, void *p) return -EADDRNOTAVAIL; ret = iavf_replace_primary_mac(adapter, addr->sa_data); - if (ret) return ret; - ret = wait_event_interruptible_timeout(adapter->vc_waitqueue, - iavf_is_mac_set_handled(netdev, addr->sa_data), - msecs_to_jiffies(2500)); - - /* If ret < 0 then it means wait was interrupted. - * If ret == 0 then it means we got a timeout. - * else it means we got response for set MAC from PF, - * check if netdev MAC was updated to requested MAC, - * if yes then set MAC succeeded otherwise it failed return -EACCES - */ - if (ret < 0) + ret = iavf_set_mac_sync(adapter, addr->sa_data); + if (ret) { + /* Rollback for local failures (timeout, send error, -EBUSY). + * Note: If PF rejects the request (sends error response), + * iavf_virtchnl_completion() automatically calls + * iavf_mac_add_reject(), ret=0, and this is not executed. + * Only local failures (no PF response received) need manual rollback. + */ + iavf_mac_add_reject(adapter); return ret; - - if (!ret) - return -EAGAIN; + } if (!ether_addr_equal(netdev->dev_addr, addr->sa_data)) return -EACCES; @@ -5415,9 +5452,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) /* Setup the wait queue for indicating transition to down status */ init_waitqueue_head(&adapter->down_waitqueue); - /* Setup the wait queue for indicating virtchannel events */ - init_waitqueue_head(&adapter->vc_waitqueue); - INIT_LIST_HEAD(&adapter->ptp.aq_cmds); init_waitqueue_head(&adapter->ptp.phc_time_waitqueue); mutex_init(&adapter->ptp.aq_cmd_lock); diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c index a52c100dcbc5..d1afb8261c24 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c @@ -2,6 +2,7 @@ /* Copyright(c) 2013 - 2018 Intel Corporation. */ #include +#include #include "iavf.h" #include "iavf_ptp.h" @@ -555,20 +556,23 @@ iavf_set_mac_addr_type(struct virtchnl_ether_addr *virtchnl_ether_addr, * @adapter: adapter structure * * Request that the PF add one or more addresses to our filters. + * + * Return: 0 on success, negative on failure **/ -void iavf_add_ether_addrs(struct iavf_adapter *adapter) +int iavf_add_ether_addrs(struct iavf_adapter *adapter) { struct virtchnl_ether_addr_list *veal; struct iavf_mac_filter *f; int i = 0, count = 0; bool more = false; size_t len; + int ret; if (adapter->current_op != VIRTCHNL_OP_UNKNOWN) { /* bail because we already have a command pending */ dev_err(&adapter->pdev->dev, "Cannot add filters, command %d pending\n", adapter->current_op); - return; + return -EBUSY; } spin_lock_bh(&adapter->mac_vlan_list_lock); @@ -580,7 +584,7 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter) if (!count) { adapter->aq_required &= ~IAVF_FLAG_AQ_ADD_MAC_FILTER; spin_unlock_bh(&adapter->mac_vlan_list_lock); - return; + return 0; } adapter->current_op = VIRTCHNL_OP_ADD_ETH_ADDR; @@ -594,8 +598,9 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter) veal = kzalloc(len, GFP_ATOMIC); if (!veal) { + adapter->current_op = VIRTCHNL_OP_UNKNOWN; spin_unlock_bh(&adapter->mac_vlan_list_lock); - return; + return -ENOMEM; } veal->vsi_id = adapter->vsi_res->vsi_id; @@ -615,8 +620,15 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter) spin_unlock_bh(&adapter->mac_vlan_list_lock); - iavf_send_pf_msg(adapter, VIRTCHNL_OP_ADD_ETH_ADDR, (u8 *)veal, len); + ret = iavf_send_pf_msg(adapter, VIRTCHNL_OP_ADD_ETH_ADDR, (u8 *)veal, len); kfree(veal); + if (ret) { + dev_err(&adapter->pdev->dev, + "Unable to send ADD_ETH_ADDR message to PF, error %d\n", ret); + adapter->current_op = VIRTCHNL_OP_UNKNOWN; + } + + return ret; } /** @@ -713,7 +725,7 @@ static void iavf_mac_add_ok(struct iavf_adapter *adapter) * * Remove filters from list based on PF response. **/ -static void iavf_mac_add_reject(struct iavf_adapter *adapter) +void iavf_mac_add_reject(struct iavf_adapter *adapter) { struct net_device *netdev = adapter->netdev; struct iavf_mac_filter *f, *ftmp; @@ -2389,7 +2401,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter, iavf_mac_add_reject(adapter); /* restore administratively set MAC address */ ether_addr_copy(adapter->hw.mac.addr, netdev->dev_addr); - wake_up(&adapter->vc_waitqueue); break; case VIRTCHNL_OP_DEL_VLAN: dev_err(&adapter->pdev->dev, "Failed to delete VLAN filter, error %s\n", @@ -2586,7 +2597,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter, eth_hw_addr_set(netdev, adapter->hw.mac.addr); netif_addr_unlock_bh(netdev); } - wake_up(&adapter->vc_waitqueue); break; case VIRTCHNL_OP_GET_STATS: { struct iavf_eth_stats *stats = @@ -2956,3 +2966,76 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter, } /* switch v_opcode */ adapter->current_op = VIRTCHNL_OP_UNKNOWN; } + +/** + * 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. + * Clear current_op on timeout to prevent permanent -EBUSY state. + * 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; + u32 v_retval; + u16 pending; + int ret = -EAGAIN; + + 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 (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; + } + + usleep_range(50, 75); + } while (time_before(jiffies, timeout)); + + if (ret == -EAGAIN && adapter->current_op != VIRTCHNL_OP_UNKNOWN) + adapter->current_op = VIRTCHNL_OP_UNKNOWN; + + kfree(event.msg_buf); + return ret; +} -- 2.53.0