linux-kernel - Re: [PATCH] dpaa2-eth: treat skb with exact headroom as scatter/gather frames

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <xl3227oc7kfa6swgaxoew7g2jzgy2ksgnpqo4qvz2nzbuludnh@ti6h25vfp4ft>
Date: Wed, 15 Oct 2025 13:18:23 +0300
From: Ioana Ciornei <ioana.ciornei@....com>
To: Mathew McBride <matt@...verse.com.au>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dpaa2-eth: treat skb with exact headroom as
 scatter/gather frames

On Wed, Oct 15, 2025 at 03:01:24PM +1100, Mathew McBride wrote:
> In commit f422abe3f23d ("dpaa2-eth: increase the needed headroom to
> account for alignment"), the required skb headroom of dpaa2-eth was
> increased to exactly 128 bytes. The headroom increase was to ensure
> frames on the Tx path were always aligned to 64 bytes.
> 
> This caused a regression when vhost-net was used to accelerate virtual
> machine frames between a KVM guest and a dpaa2-eth interface over a bridge.
> While the skb passed to the driver had the required headroom (128 bytes),
> the skb->head pointer did not match the alignment expected by the driver
> (aligned_start => skb->head in dpaa2_eth_build_single_fd).
> 
> Treating outbound skb's where skb_headroom() == net_dev->needed_headroom
> the same as skb's with inadequate headroom resolves this issue.
> 
> Signed-off-by: Mathew McBride <matt@...verse.com.au>
> Fixes: f422abe3f23d ("dpaa2-eth: increase the needed headroom to account for alignment")
> Closes: https://lore.kernel.org/netdev/70f0dcd9-1906-4d13-82df-7bbbbe7194c6@app.fastmail.com/T/#u
> ---
> A while ago, changes were made to the dpaa2-eth driver to workaround
> an issue when TX frames were not aligned to 64 bytes.
> 
> As part of this change, the required skb headroom in dpaa2-eth
> was increased to 128 bytes.
> 
> When frames originating from a virtual machine over vhost-net
> were forwarded to the dpaa2-eth driver for transmission,
> the vhost frames were being dropped as they failed an alignment check.
> 
> The skb's originating from vhost-net had exactly the required headroom
> (128 bytes).
> 
> I have tested a fix to the issue which treats frames with the "exact"
> headroom the same as frames with inadequate headroom. These are
> transmitted using the scatter/gather (S/G) process.
> 
> Network drivers are not my area of expertise so I cannot be 100%
> confident this is the correct solution, however, I've done extensive
> reliability testing on this fix to confirm it resolves the regression
> involving vhost-net without any other side effects.
> 
> What I can't answer (yet) is if there are performance or other ramifcations
> from having all VM-originating frames handled as S/G.
> 
> As far as I am aware, the virtual machine / vhost-net workload is the
> only workload that generates skb's that require the S/G handling in
> vhost-net. I have not seen any variants of this issue without vhost-net.
> 
> My original analysis of the problem can be found in the message below.
> The diagnosis of the issue is still correct at the time of writing
> (circa 6.18-rc1)
> 
> https://lore.kernel.org/netdev/70f0dcd9-1906-4d13-82df-7bbbbe7194c6@app.fastmail.com/T/#u

Hi Mathew,

First of all, sorry for missing your initial message.

While I was trying to understand how the 'aligned_start >= skb->head'
check ends up failing while you have the necessary 128bytes of headroom,
I think I discovered that this is not some kind of off-by-one issue.

The below snippet is from your original message.
	fsl_dpaa2_eth dpni.9: dpaa2_eth_build_single_fd alignment issue, aligned_start=ffff008002e09140 skb->head=ffff008002e09180
	fsl_dpaa2_eth dpni.9: dpaa2_eth_build_single_fd data=ffff008002e09200
	fsl_dpaa2_eth dpni.9: dpaa2_eth_build_single_fd is cloned=0
	dpaa2_eth_build_single_fdskb len=150 headroom=128 headlen=150 tailroom=42

If my understanding is correct skb->data=ffff008002e09200.
Following the dpaa2_eth_build_single_fd() logic, this means that
	buffer_start = 0xffff008002e09200 - 0x80
	buffer_start = 0xFFFF008002E09180

Now buffer_start is already pointing to the start of the skb's memory
and I don't think the extra 'buffer_start - DPAA2_ETH_TX_BUF_ALIGN'
adjustment is correct.

What I think happened is that I did not take into consideration that by
adding the DPAA2_ETH_TX_BUF_ALIGN inside the dpaa2_eth_needed_headroom()
function I also need to remove it from the manual adjustment.

Could you please try with the following diff and let me know how it does
in your setup?

--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -1077,7 +1077,7 @@ static int dpaa2_eth_build_single_fd(struct dpaa2_eth_priv *priv,
        dma_addr_t addr;

        buffer_start = skb->data - dpaa2_eth_needed_headroom(skb);
-       aligned_start = PTR_ALIGN(buffer_start - DPAA2_ETH_TX_BUF_ALIGN,
+       aligned_start = PTR_ALIGN(buffer_start,
                                  DPAA2_ETH_TX_BUF_ALIGN);
        if (aligned_start >= skb->head)
                buffer_start = aligned_start;

Ioana