Re: [PATCH net-next v4 12/15] net: ethernet: cortina: Account RX buffer size in truesize

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Commit "net: ethernet: cortina: Account RX buffer size in
  truesize" fixes a real bug but has no Fixes: trailer.

--

net: ethernet: cortina: Account RX buffer size in truesize

This changes gmac_rx() so that each page fragment attached to an skb adds
the full posted free queue fragment size (freeq_frag_len) to skb->truesize.
Before this change it added only the received payload length (frag_len).

> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index d8bafcdf4d5f1..43fd8fd773e3c 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -1849,7 +1849,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
>  		skb_fill_page_desc(skb, frag_nr, page, page_offs, frag_len);
>  		skb->len += frag_len;
>  		skb->data_len += frag_len;
> -		skb->truesize += frag_len;
> +		skb->truesize += freeq_frag_len;

[Severity: Low]
The code change looks fine, but should this commit have a Fixes: tag?

The commit message describes a functional problem:

  "short packets understate their memory use to GRO and socket
  accounting."

The old skb->truesize += frag_len line in gmac_rx() has been there since
the original driver commit:

  Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")

Other patches in this series already carry this tag, for example "Keep
shared free queue parent-owned" and "Correct free queue DMA mappings".
Without it, stable tooling won't pick up this fix.

The freeq_frag_len local in gmac_rx() is also missing from the tree
before this series. An earlier patch in the series adds it, so a backport
of this fix needs that patch too. Could that dependency be noted for
backporters?

>  		frag_nr++;
>  
>  		if (word3.bits32 & EOF_BIT) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261011-gemini-ethernet-fixes-3-v4-0-4b1631b08f5d%40kernel.org