From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0B840C5AD49 for ; Wed, 4 Jun 2025 02:01:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=aUbro9mdyQRb2NWrgGe3dJfE6eyO+ffIZzcT1BI8qKc=; b=qOVkiD3A8mazKqoX7aIeyDoiBt 0cIinUhVxKFpqXiCj8Fmr/CXToGqkUjCUco7vUU5nxrFl9DY6F29vpLZNluSKo3WYXQIYuam9Qx4H nHLxTjtJiB9fK1j7lUCWxtDptBfJGNyPKm6SxvtwOkH1Ua38HrPtPhpJ150uIkCpeWPgsGIcOIKhA 3RiSGcVoMupdwljSkS32aElpvFAzOVXjYyqZ5gnG4enHxfZDiDxdegRLGu9F4R2PKrbkUZU7ZI0oT 4+cwLXwBZY1lJOg+8nc0Gz11slPioseC+C3iMwTUkaamCa3oSEexR1u2x5Po/eNOPR5i9OkoiF2JA QQz1XyZg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uMdRw-0000000CGZX-2i4E; Wed, 04 Jun 2025 02:01:52 +0000 Received: from sea.source.kernel.org ([172.234.252.31]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uMcSz-0000000C7hU-44JN for linux-arm-kernel@lists.infradead.org; Wed, 04 Jun 2025 00:58:55 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id ACD204A2D8; Wed, 4 Jun 2025 00:58:53 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B9695C4CEF1; Wed, 4 Jun 2025 00:58:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1748998733; bh=QRlrp2ySsyAfBDW6dhKKmQtWIXno/2j/eDC59JEqUFM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=n2B748en8BxtYopZ4JvT4J6NMv8R23DSwlJboskOQyb7t+dN3TdjxkY2MVliGw3a9 6L+A+2PskykBLLrSnIPwiN5l/hZ4vf0pR4fpecCP1vMEfJrahxbOKZzpP5hZ+wbgba N4Z0KkntvmD70N+s1lx6pfCe1tKXbGAAHwbeyuSDK5qu+rZ4Sd9n3s2gShe/ZDBVe0 cDWXm9x7FvR14AdHOPXys9NL2Yvolk8lKVQU60Xev8ZzBSmVxqZ/dYMmxHffKLwG06 /IjIUBpuV5oiPd93ihluOvpVYF0ZSVtiWc8uIUWjD4r4TebOWNZ0oyJYPPSZASg9i4 kFdH+AgJUYVrA== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Linus Walleij , Jakub Kicinski , Sasha Levin , ulli.kroll@googlemail.com, linux-arm-kernel@lists.infradead.org Subject: [PATCH AUTOSEL 6.14 099/108] net: ethernet: cortina: Use TOE/TSO on all TCP Date: Tue, 3 Jun 2025 20:55:22 -0400 Message-Id: <20250604005531.4178547-99-sashal@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250604005531.4178547-1-sashal@kernel.org> References: <20250604005531.4178547-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.14.9 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250603_175854_053003_9367B603 X-CRM114-Status: GOOD ( 28.41 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Linus Walleij [ Upstream commit 6a07e3af4973402fa199a80036c10060b922c92c ] It is desireable to push the hardware accelerator to also process non-segmented TCP frames: we pass the skb->len to the "TOE/TSO" offloader and it will handle them. Without this quirk the driver becomes unstable and lock up and and crash. I do not know exactly why, but it is probably due to the TOE (TCP offload engine) feature that is coupled with the segmentation feature - it is not possible to turn one part off and not the other, either both TOE and TSO are active, or neither of them. Not having the TOE part active seems detrimental, as if that hardware feature is not really supposed to be turned off. The datasheet says: "Based on packet parsing and TCP connection/NAT table lookup results, the NetEngine puts the packets belonging to the same TCP connection to the same queue for the software to process. The NetEngine puts incoming packets to the buffer or series of buffers for a jumbo packet. With this hardware acceleration, IP/TCP header parsing, checksum validation and connection lookup are offloaded from the software processing." After numerous tests with the hardware locking up after something between minutes and hours depending on load using iperf3 I have concluded this is necessary to stabilize the hardware. Signed-off-by: Linus Walleij Link: https://patch.msgid.link/20250408-gemini-ethernet-tso-always-v1-1-e669f932359c@linaro.org Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- **YES** This commit should be backported to stable kernel trees based on the following analysis: ## Critical Stability Fix This commit addresses a critical hardware stability issue where the Cortina Gemini ethernet driver would "lock up and crash" after minutes to hours of operation under load. The commit message explicitly states: > "Without this quirk the driver becomes unstable and lock up and and crash." > "After numerous tests with the hardware locking up after something between minutes and hours depending on load using iperf3 I have concluded this is necessary to stabilize the hardware." ## Code Analysis of Changes The changes are minimal and contained within the `gmac_map_tx_bufs` function in `drivers/net/ethernet/cortina/gemini.c`: 1. **TCP Detection Logic**: The code adds early TCP protocol detection: ```c bool tcp = false; if (skb->protocol == htons(ETH_P_IP)) tcp = (ip_hdr(skb)->protocol == IPPROTO_TCP); else tcp = (ipv6_hdr(skb)->nexthdr == IPPROTO_TCP); ``` 2. **TOE/TSO Activation for All TCP**: For non-GSO TCP frames, it now activates the hardware TOE/TSO engine: ```c } else if (tcp) { mss = netdev->mtu + skb_tcp_all_headers(skb); mss = min(mss, skb->len); word1 |= TSS_MTU_ENABLE_BIT; word3 |= mss; ``` 3. **Simplified Checksum Logic**: The checksum handling is simplified by reusing the already- determined `tcp` boolean instead of recalculating it. ## Hardware Architecture Justification The commit explains this is necessary due to the coupled nature of the hardware's TOE (TCP Offload Engine) and TSO features: > "it is probably due to the TOE (TCP offload engine) feature that is coupled with the segmentation feature - it is not possible to turn one part off and not the other" This aligns with the datasheet description that the NetEngine performs "IP/TCP header parsing, checksum validation and connection lookup" as integrated hardware acceleration. ## Pattern Match with Historical Commits Looking at the provided similar commits: - **Similar Commit #2**: "net: ethernet: cortina: Restore TSO support" (Status: YES) - This is the direct predecessor that restored TSO but didn't address the stability issue for non-segmented TCP frames - **Similar Commit #3**: "net: ethernet: cortina: Locking fixes" (Status: YES) - Another stability fix for the same driver Both historical commits addressing Cortina Gemini stability issues were marked as backport-worthy, establishing a pattern. ## Backport Criteria Assessment ✅ **Fixes important bugs**: Prevents driver crashes and hardware lockups ✅ **Small and contained**: Changes are localized to one function with clear logic ✅ **Minimal risk**: Only affects TCP frame handling, doesn't change architectural behavior ✅ **No new features**: Uses existing hardware capabilities more appropriately ✅ **Critical subsystem**: Network drivers are essential for system stability ## Risk Assessment The risk is very low because: - The changes only affect how TCP frames are processed by existing hardware features - The TOE/TSO hardware engine was already in use for segmented frames - The modification ensures consistent hardware state rather than leaving it partially enabled - The fix has been tested extensively by the author under load conditions This commit follows the stable tree criteria perfectly: it's an important bug fix that prevents system crashes, has minimal risk of regression, and doesn't introduce new features. drivers/net/ethernet/cortina/gemini.c | 37 +++++++++++++++++++++------ 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c index 991e3839858b5..3735a02e2f977 100644 --- a/drivers/net/ethernet/cortina/gemini.c +++ b/drivers/net/ethernet/cortina/gemini.c @@ -1143,6 +1143,7 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb, struct gmac_txdesc *txd; skb_frag_t *skb_frag; dma_addr_t mapping; + bool tcp = false; void *buffer; u16 mss; int ret; @@ -1150,6 +1151,13 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb, word1 = skb->len; word3 = SOF_BIT; + /* Determine if we are doing TCP */ + if (skb->protocol == htons(ETH_P_IP)) + tcp = (ip_hdr(skb)->protocol == IPPROTO_TCP); + else + /* IPv6 */ + tcp = (ipv6_hdr(skb)->nexthdr == IPPROTO_TCP); + mss = skb_shinfo(skb)->gso_size; if (mss) { /* This means we are dealing with TCP and skb->len is the @@ -1162,8 +1170,26 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb, mss, skb->len); word1 |= TSS_MTU_ENABLE_BIT; word3 |= mss; + } else if (tcp) { + /* Even if we are not using TSO, use the hardware offloader + * for transferring the TCP frame: this hardware has partial + * TCP awareness (called TOE - TCP Offload Engine) and will + * according to the datasheet put packets belonging to the + * same TCP connection in the same queue for the TOE/TSO + * engine to process. The engine will deal with chopping + * up frames that exceed ETH_DATA_LEN which the + * checksumming engine cannot handle (see below) into + * manageable chunks. It flawlessly deals with quite big + * frames and frames containing custom DSA EtherTypes. + */ + mss = netdev->mtu + skb_tcp_all_headers(skb); + mss = min(mss, skb->len); + netdev_dbg(netdev, "TOE/TSO len %04x mtu %04x mss %04x\n", + skb->len, netdev->mtu, mss); + word1 |= TSS_MTU_ENABLE_BIT; + word3 |= mss; } else if (skb->len >= ETH_FRAME_LEN) { - /* Hardware offloaded checksumming isn't working on frames + /* Hardware offloaded checksumming isn't working on non-TCP frames * bigger than 1514 bytes. A hypothesis about this is that the * checksum buffer is only 1518 bytes, so when the frames get * bigger they get truncated, or the last few bytes get @@ -1180,21 +1206,16 @@ static int gmac_map_tx_bufs(struct net_device *netdev, struct sk_buff *skb, } if (skb->ip_summed == CHECKSUM_PARTIAL) { - int tcp = 0; - /* We do not switch off the checksumming on non TCP/UDP * frames: as is shown from tests, the checksumming engine * is smart enough to see that a frame is not actually TCP * or UDP and then just pass it through without any changes * to the frame. */ - if (skb->protocol == htons(ETH_P_IP)) { + if (skb->protocol == htons(ETH_P_IP)) word1 |= TSS_IP_CHKSUM_BIT; - tcp = ip_hdr(skb)->protocol == IPPROTO_TCP; - } else { /* IPv6 */ + else word1 |= TSS_IPV6_ENABLE_BIT; - tcp = ipv6_hdr(skb)->nexthdr == IPPROTO_TCP; - } word1 |= tcp ? TSS_TCP_CHKSUM_BIT : TSS_UDP_CHKSUM_BIT; } -- 2.39.5