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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC882C4338F for ; Thu, 12 Aug 2021 11:05:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2776A61059 for ; Thu, 12 Aug 2021 11:05:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236718AbhHLLGN (ORCPT ); Thu, 12 Aug 2021 07:06:13 -0400 Received: from mail.kernel.org ([198.145.29.99]:42062 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231667AbhHLLGF (ORCPT ); Thu, 12 Aug 2021 07:06:05 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 9ECFD60EE2; Thu, 12 Aug 2021 11:05:40 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mE8Wo-004XFv-JM; Thu, 12 Aug 2021 12:05:38 +0100 Date: Thu, 12 Aug 2021 12:05:38 +0100 Message-ID: <874kbuapod.wl-maz@kernel.org> From: Marc Zyngier To: Matteo Croce Cc: Eric Dumazet , Thierry Reding , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, Giuseppe Cavallaro , Alexandre Torgue , "David S. Miller" , Jakub Kicinski , Palmer Dabbelt , Paul Walmsley , Drew Fustini , Emil Renner Berthing , Jon Hunter , Will Deacon Subject: Re: [PATCH net-next] stmmac: align RX buffers In-Reply-To: <20210812121835.405d2e37@linux.microsoft.com> References: <20210614022504.24458-1-mcroce@linux.microsoft.com> <871r71azjw.wl-maz@kernel.org> <202417ef-f8ae-895d-4d07-1f9f3d89b4a4@gmail.com> <87o8a49idp.wl-maz@kernel.org> <20210812121835.405d2e37@linux.microsoft.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: mcroce@linux.microsoft.com, eric.dumazet@gmail.com, thierry.reding@gmail.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, peppe.cavallaro@st.com, alexandre.torgue@foss.st.com, davem@davemloft.net, kuba@kernel.org, palmer@dabbelt.com, paul.walmsley@sifive.com, drew@beagleboard.org, kernel@esmil.dk, jonathanh@nvidia.com, will@kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 12 Aug 2021 11:18:35 +0100, Matteo Croce wrote: > > On Thu, 12 Aug 2021 10:48:03 +0200 > Eric Dumazet wrote: > > > > > > > On 8/11/21 4:16 PM, Marc Zyngier wrote: > > > On Wed, 11 Aug 2021 13:53:59 +0100, > > > Eric Dumazet wrote: > > > > > >> Are you sure you do not need to adjust stmmac_set_bfsize(), > > >> stmmac_rx_buf1_len() and stmmac_rx_buf2_len() ? > > >> > > >> Presumably DEFAULT_BUFSIZE also want to be increased by NET_SKB_PAD > > >> > > >> Patch for stmmac_rx_buf1_len() : > > >> > > >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > >> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index > > >> 7b8404a21544cf29668e8a14240c3971e6bce0c3..041a74e7efca3436bfe3e17f972dd156173957a9 > > >> 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ > > >> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -4508,12 > > >> +4508,12 @@ static unsigned int stmmac_rx_buf1_len(struct > > >> stmmac_priv *priv, /* First descriptor, not last descriptor and > > >> not split header */ if (status & rx_not_ls) > > >> - return priv->dma_buf_sz; > > >> + return priv->dma_buf_sz - NET_SKB_PAD - > > >> NET_IP_ALIGN; > > >> plen = stmmac_get_rx_frame_len(priv, p, coe); > > >> > > >> /* First descriptor and last descriptor and not split > > >> header */ > > >> - return min_t(unsigned int, priv->dma_buf_sz, plen); > > >> + return min_t(unsigned int, priv->dma_buf_sz - NET_SKB_PAD > > >> - NET_IP_ALIGN, plen); } > > >> > > >> static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv, > > > > > > Feels like a major deficiency of the original patch. Happy to test a > > > more complete patch if/when you have one. > > > > I wont have time in the immediate future. > > > > Matteo, if you do not work on a fix, I suggest we revert > > a955318fe67ec0d962760b5ee58e74bffaf649b8 stmmac: align RX buffers > > > > before a more polished version can be submitted. > > > > Better to use stmmac_rx_offset() so to have the correct length when > using XDP. Also, when XDP is enabled, the offset was > XDP_PACKET_HEADROOM (i.e. 256 bytes) even before the change, so it > could be already broken. Mark, can you try on the Jetson TX2 by > attaching an XDP program and see if it works without my change? Sorry, you'll have to hold my hand here, as I know exactly nothing about XDP.... > A possible fix, which takes in account also the XDP headroom for > stmmac_rx_buf1_len() only could be (only compile tested, I don't have > the hardware now): However, this doesn't fix my issue. I still get all sort of corruption. Probably stmmac_rx_buf2_len() also need adjusting (it has a similar logic as its buf1 counterpart...) Unless you can fix it very quickly, and given that we're towards the end of the cycle, I'd be more comfortable if we reverted this patch. Thanks, M. -- Without deviation from the norm, progress is not possible.