[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <9beeb68d-2973-4d40-b48b-ee9cc984b9db@app.fastmail.com>
Date: Thu, 16 Oct 2025 20:33:27 +1100
From: "Mathew McBride" <matt@...verse.com.au>
To: "Ioana Ciornei" <ioana.ciornei@....com>
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 9:18 PM, Ioana Ciornei wrote:
> On Wed, Oct 15, 2025 at 03:01:24PM +1100, Mathew McBride wrote:
[snip]
> Hi Mathew,
>
> First of all, sorry for missing your initial message.
>
No problem. I had a revert patch in my tree and mostly forgot about the problem until a customer of ours reminded me recently. The S/G "solution" looked easy enough to try.
> 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?
>
It looks good to me! I've tested across two different kernel series (6.6 and 6.18) with two different host userlands (Debian and OpenWrt). Both VM (vhost-net) and normal system traffic are OK.
Many thanks,
Matt
> --- 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
>
Powered by blists - more mailing lists