[{"id":3677200,"web_url":"http://patchwork.ozlabs.org/comment/3677200/","msgid":"<20260414132301.756968-1-horms@kernel.org>","list_archive_url":null,"date":"2026-04-14T13:23:00","subject":"Re: [Intel-wired-lan] [PATCH iwl-next v2 5/8] ixgbe: remove\n ixgbe_ping_all_vfs() from link state change handlers","submitter":{"id":82748,"url":"http://patchwork.ozlabs.org/api/people/82748/","name":"Simon Horman","email":"horms@kernel.org"},"content":"From: 'Simon Horman' \n\nThis is an AI-generated review of your patch. The human sending this\nemail has considered the AI review valid, or at least plausible.\nFull review at: https://sashiko.dev\n---\nixgbe: remove ixgbe_ping_all_vfs() from link state change handlers\n\nThis commit removes ixgbe_ping_all_vfs() from the link watchdog handlers\nto prevent a race condition during VF initialization where a gratuitous\nping collides with the VF mailbox state machine.\n\n> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c\n> index 8110bcf006a30..3071302901499 100644\n> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c\n> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c\n> @@ -8164,9 +8164,6 @@ static void ixgbe_watchdog_link_is_up(struct ixgbe_adapter *adapter)\n> \n> \t/* update the default user priority for VFs */\n> \tixgbe_update_default_up(adapter);\n> -\n> -\t/* ping all the active vfs to let them know link has changed */\n> -\tixgbe_ping_all_vfs(adapter);\n> }\n\nHow will legacy VFs (such as 82599, X540, and X550) receive link state\nchanges without these pings?\n\nThe commit message states that link-state changes are already communicated\nto VFs through the normal mailbox protocol. However, older VFs appear to\nrely on receiving an IXGBE_VT_MSGTYPE_CTS message from the PF to clear\nmac->get_link_status and complete the link up sequence in\nixgbevf_check_mac_link_vf().\n\nWithout the ping, will these legacy VFs fail to read the mailbox and stay\nstuck reporting the link as down?\n\nIs the mailbox corruption race still triggerable from other paths?\n\nIf ixgbe_ping_vf() is called locklessly from process-context NDO callbacks\nlike ixgbe_ndo_set_vf_link_state() or ixgbe_ndo_set_vf_trust(), could it be\npreempted by a hardirq triggering ixgbe_msg_task() (which correctly acquires\nvfs_lock before writing to the mailbox)?\n\nSince ixgbe_write_mbx_pf() only arbitrates between the PF and VF via the\nIXGBE_PFMAILBOX_PFU hardware lock, it provides no mutual exclusion between\nconcurrent host threads. Could concurrent writes corrupt the IXGBE_PFMBMEM\nbuffer and cause the exact same VF initialization failure this patch intended\nto fix?\n\nWould wrapping the pings in spin_lock_irqsave(&adapter->vfs_lock, flags)\nprevent the mailbox corruption instead of removing the pings entirely?\n\n[ ... ]\n\n> @@ -8197,9 +8194,6 @@ static void ixgbe_watchdog_link_is_down(struct ixgbe_adapter *adapter)\n> \n> \te_info(drv, \"NIC Link is Down\\n\");\n> \tnetif_carrier_off(netdev);\n> -\n> -\t/* ping all the active vfs to let them know link has changed */\n> -\tixgbe_ping_all_vfs(adapter);\n> }\n\nWithout this ping, will older VFs miss the mailbox interrupt needed to set\nmac->get_link_status = 1, causing them to remain stuck reporting the link as\nup indefinitely and silently drop traffic when the cable is unplugged?\n\nThis isn't a bug, but removing the calls from the link handlers appears to\nleave ixgbe_ping_all_vfs() as dead code without any remaining callers in\nthe kernel. Should the function definition in ixgbe_sriov.c and its\ndeclaration in ixgbe_sriov.h be removed as well?","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","intel-wired-lan@lists.osuosl.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","intel-wired-lan@lists.osuosl.org"],"Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=osuosl.org header.i=@osuosl.org header.a=rsa-sha256\n header.s=default header.b=a96K2WfB;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=osuosl.org\n (client-ip=140.211.166.137; helo=smtp4.osuosl.org;\n envelope-from=intel-wired-lan-bounces@osuosl.org;\n receiver=patchwork.ozlabs.org)"],"Received":["from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fw4jN02QQz1y2d\n\tfor ; Tue, 14 Apr 2026 23:23:34 +1000 (AEST)","from localhost (localhost [127.0.0.1])\n\tby smtp4.osuosl.org (Postfix) with ESMTP id C8A0442A35;\n\tTue, 14 Apr 2026 13:23:31 +0000 (UTC)","from smtp4.osuosl.org ([127.0.0.1])\n by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id 2putHzQrlPf3; Tue, 14 Apr 2026 13:23:31 +0000 (UTC)","from lists1.osuosl.org (lists1.osuosl.org [140.211.166.142])\n\tby smtp4.osuosl.org (Postfix) with ESMTP id 1645842A31;\n\tTue, 14 Apr 2026 13:23:31 +0000 (UTC)","from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137])\n by lists1.osuosl.org (Postfix) with ESMTP id 0B8D9283\n for ; Tue, 14 Apr 2026 13:23:30 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp4.osuosl.org (Postfix) with ESMTP id E5CA042A31\n for ; Tue, 14 Apr 2026 13:23:29 +0000 (UTC)","from smtp4.osuosl.org ([127.0.0.1])\n by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id XDGWuJq7uvCq for ;\n Tue, 14 Apr 2026 13:23:29 +0000 (UTC)","from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31])\n by smtp4.osuosl.org (Postfix) with ESMTPS id 275C94291B\n for ; Tue, 14 Apr 2026 13:23:28 +0000 (UTC)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n by sea.source.kernel.org (Postfix) with ESMTP id 8DC44438E4;\n Tue, 14 Apr 2026 13:23:28 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 461F5C19425;\n Tue, 14 Apr 2026 13:23:27 +0000 (UTC)"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections - client-ip=140.211.166.142;\n helo=lists1.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org;\n receiver= ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp4.osuosl.org 1645842A31","OpenDKIM Filter v2.11.0 smtp4.osuosl.org 275C94291B"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org;\n\ts=default; t=1776173011;\n\tbh=UHYbtxeZb8qWTq5FhDtjzGXR8mk+3VBFPV2mka5+pv4=;\n\th=From:To:Cc:Date:In-Reply-To:References:Subject:List-Id:\n\t List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:\n\t From;\n\tb=a96K2WfBU0JiFYyQog0CbzcOXbj/Fp197SxCNpsTfIRk1NEmsWjTIwNEsUr7ZyJYU\n\t eWR1fEhf/SIE/hXNffLUSAN81q+6cGoMgsVKipYK8AzBWzbIcPZy++m/GNMGvLJ1+f\n\t I6X0CoQ5SU/62vZQrKyn6ubBOIviwZPmAZQlC1vf+l3nvM0n+tQYmTNMhuRJcrLF/X\n\t cbwGV0M8GlBNyAAydbt//E1UfF7PqJ6bIyt0V3KsIFTdV14oI6t97c3rMk5BZYSlvr\n\t 5C5LGBip09t3b3+2R2UE5hTewXkAJU/3UydUtJ6BTmEzsbAkeOZSnUaMI/8PEgtqoR\n\t kv3Pc33vYUg9A==","Received-SPF":"Pass (mailfrom) identity=mailfrom; client-ip=172.234.252.31;\n helo=sea.source.kernel.org; envelope-from=horms@kernel.org;\n receiver=","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp4.osuosl.org 275C94291B","From":"Simon Horman ","To":"aleksandr.loktionov@intel.com","Cc":"'Simon Horman' , intel-wired-lan@lists.osuosl.org,\n anthony.l.nguyen@intel.com, netdev@vger.kernel.org","Date":"Tue, 14 Apr 2026 14:23:00 +0100","Message-ID":"<20260414132301.756968-1-horms@kernel.org>","X-Mailer":"git-send-email 2.53.0","In-Reply-To":"<20260408131216.2662245-6-aleksandr.loktionov@intel.com>","References":"<20260408131216.2662245-6-aleksandr.loktionov@intel.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-Mailman-Original-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n d=kernel.org; s=k20201202; t=1776173008;\n bh=i28YYCrUHHMDpraT0Z5aLv8IuBAQodP0RU10t8qP06M=;\n h=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n b=nziiY2cHF9SaKIleFtCmSzONxdI75XT7GwROAvqbs3etvAtGuC7zRg/p2mFMsMKID\n UKe0EpUVm/KmVEbZZjPNMUNYvh8XnzEmKQZSU+HX2ztEeQyt5xW4UuNQHqHVhk4bwZ\n Rm11fEs0gfl3m4BjdvxG2/sBJrwSE2dmu52Fx2MD5l0lec9TmmDE7hHeKCYVmXvWrH\n xz2ZHSXJsOg8Jbom2t+kdj/PVsfOs7tUG7U/4qlLmy9ac/dPl4dieg/xuiKxk2fBML\n USfeDJzFTmwVPqttkH4nKaF7Xb5H/FNrZlwEsIXgX4ZpPHLBX4kBwTgbzOHzxAXIYT\n a7DRHJZgXMwHQ==","X-Mailman-Original-Authentication-Results":["smtp4.osuosl.org;\n dmarc=pass (p=quarantine dis=none)\n header.from=kernel.org","smtp4.osuosl.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.a=rsa-sha256 header.s=k20201202 header.b=nziiY2cH"],"Subject":"Re: [Intel-wired-lan] [PATCH iwl-next v2 5/8] ixgbe: remove\n ixgbe_ping_all_vfs() from link state change handlers","X-BeenThere":"intel-wired-lan@osuosl.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"Intel Wired Ethernet Linux Kernel Driver Development\n ","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Errors-To":"intel-wired-lan-bounces@osuosl.org","Sender":"\"Intel-wired-lan\" "}}]