Re: [PATCH 2/3] net: ethernet: cortina: Count dropped frames as NAPI work

[Joe Damato <joe@dama.to>]

On Tue, Sep 01, 2026 at 07:04:56PM +0200, Linus Walleij wrote:
> The RX loop only consumes budget when it successfully delivers a frame.
> Error paths keep consuming descriptors without reducing the budget, so a
> stream of bad frames can process the entire receive ring in one poll.
> 
> Move the budget accounting to a common end-of-frame path. This counts
> each completed frame as NAPI work whether it was delivered or dropped,
> matching the behavior of the vendor driver.
> 
> Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/net/ethernet/cortina/gemini.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 1d9824d1716c..699354604b93 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -1450,7 +1450,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
>  	unsigned int frame_len, frag_len;
>  	struct gmac_rxdesc *rx = NULL;
>  	struct gmac_queue_page *gpage;
> -	unsigned int received = 0;
> +	unsigned int work_done = 0;
>  	union gmac_rxdesc_0 word0;
>  	union gmac_rxdesc_1 word1;
>  	union gmac_rxdesc_3 word3;
> @@ -1501,7 +1501,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
>  				skb = NULL;
>  				frag_nr = 0;
>  			}
> -			continue;
> +			goto next_desc;
>  		}
>  		page = gpage->page;
>  
> @@ -1523,7 +1523,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
>  
>  		} else if (!skb) {
>  			put_page(page);
> -			continue;
> +			goto next_desc;
>  		}
>  
>  		if (word3.bits32 & EOF_BIT)
> @@ -1546,10 +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--;
> -			received++;
>  		}
> -		continue;
> +		goto next_desc;

if i understand correctly, removing the continue here results in the success
case several blocks above here jumping to next_desc. in that case, is
word3.bits32 & EOF_BIT always true? just asking because it looked like in the
success case the SOF_BIT is set and wanted to check that the budget and
work_done math that is being moved down to next_desc will still be run.

>  
>  err_drop:
>  		if (skb) {
> @@ -1562,12 +1560,18 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
>  			put_page(page);
>  
>  		port->stats.rx_dropped++;
> +
> +next_desc:
> +		if (word3.bits32 & EOF_BIT) {
> +			budget--;
> +			work_done++;
> +		}

idk if it matters but i noted that the commit message says:

  > Move the budget accounting to a common end-of-frame path. This counts
  > each completed frame as NAPI work whether it was delivered or dropped,

but i noted that some of the jumps to next_desc don't seem to increment
port->stats.rx_dropped, but others do.

not sure if that matters for the correctness of the stats