netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matteo Croce <mcroce@linux.microsoft.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org, peppe.cavallaro@st.com,
	alexandre.torgue@foss.st.com, kuba@kernel.org,
	palmer@dabbelt.com, paul.walmsley@sifive.com,
	drew@beagleboard.org, kernel@esmil.dk
Subject: Re: [PATCH net-next] stmmac: align RX buffers
Date: Tue, 15 Jun 2021 01:21:07 +0200	[thread overview]
Message-ID: <20210615012107.577ead86@linux.microsoft.com> (raw)
In-Reply-To: <20210614.125111.1519954686951337716.davem@davemloft.net>

On Mon, 14 Jun 2021 12:51:11 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> 
> But thois means the ethernet header will be misaliugned and this will
> kill performance on some cpus as misaligned accessed are resolved
> wioth a trap handler.
> 
> Even on cpus that don't trap, the access will be slower.
> 
> Thanks.

Isn't the IP header which should be aligned to avoid expensive traps?
From include/linux/skbuff.h:

 * Since an ethernet header is 14 bytes network drivers often end up with
 * the IP header at an unaligned offset. The IP header can be aligned by
 * shifting the start of the packet by 2 bytes. Drivers should do this
 * with:
 *
 * skb_reserve(skb, NET_IP_ALIGN);

But the problem here really is not the header alignment, the problem is
that the rx buffer is copied into an skb, and the two buffers have
different alignments.
If I add this print, I get this for every packet:

--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5460,6 +5460,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
+               printk("skb->data alignment: %lu\n", (uintptr_t)skb->data & 7);
+               printk("xdp.data alignment: %lu\n" , (uintptr_t)xdp.data & 7);
                skb_copy_to_linear_data(skb, xdp.data, buf1_len);

[ 1060.967768] skb->data alignment: 2
[ 1060.971174] xdp.data alignment: 0
[ 1061.967589] skb->data alignment: 2
[ 1061.970994] xdp.data alignment: 0

And many architectures do an optimized memcpy when the low order bits of the
two pointers match, to name a few:

arch/alpha/lib/memcpy.c:
	/* If both source and dest are word aligned copy words */
	if (!((unsigned int)dest_w & 3) && !((unsigned int)src_w & 3)) {

arch/xtensa/lib/memcopy.S:
	/*
	 * Destination and source are word-aligned, use word copy.
	 */
	# copy 16 bytes per iteration for word-aligned dst and word-aligned src

arch/openrisc/lib/memcpy.c:
	/* If both source and dest are word aligned copy words */
	if (!((unsigned int)dest_w & 3) && !((unsigned int)src_w & 3)) {

And so on. With my patch I (mis)align the two buffer at an offset 2
(NET_IP_ALIGN) so the data can be copied faster:

[   16.648485] skb->data alignment: 2
[   16.651894] xdp.data alignment: 2
[   16.714260] skb->data alignment: 2
[   16.717688] xdp.data alignment: 2

Does this make sense?

Regards,
-- 
per aspera ad upstream

  reply	other threads:[~2021-06-14 23:21 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 [this message]
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
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=20210615012107.577ead86@linux.microsoft.com \
    --to=mcroce@linux.microsoft.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=davem@davemloft.net \
    --cc=drew@beagleboard.org \
    --cc=kernel@esmil.dk \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=netdev@vger.kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=peppe.cavallaro@st.com \
    /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).