From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 6516526529A; Tue, 9 Dec 2025 00:17:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765239447; cv=none; b=H/zP1C5MvS0fREvNbUu+03CPHaV8+dL95WIXnQa8X1fLvo9dfirY8uC03zJb13hRFPFrg/DRudUqpmrsQErRYwuLcLVaBFEIjULEzGQDTjKu0ZsoEzGN3fMApblrgYG8466pDU60e/yMdqkYrffZHmyYShoJTVPQjoVc3GtQdPQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765239447; c=relaxed/simple; bh=NSvVx6PnzLFUc6PFMEvhuwqu/I7QZM68YLY7Ca8pH1M=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Cqx8IfsEfBWHm/Qqxgm8mSLdy0TBO5cGQ2FmC56YSiiyXtUdlJLKXa5+NpXx4Fr22P6NeayrOhhlqPrQnBNl0jvRoi2YEtsnWElPUsP1hLbjbKl0Jmy8gSwe+y6YmPvYwCQcVW7lrSqfUhDbfl0K6nDieui7jOhUAdrcFK8SW0A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=PIV1JXEW; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="PIV1JXEW" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 70C05C19421; Tue, 9 Dec 2025 00:17:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1765239447; bh=NSvVx6PnzLFUc6PFMEvhuwqu/I7QZM68YLY7Ca8pH1M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=PIV1JXEWJpkWYe+78yXMcKOSkQQgkIohEzrUv/SfQ2PJp+dvRRFbte/ex52JeZk4P z5A9Q3oqSx62Lcqd0xDTajN//zer/WaW8UU1AcnxBc6sIc1kA5nblnxiLkKGT4iDrF D4nWw2FBLlybiFPO4MqaYXgaMzkdjRym4UOEQESbxWxhdb2iZu2Z1nfEE3kURvmJR0 qI+z9fHKkvj/T3ArXC6n2jzmwRhW5JYgOztKOjF5+9A+ejDg3wdW1wrpEYwJUkCVB+ rYOysTwtp44tPA2tcWUuhOiChyZmRjJ81w7K0G935Tq8IisrpfaQXLt55bpXLtbaTW +czV5dvAi//5w== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Alok Tiwari , Simon Horman , Shyam Sundar S K , Jakub Kicinski , Sasha Levin , linux@armlinux.org.uk, netdev@vger.kernel.org Subject: [PATCH AUTOSEL 6.18-5.10] net: amd-xgbe: use EOPNOTSUPP instead of ENOTSUPP in xgbe_phy_mii_read_c45 Date: Mon, 8 Dec 2025 19:15:16 -0500 Message-ID: <20251209001610.611575-24-sashal@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20251209001610.611575-1-sashal@kernel.org> References: <20251209001610.611575-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.18 Content-Transfer-Encoding: 8bit From: Alok Tiwari [ Upstream commit bd853a59a87ecf9fc060dd54b10a3a94e9786182 ] The MDIO read callback xgbe_phy_mii_read_c45() can propagate its return value up through phylink_mii_ioctl() to user space via netdev ioctls such as SIOCGMIIREG. Returning ENOTSUPP results in user space seeing "Unknown error", since ENOTSUPP is not a standard errno value. Replace ENOTSUPP with EOPNOTSUPP to align with the MDIO core’s usage and ensure user space receives a proper "Operation not supported" error instead of an unknown code. Signed-off-by: Alok Tiwari Reviewed-by: Simon Horman Acked-by: Shyam Sundar S K Link: https://patch.msgid.link/20251015025751.1532149-1-alok.a.tiwari@oracle.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: Now I have all the information I need for a complete analysis. Let me compile my findings: ## Complete Analysis ### 1. COMMIT MESSAGE ANALYSIS **Subject:** `net: amd-xgbe: use EOPNOTSUPP instead of ENOTSUPP in xgbe_phy_mii_read_c45` **Key points from the commit message:** - The MDIO read callback `xgbe_phy_mii_read_c45()` can propagate its return value up through `phylink_mii_ioctl()` to **user space** via netdev ioctls (SIOCGMIIREG) - Returning ENOTSUPP (error code 524) results in **user space seeing "Unknown error"** since ENOTSUPP is not a standard errno value - The fix replaces ENOTSUPP (524, kernel-internal) with EOPNOTSUPP (95, standard POSIX errno) **Tags present:** - `Signed-off-by: Alok Tiwari ` - `Reviewed-by: Simon Horman ` - Important! This is a well-known networking reviewer - `Acked-by: Shyam Sundar S K ` - AMD maintainer acknowledgment - `Signed-off-by: Jakub Kicinski ` - Networking maintainer **Missing tags:** - No `Cc: stable@vger.kernel.org` - No `Fixes:` tag (though one should have been added: `Fixes: 070f6186a2f1d ("amd-xgbe: Separate C22 and C45 transactions")`) ### 2. CODE CHANGE ANALYSIS **The diff (single line change):** ```c - ret = -ENOTSUPP; + ret = -EOPNOTSUPP; ``` **Location:** `drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c`, function `xgbe_phy_mii_read_c45()` **Context Analysis:** The function handles MII (Media Independent Interface) Clause 45 read operations: ```c static int xgbe_phy_mii_read_c45(struct mii_bus *mii, int addr, int devad, int reg) { ... if (phy_data->conn_type == XGBE_CONN_TYPE_SFP) ret = -EOPNOTSUPP; // Already correct else if (phy_data->conn_type & XGBE_CONN_TYPE_MDIO) ret = xgbe_phy_mdio_mii_read_c45(pdata, addr, devad, reg); else ret = -ENOTSUPP; // BUG: should be -EOPNOTSUPP ``` **Root cause of the bug:** - Commit `070f6186a2f1d` ("amd-xgbe: Separate C22 and C45 transactions") introduced this function in January 2023 - Andrew Lunn correctly used `EOPNOTSUPP` for the SFP case - But **inconsistently/accidentally** used `ENOTSUPP` for the final else branch - This is clearly an oversight/typo during the refactoring **Technical explanation of ENOTSUPP vs EOPNOTSUPP:** - `ENOTSUPP` (524): Defined in `include/linux/errno.h` as a **kernel- internal error code** originally for NFSv3 protocol - `EOPNOTSUPP` (95): Defined in `include/uapi/asm-generic/errno.h` as a **standard POSIX errno** "Operation not supported" - When kernel code returns errors through syscalls/ioctls, it must use standard POSIX errno values - User-space `strerror()` doesn't know what errno 524 means → "Unknown error 524" ### 3. CLASSIFICATION - **Type:** Bug fix (incorrect error code returned to userspace) - **NOT a feature addition** - **NOT a device ID, quirk, or DT update** - This is fixing **incorrect API behavior** - returning a non-standard errno to userspace ### 4. SCOPE AND RISK ASSESSMENT **Size:** - 1 line changed - 1 file touched - **Minimal scope** **Risk:** - **EXTREMELY LOW** - This is a pure error code change - Cannot cause crashes, data corruption, or regressions - The error path itself is unchanged; only the error code returned differs - Changes from an unknown error (524) to a proper descriptive error (95) **Similar precedents:** I found multiple similar fixes already merged: - `f82acf6fb4211` - ixgbe: use EOPNOTSUPP instead of ENOTSUPP - `21d08d1c4c29f` - igc: use EOPNOTSUPP instead of ENOTSUPP - `bc73c5885c606` - igb: use EOPNOTSUPP instead of ENOTSUPP - `ab15aabac028a` - mtd: rawnand: qcom: Use EOPNOTSUPP instead of ENOTSUPP All these have `Fixes:` tags and address the same class of bug. ### 5. USER IMPACT **Who is affected:** - Users of AMD XGBE network devices (10Gb Ethernet) - Any tool or application using SIOCGMIIREG/SIOCGMIIPHY ioctls - Diagnostic tools like `mii-tool`, `ethtool`, or custom MDIO access utilities **Severity:** - When the hardware connection type is neither SFP nor MDIO, the user sees: - **Before fix:** "Unknown error 524" - confusing, hard to debug - **After fix:** "Operation not supported" - clear, standard message **Real-world impact:** - This is a **user-visible bug** affecting error reporting - While not a crash or data corruption, it degrades user experience - Makes debugging and troubleshooting harder ### 6. STABILITY INDICATORS - `Reviewed-by: Simon Horman` - Very experienced networking reviewer - `Acked-by: Shyam Sundar S K` - AMD driver maintainer - `Signed-off-by: Jakub Kicinski` - Net maintainer - The change is trivially correct - single character difference in error code ### 7. DEPENDENCY CHECK **Dependencies:** None - The change is self-contained - No other commits are required - The affected code exists in all stable kernels from 6.3 onwards (where `070f6186a2f1d` was merged) **Affected stable trees:** - 6.6.y: **Yes** - Contains the buggy commit, needs this fix - 6.10.y through 6.18.y: **Yes** - All affected - 6.1.y and earlier: **No** - The buggy code doesn't exist (function was introduced in 6.3) ### FINAL ASSESSMENT **Pros for backporting:** 1. ✅ Fixes a **real user-visible bug** - users see "Unknown error" messages 2. ✅ **Extremely small and surgical** - single 1-line change 3. ✅ **Obviously correct** - changing from kernel-internal errno to standard POSIX errno 4. ✅ **Zero risk** - cannot introduce regressions 5. ✅ **Well reviewed** - by networking and AMD maintainers 6. ✅ **Precedent exists** - similar fixes have been applied to stable trees 7. ✅ **No dependencies** - applies cleanly to all affected stable trees **Cons:** 1. ⚠️ No `Cc: stable@vger.kernel.org` tag - author didn't explicitly request stable backport 2. ⚠️ No `Fixes:` tag - though it clearly should have had one pointing to `070f6186a2f1d` 3. ⚠️ Minor severity - not a crash or security issue **Conclusion:** This commit fixes a clear bug where an incorrect kernel-internal error code (ENOTSUPP, 524) is returned to userspace instead of the proper POSIX errno (EOPNOTSUPP, 95). This causes user-space tools to display "Unknown error 524" instead of "Operation not supported", degrading user experience and making debugging harder. The fix is: - Trivially small (1 line) - Obviously correct - Zero risk of regression - Well-reviewed by subsystem experts - Has precedent with identical fixes in other drivers While the commit lacks explicit `Cc: stable` and `Fixes:` tags, it meets all the criteria from stable-kernel-rules.rst: it fixes "a real bug that bothers people", is small, obviously correct, and tested (reviewed by multiple maintainers). **YES** drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c index a56efc1bee339..35a381a83647d 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c @@ -668,7 +668,7 @@ static int xgbe_phy_mii_read_c45(struct mii_bus *mii, int addr, int devad, else if (phy_data->conn_type & XGBE_CONN_TYPE_MDIO) ret = xgbe_phy_mdio_mii_read_c45(pdata, addr, devad, reg); else - ret = -ENOTSUPP; + ret = -EOPNOTSUPP; xgbe_phy_put_comm_ownership(pdata); -- 2.51.0