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

["Mathew McBride" <matt@traverse.com.au>]

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
>