From: Matteo Croce <mcroce@linux.microsoft.com>
To: Marc Zyngier <maz@kernel.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
Thierry Reding <thierry.reding@gmail.com>,
netdev@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-riscv <linux-riscv@lists.infradead.org>,
Giuseppe Cavallaro <peppe.cavallaro@st.com>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Palmer Dabbelt <palmer@dabbelt.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Drew Fustini <drew@beagleboard.org>,
Emil Renner Berthing <kernel@esmil.dk>,
Jon Hunter <jonathanh@nvidia.com>, Will Deacon <will@kernel.org>
Subject: Re: [PATCH net-next] stmmac: align RX buffers
Date: Thu, 12 Aug 2021 13:18:48 +0200 [thread overview]
Message-ID: <CAFnufp2=1t2+fmxyGJ0Qu3Z+=wRwAX8faaPvrJdFpFeTS3J7Uw@mail.gmail.com> (raw)
In-Reply-To: <874kbuapod.wl-maz@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3658 bytes --]
On Thu, Aug 12, 2021 at 1:05 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Thu, 12 Aug 2021 11:18:35 +0100,
> Matteo Croce <mcroce@linux.microsoft.com> wrote:
> >
> > On Thu, 12 Aug 2021 10:48:03 +0200
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > >
> > >
> > > On 8/11/21 4:16 PM, Marc Zyngier wrote:
> > > > On Wed, 11 Aug 2021 13:53:59 +0100,
> > > > Eric Dumazet <eric.dumazet@gmail.com> 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....
>
Attach the attached object with:
ip link set eth0 xdp object passall.o
This is an empty XDP program, its source:
__attribute__((section("prog"), used))
int xdp_main(struct xdp_md *ctx)
{
return XDP_PASS;
}
Every packet will pass untouched, but the offset will be shifted from
0 to 256 bytes, which could trigger the problem anyway:
> > 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.
>
Can it be that the HW can't do DMA on an address which is not word aligned?
What if you replace NET_SKB_PAD with, let's say, 8?
Regards,
--
per aspera ad upstream
[-- Attachment #2: passall.o --]
[-- Type: application/x-object, Size: 864 bytes --]
next prev parent reply other threads:[~2021-08-12 11:19 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-14 2:25 [PATCH net-next] stmmac: align RX buffers Matteo Croce
2021-06-14 19:51 ` David Miller
2021-06-14 23:21 ` Matteo Croce
2021-06-15 17:28 ` David Miller
2021-06-15 17:30 ` patchwork-bot+netdevbpf
2021-08-10 19:07 ` Marc Zyngier
2021-08-11 10:28 ` Thierry Reding
2021-08-11 12:53 ` Eric Dumazet
2021-08-11 14:16 ` Marc Zyngier
2021-08-12 8:48 ` Eric Dumazet
2021-08-12 10:18 ` Matteo Croce
2021-08-12 11:05 ` Marc Zyngier
2021-08-12 11:18 ` Matteo Croce [this message]
2021-08-19 16:29 ` Marc Zyngier
2021-08-20 10:37 ` Matteo Croce
2021-08-20 16:26 ` Marc Zyngier
2021-08-20 16:38 ` Matteo Croce
2021-08-20 17:09 ` Marc Zyngier
2021-08-20 17:14 ` Matteo Croce
2021-08-20 17:24 ` Marc Zyngier
2021-08-20 17:35 ` Matteo Croce
2021-08-20 17:51 ` Marc Zyngier
2021-08-20 17:56 ` Matteo Croce
2021-08-20 18:05 ` Matteo Croce
2021-08-20 18:14 ` Marc Zyngier
2021-08-20 18:09 ` Marc Zyngier
2021-08-20 18:14 ` Matteo Croce
2021-08-20 18:41 ` Marc Zyngier
2021-08-16 15:12 ` Jakub Kicinski
2021-08-17 0:01 ` Matteo Croce
2021-08-19 15:26 ` Marc Zyngier
2021-08-11 10:41 ` Thierry Reding
2021-08-11 10:56 ` Joakim Zhang
2021-08-11 13:23 ` Marc Zyngier
2021-08-12 14:29 ` Thierry Reding
2021-08-12 15:26 ` Marc Zyngier
2021-08-13 14:44 ` Thierry Reding
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFnufp2=1t2+fmxyGJ0Qu3Z+=wRwAX8faaPvrJdFpFeTS3J7Uw@mail.gmail.com' \
--to=mcroce@linux.microsoft.com \
--cc=alexandre.torgue@foss.st.com \
--cc=davem@davemloft.net \
--cc=drew@beagleboard.org \
--cc=eric.dumazet@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=kernel@esmil.dk \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=maz@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=peppe.cavallaro@st.com \
--cc=thierry.reding@gmail.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).