Re: [PATCH net 4/4] net: ethernet: cortina: Fix budget accounting

[Paolo Abeni <pabeni@redhat.com>]

From: AI Reviewer <ai@example.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: ethernet: cortina: Fix budget accounting

This patch changes gmac_rx() to return the number of packets received
rather than the remaining budget, and updates the napi_complete_done()
call in gmac_napi_poll() to pass the new value.

Link: https://sashiko.dev/#/patchset/20260505-gemini-ethernet-fix-v2-1-997c31d06079%40kernel.org

> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 4c762229ce42..5839e00aa649 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -1545,7 +1546,8 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
>  			napi_gro_frags(&port->napi);
>  			skb = NULL;
>  			frag_nr = 0;
> -			--budget;
> +			budget--;
> +			received++;
>  		}
>  		continue;
>
> @@ -1565,7 +1567,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
>  	port->rx_skb = skb;
>  	port->rx_frag_nr = frag_nr;
>  	writew(r, ptr_reg);
> -	return budget;
> +	return received;
>  }

[High]
With the return value's meaning inverted, does the second consumer of
gmac_rx()'s return value in gmac_napi_poll() still compute what it used
to?

drivers/net/ethernet/cortina/gemini.c:gmac_napi_poll() {
    ...
    received = gmac_rx(napi->dev, budget);
    if (received < budget) {
        napi_gro_flush(napi, false);
        napi_complete_done(napi, received);
        gmac_enable_rx_irq(napi->dev, 1);
        ++port->rx_napi_exits;
    }

    port->freeq_refill += (budget - received);
    ...
}

Before this patch, gmac_rx() returned the remaining budget, so
(budget - received) evaluated to the number of packets received, i.e.
the number of freeq pages consumed.

After this patch, received already holds the number of packets
received, so (budget - received) now evaluates to the unused budget,
which is the opposite quantity.

Does this invert the freeq_refill accounting?

On an idle/light-load poll where received is 0, freeq_refill would now
be incremented by the full budget every poll, causing geth_fill_freeq()
to be invoked much more often than before.

On a saturated poll where received is close to budget, freeq_refill
would barely grow, which is precisely the case where refilling matters
most (and also the case where the napi_complete_done() early-exit
branch above is skipped).

Would updating this consumer in the same patch, for example:

    port->freeq_refill += received;

restore the original behavior?

> 
>  static int gmac_napi_poll(struct napi_struct *napi, int budget)
-- 
This is an AI-generated review.