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 20F4EC47073 for ; Fri, 29 Dec 2023 16:12:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:Message-ID: In-Reply-To:Subject:cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=uvyrzTQiGHLIfzAqV0m15KSnmS4TBS6dezylIKcSABw=; b=L4C/KfI0lGoFMa qU3gA2AXXd+akvi/qIHPpL6JobV/LMuw8nDnkbs1TZNoAuAgTYDRs5fJKrj5/dU81fh3C/pPG/WIu VDBiMIwdzRsbhaBldG6R39H5GSOoWgMpqftYlV1rbf6eMai5kQsFE6bqO8MYvr+T1Z3r/ad2P09HO ffCZs7KkpW5p1xrildmcxvHRGSd6gMrDvCg4BNbB/E69YKl3iV6UoGuFa2nCPmKvX8gIsnfDUVpAQ xF4Hbb7nSD5udkikSYEVoMv4oi3vT+Kxnc+q72c7J9Dy3nEmAvSRdLa+Qsd/wsa4o5V6mgnLEdlnm htmrX2H2H9BMIzgeaiRA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rJFST-001A2I-36; Fri, 29 Dec 2023 16:11:37 +0000 Received: from relay1-d.mail.gandi.net ([217.70.183.193]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rJFSQ-001A1j-1s for linux-arm-kernel@lists.infradead.org; Fri, 29 Dec 2023 16:11:36 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id BF78A240002; Fri, 29 Dec 2023 16:11:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1703866288; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=zqi8dmMeOU3XDtyNyrJLxgovOsvnL1n+HnUCWjiryzI=; b=mzl0h77RyMj6S5QimPrixX8KNRjlHePNB2ZkUiW1I9hnJLejc/VQzGFm8nS326dqZfS/xl bKxX9mx4sqb41UzWjA7WEILOygNJBiY3aF+fLvRNRIRx6piDkxSq6D6QjpfZWfHUaUXoBn EGqUEC7N6KQLY+EEXZVaq4glUhZGUMhpeh2/L3dqd4kzSptktj4f0Xq+V/7sLwXJyw9yrw 8LbIR34NmLppvtiNnUyC1RlZoVp65QhK2CNsUmkWp6d8A1MGIWlQyrzRCrJ3lpFeChxtTS uB/RNEHBgtTq2yrcgaWItC+6TVZK+2vqh7awbOTGv4iN1prJ8kR1js1/I9Mx5g== Date: Fri, 29 Dec 2023 17:11:48 +0100 (CET) From: Romain Gantois To: Vladimir Oltean cc: Romain Gantois , Alexandre Torgue , Jose Abreu , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , Miquel Raynal , Maxime Chevallier , Sylvain Girard , Pascal EBERHARD , Richard Tresidder , netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org Subject: Re: [PATCH net 1/1] net: stmmac: Prevent DSA tags from breaking COE In-Reply-To: <20231219122034.pg2djgrosa4irubh@skbuf> Message-ID: <3b53aa8a-73e9-9260-f05b-05dac80a4276@bootlin.com> References: <20231218162326.173127-1-romain.gantois@bootlin.com> <20231218162326.173127-2-romain.gantois@bootlin.com> <20231219122034.pg2djgrosa4irubh@skbuf> MIME-Version: 1.0 X-GND-Sasl: romain.gantois@bootlin.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231229_081134_910245_8D9CEAE2 X-CRM114-Status: GOOD ( 19.98 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 19 Dec 2023, Vladimir Oltean wrote: > DSA_TAG_PROTO_LAN9303, DSA_TAG_PROTO_SJA1105 and DSA_TAG_PROTO_SJA1110 > construct tags with ETH_P_8021Q as EtherType. Do you still think it > would be correct to say that all DSA tags break COE on the stmmac, as > this patch assumes? > > The NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM convention is not about > statically checking whether the interface using DSA, but about looking > at each packet before deciding whether to use the offload engine or to > call skb_checksum_help(). > > You can experiment with any tagging protocol on the stmmac driver, and > thus with the controller's response to any kind of traffic, even if the > port is not attached to a hardware switch. You need to enable the Thanks for telling me about DSA_LOOP, I've tested several DSA tagging protocols with the RZN1 GMAC1 hardware using this method. Here's what I found in a nutshell: For tagging protocols that change the EtherType field in the MAC header (e.g. DSA_TAG_PROTO_(DSA/EDSA/BRCM/MTK/RTL4C_A/SJA1105): On TX the tagged frames are almost always ignored by the checksum offload engine and IP header checker of the MAC device. I say "almost always" because there is an unlikely but nasty corner case where a DSA tag can be identical to an IP EtherType value. In these cases, the frame will likely fail IP header checks and be dropped by the MAC. Ignoring these corner cases, the DSA frames will egress with a partial checksum and be dropped by the recipient. On RX, these frames will, once again, not be detected as IP frames by the MAC. So they will be transmitted to the CPU. However, the stmmac driver will assume (wrongly in this case) that these frames' checksums have been verified by the MAC. So it will set CHECKSUM_UNECESSARY: https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L5493 And so the IP/TCP checksums will not be checked at all, which is not ideal. There are other DSA tagging protocols which cause different issues. For example DSA_TAG_PROTO_BRCM_PREPEND, which seems to offset the whole MAC header, and DSA_TAG_PROTO_LAN9303 which sets ETH_P_8021Q as its EtherType. I haven't dug too deeply on these issues yet, since I'd rather deal with the checksumming issue before getting distracted by VLAN offloading and other stuff. Among the tagging protocols I tested, the only one that didn't cause any issues was DSA_TAG_PROTO_TRAILER, which only appends stuff to the frame. TLDR: The simplest solution seems to be to modify the stmmac TX and RX paths to disable checksum offloading for frames that have a non-IP ethertype in their MAC header. This will fix the checksum situation for DSA tagging protocols that set non-IP and non-8021Q EtherTypes. Some edge cases like DSA_TAG_PROTO_BRCM_PREPEND and DSA_TAG_PROTO_LAN9303 will require a completely different solution if we want these MAC devices to handle them properly. Please share any thoughts you might have on this suggestion. Best Regards, -- Romain Gantois, Bootlin Embedded Linux and Kernel engineering https://bootlin.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel