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 B7C23C41535 for ; Tue, 19 Dec 2023 12:21:14 +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:In-Reply-To:MIME-Version:References: Message-ID: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=xEYYqelYXZleAb62GxOVm7hBX3Cb5esUe+c0lBfivWI=; b=hmPV8cDPSdhkIF C0wssCQbgne8Nv2kVc1xK5lr9EdtcdLpjPPC2HmUSN4jcLtxdc7XHuAxkBLOdeq1MegPGx2PACCNS DQlTTaBqhFpfiz0DyN/oq2x8efaHk6EQktK6WbMHxmn7KFxBeNnIpLQPLsOaFwArhKrFlbtdpX8iT DdFowWc4InahX0DYZCg4ukVVrPsVeu2wQIiWOQic6lEPERd1Wh6kCx4itV6gWwrizrOJxFwqTVbSZ EaId0CtHhcPmaqqd+JyW/ActmQEhKXN00P/nm8u2Ps964FnvyY7IlpACH8cO7njfVHHdwZSkhMgpp ixZ+MxOFhxaiDWrzZQiA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rFZ5W-00DzdO-0D; Tue, 19 Dec 2023 12:20:42 +0000 Received: from mail-ej1-x630.google.com ([2a00:1450:4864:20::630]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rFZ5T-00Dzc7-1u for linux-arm-kernel@lists.infradead.org; Tue, 19 Dec 2023 12:20:41 +0000 Received: by mail-ej1-x630.google.com with SMTP id a640c23a62f3a-a1fae88e66eso499249866b.3 for ; Tue, 19 Dec 2023 04:20:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702988437; x=1703593237; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=l6TxmZzSrla4vvXDQC7Cn74njvru9m/j+6El3HghLS4=; b=hfebOlw13eajrTFKqLofY4bDGMvpkhKq6xPcg+OvEKirO3D3rN/PaC6JXEpmVrA7Lq sQ4VwYFQBNElKZ5Ruixha1kGtn5dYxOjJazgnw4o1qB2/xtcTl5gFGqq1+ExwZoZGFlf CIJ5AwsdAi0Id+/UdOc/W4Bkpyt1VwYv5IlEKjUGAREqBS4wTc8nnT6hZXn8hcEjeTKX dAOUvdbTVdjV+u/R4hXITiLpPRQBq4Z/Gyiru5MXce31wRdd+XJZNZMn9z3Lmv4iutmt 1mfiN/H8feRdd/v+DDq/YLiXQCbe0cJVSzKGUqlZDotGrEHdI2w5a6AumQoRLUW3PLja 8zkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702988437; x=1703593237; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=l6TxmZzSrla4vvXDQC7Cn74njvru9m/j+6El3HghLS4=; b=TsedofMwIiqRullZS6zrE59OcwFrchTNLlrf2hACvz9T+z3+7zcnDZJia3+Zruq4bt ODMHR5ZMbVmrkUOYlRa1F7NprczMSJrY0upJ49eivi1eRn0oVUGr5cnSnutONAjIsAWf nfE1hSgFIeGhvObzckwi7zY/gGERaGT+nfiI6M0FboHtET6fZ8FV8DaqnyI1kOshhGI7 mvesi7YZmUqGlddkFEmyWr+6Hh5BWMzun/VrSqf9E/GUEaY/s4YAzEg9Pd/jyKvzFCjc IqbZc3bTRGkrU/aI1ZZTePlA6rKynKRRB5Q2OavKyAAdBJPonC9vAwYlZYdHLxEPtj80 YczA== X-Gm-Message-State: AOJu0Yzqs3+q24BeguQtHvlRPypA+2ILJ8dG2y1VWrE2i1MLY55GBfUn 7JLmPH6OLAkLc9ayUv4gUC0= X-Google-Smtp-Source: AGHT+IEF85zDgPhfMUFpTsLoXGcLUVBLjiYkSXZm9jyM0zEWCF6zNqlV9cTbyQPPvo84zRJXC5qRFg== X-Received: by 2002:a17:906:b0d4:b0:a1d:7792:cdbe with SMTP id bk20-20020a170906b0d400b00a1d7792cdbemr7007558ejb.146.1702988437094; Tue, 19 Dec 2023 04:20:37 -0800 (PST) Received: from skbuf ([188.27.185.68]) by smtp.gmail.com with ESMTPSA id lm11-20020a17090718cb00b00a1cf3fce937sm15284866ejc.162.2023.12.19.04.20.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Dec 2023 04:20:36 -0800 (PST) Date: Tue, 19 Dec 2023 14:20:34 +0200 From: Vladimir Oltean To: Romain Gantois Cc: 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 Message-ID: <20231219122034.pg2djgrosa4irubh@skbuf> References: <20231218162326.173127-1-romain.gantois@bootlin.com> <20231218162326.173127-2-romain.gantois@bootlin.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231218162326.173127-2-romain.gantois@bootlin.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231219_042039_627591_E58BFD9E X-CRM114-Status: GOOD ( 31.61 ) 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 Hi Romain, On Mon, Dec 18, 2023 at 05:23:23PM +0100, Romain Gantois wrote: > Some stmmac cores have Checksum Offload Engines that cannot handle DSA tags > properly. These cores find the IP/TCP headers on their own and end up > computing an incorrect checksum when a DSA tag is inserted between the MAC > header and IP header. > > Add an additional check on stmmac link up so that COE is deactivated > when the stmmac device is used as a DSA conduit. > > Add a new dma_feature flag to allow cores to signal that their COEs can't > handle DSA tags on TX. > > Fixes: 6b2c6e4a938f ("net: stmmac: propagate feature flags to vlan") > Cc: stable@vger.kernel.org > Reported-by: Richard Tresidder > Closes: https://lore.kernel.org/netdev/e5c6c75f-2dfa-4e50-a1fb-6bf4cdb617c2@electromag.com.au/ > Reported-by: Romain Gantois > Closes: https://lore.kernel.org/netdev/c57283ed-6b9b-b0e6-ee12-5655c1c54495@bootlin.com/ > Signed-off-by: Romain Gantois > --- 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 CONFIG_NET_DSA_LOOP option, edit the return value of dsa_loop_get_protocol() and the "netdev" field of dsa_loop_pdata. The packets need to be analyzed on the link partner with a packet analysis tool, since there is no switch to strip the DSA tag. > drivers/net/ethernet/stmicro/stmmac/common.h | 1 + > .../net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 1 + > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 16 +++++++++++++++- > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c > index daf79cdbd3ec..50686cdc3320 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c > @@ -264,6 +264,7 @@ static int dwmac1000_get_hw_feature(void __iomem *ioaddr, > dma_cap->number_tx_channel = (hw_cap & DMA_HW_FEAT_TXCHCNT) >> 22; > /* Alternate (enhanced) DESC mode */ > dma_cap->enh_desc = (hw_cap & DMA_HW_FEAT_ENHDESSEL) >> 24; > + dma_cap->dsa_breaks_tx_coe = 1; > > return 0; > } > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index a9b6b383e863..733348c65e04 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -42,6 +42,7 @@ > #include > #include > #include > +#include > #include "stmmac_ptp.h" > #include "stmmac.h" > #include "stmmac_xdp.h" > @@ -993,8 +994,11 @@ static void stmmac_mac_link_up(struct phylink_config *config, > int speed, int duplex, > bool tx_pause, bool rx_pause) > { > - struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); > + struct net_device *ndev = to_net_dev(config->dev); > + struct stmmac_priv *priv = netdev_priv(ndev); > + unsigned int tx_queue_cnt; > u32 old_ctrl, ctrl; > + int queue; > > if ((priv->plat->flags & STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP) && > priv->plat->serdes_powerup) > @@ -1102,6 +1106,16 @@ static void stmmac_mac_link_up(struct phylink_config *config, > > if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY) > stmmac_hwtstamp_correct_latency(priv, priv); > + > + /* DSA tags break COEs on some cores. Disable TX checksum > + * offloading on those cores if the netdevice is a DSA conduit. > + */ > + if (priv->dma_cap.dsa_breaks_tx_coe && netdev_uses_dsa(ndev)) { > + tx_queue_cnt = priv->plat->tx_queues_to_use; > + for (queue = 0; queue < tx_queue_cnt; queue++) > + priv->plat->tx_queues_cfg[queue].coe_unsupported = true; > + } > + The DSA switch driver can load after stmmac_mac_link_up() runs. This implementation is racy anyway. > } > > static const struct phylink_mac_ops stmmac_phylink_mac_ops = { > -- > 2.43.0 > > pw-bot: cr _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel