[PATCH 0/3] net: ethernet: cortina: Fix RX budget accounting

[PATCH 0/3] net: ethernet: cortina: Fix RX budget accounting

[Linus Walleij]

Report the actual NAPI work, charge dropped frames to the poll budget,
and drive free-queue refills from consumed RX descriptors.

Tested on the D-Link DIR-685.

Hi Sashiko, yes there are more latent issues I will get to them, but
my LLM thinks those are on the top of the list.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Linus Walleij (3):
      net: ethernet: cortina: Fix budget accounting
      net: ethernet: cortina: Count dropped frames as NAPI work
      net: ethernet: cortina: Count RX descriptors for freeq refill

 drivers/net/ethernet/cortina/gemini.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260901-gemini-ethernet-fixes-e6d2e7c53c1b

Best regards,
--  
Linus Walleij <linusw@kernel.org>

[PATCH 1/3] net: ethernet: cortina: Fix budget accounting

[Linus Walleij]

The gmac_rx() function returns the remaining NAPI budget, but its
caller treats the return value as the number of packets received. An
idle poll therefore reports a full budget and remains scheduled.

Return the number of received packets instead. Preserve the existing
free queue refill accounting by adding that count directly; continuing
to subtract it from the budget would invert the refill behavior.

Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Link: https://lore.kernel.org/r/20260509-gemini-ethernet-fixes-v1-4-6c5d20ddc35b@kernel.org
Link: https://lore.kernel.org/r/20260512131456.189452-1-pabeni@redhat.com
Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/ethernet/cortina/gemini.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 4c762229ce42..1d9824d1716c 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1450,6 +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;
 	union gmac_rxdesc_0 word0;
 	union gmac_rxdesc_1 word1;
 	union gmac_rxdesc_3 word3;
@@ -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;
 }
 
 static int gmac_napi_poll(struct napi_struct *napi, int budget)
@@ -1586,7 +1588,7 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
 		++port->rx_napi_exits;
 	}
 
-	port->freeq_refill += (budget - received);
+	port->freeq_refill += received;
 	if (port->freeq_refill > freeq_threshold) {
 		port->freeq_refill -= freeq_threshold;
 		geth_fill_freeq(geth, true);

-- 
2.55.0

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

[Linus Walleij]

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;
 
 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++;
+		}
 	}
 
 	port->rx_skb = skb;
 	port->rx_frag_nr = frag_nr;
 	writew(r, ptr_reg);
-	return received;
+	return work_done;
 }
 
 static int gmac_napi_poll(struct napi_struct *napi, int budget)
@@ -1575,27 +1579,27 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
 	struct gemini_ethernet_port *port = netdev_priv(napi->dev);
 	struct gemini_ethernet *geth = port->geth;
 	unsigned int freeq_threshold;
-	unsigned int received;
+	unsigned int work_done;
 
 	freeq_threshold = 1 << (geth->freeq_order - 1);
 	u64_stats_update_begin(&port->rx_stats_syncp);
 
-	received = gmac_rx(napi->dev, budget);
-	if (received < budget) {
+	work_done = gmac_rx(napi->dev, budget);
+	if (work_done < budget) {
 		napi_gro_flush(napi, false);
-		napi_complete_done(napi, received);
+		napi_complete_done(napi, work_done);
 		gmac_enable_rx_irq(napi->dev, 1);
 		++port->rx_napi_exits;
 	}
 
-	port->freeq_refill += received;
+	port->freeq_refill += work_done;
 	if (port->freeq_refill > freeq_threshold) {
 		port->freeq_refill -= freeq_threshold;
 		geth_fill_freeq(geth, true);
 	}
 
 	u64_stats_update_end(&port->rx_stats_syncp);
-	return received;
+	return work_done;
 }
 
 static void gmac_dump_dma_state(struct net_device *netdev)

-- 
2.55.0

[PATCH 3/3] net: ethernet: cortina: Count RX descriptors for freeq refill

[Linus Walleij]

The software free queue provides one buffer fragment for every descriptor
moved to an RX queue. The refill heuristic instead advances by NAPI work,
which counts frames. A fragmented or discarded frame can consume several
queue entries while adding only one to the refill count.

Count the RX descriptors as they are consumed and report that separately
from NAPI work. Use the descriptor count to drive free queue refills.

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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 699354604b93..c17b06159f55 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1439,7 +1439,8 @@ static struct sk_buff *gmac_skb_if_good_frame(struct gemini_ethernet_port *port,
 	return skb;
 }
 
-static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
+static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
+			    unsigned int *freeq_consumed)
 {
 	struct gemini_ethernet_port *port = netdev_priv(netdev);
 	unsigned short m = (1 << port->rxq_order) - 1;
@@ -1447,6 +1448,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 	void __iomem *ptr_reg = port->rxq_rwptr;
 	unsigned int frag_nr = port->rx_frag_nr;
 	struct sk_buff *skb = port->rx_skb;
+	unsigned int consumed = 0;
 	unsigned int frame_len, frag_len;
 	struct gmac_rxdesc *rx = NULL;
 	struct gmac_queue_page *gpage;
@@ -1480,6 +1482,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 
 		r++;
 		r &= m;
+		consumed++;
 
 		frag_len = word0.bits.buffer_size;
 		frame_len = word1.bits.byte_count;
@@ -1570,6 +1573,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 
 	port->rx_skb = skb;
 	port->rx_frag_nr = frag_nr;
+	*freeq_consumed = consumed;
 	writew(r, ptr_reg);
 	return work_done;
 }
@@ -1579,12 +1583,13 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
 	struct gemini_ethernet_port *port = netdev_priv(napi->dev);
 	struct gemini_ethernet *geth = port->geth;
 	unsigned int freeq_threshold;
+	unsigned int freeq_consumed;
 	unsigned int work_done;
 
 	freeq_threshold = 1 << (geth->freeq_order - 1);
 	u64_stats_update_begin(&port->rx_stats_syncp);
 
-	work_done = gmac_rx(napi->dev, budget);
+	work_done = gmac_rx(napi->dev, budget, &freeq_consumed);
 	if (work_done < budget) {
 		napi_gro_flush(napi, false);
 		napi_complete_done(napi, work_done);
@@ -1592,7 +1597,7 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
 		++port->rx_napi_exits;
 	}
 
-	port->freeq_refill += work_done;
+	port->freeq_refill += freeq_consumed;
 	if (port->freeq_refill > freeq_threshold) {
 		port->freeq_refill -= freeq_threshold;
 		geth_fill_freeq(geth, true);

-- 
2.55.0

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

[Joe Damato]

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(-)

[...]

> @@ -1575,27 +1579,27 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
>  	struct gemini_ethernet_port *port = netdev_priv(napi->dev);
>  	struct gemini_ethernet *geth = port->geth;
>  	unsigned int freeq_threshold;
> -	unsigned int received;
> +	unsigned int work_done;
>  
>  	freeq_threshold = 1 << (geth->freeq_order - 1);
>  	u64_stats_update_begin(&port->rx_stats_syncp);
>  
> -	received = gmac_rx(napi->dev, budget);
> -	if (received < budget) {
> +	work_done = gmac_rx(napi->dev, budget);
> +	if (work_done < budget) {
>  		napi_gro_flush(napi, false);
> -		napi_complete_done(napi, received);
> +		napi_complete_done(napi, work_done);
>  		gmac_enable_rx_irq(napi->dev, 1);
>  		++port->rx_napi_exits;
>  	}

Hmm. I could be wrong, but I think the above code could be simplified by
calling napi_complete_done and letting it do the gro_flush? You can check the
return value and then optionally enable interrupts. That's what most other
drivers do, IIRC.

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

[Joe Damato]

On Tue, Sep 01, 2026 at 07:04:55PM +0200, Linus Walleij wrote:
> The gmac_rx() function returns the remaining NAPI budget, but its
> caller treats the return value as the number of packets received. An
> idle poll therefore reports a full budget and remains scheduled.
> 
> Return the number of received packets instead. Preserve the existing
> free queue refill accounting by adding that count directly; continuing
> to subtract it from the budget would invert the refill behavior.
> 
> Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
> Link: https://lore.kernel.org/r/20260509-gemini-ethernet-fixes-v1-4-6c5d20ddc35b@kernel.org
> Link: https://lore.kernel.org/r/20260512131456.189452-1-pabeni@redhat.com
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/net/ethernet/cortina/gemini.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>

Looks right to me. I assume the LLM mentioned
u64_stats_update_begin/u64_stats_update_end ? Just feels like there's a lot of
code in between but maybe the only system this NIC runs on, u64_stats_update_*
are no-ops ? idk.

Anyway:

Reviewed-by: Joe Damato <joe@dama.to>

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

[Joe Damato]

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

Re: [PATCH 3/3] net: ethernet: cortina: Count RX descriptors for freeq refill

[Joe Damato]

On Tue, Sep 01, 2026 at 07:04:57PM +0200, Linus Walleij wrote:
> The software free queue provides one buffer fragment for every descriptor
> moved to an RX queue. The refill heuristic instead advances by NAPI work,
> which counts frames. A fragmented or discarded frame can consume several
> queue entries while adding only one to the refill count.
> 
> Count the RX descriptors as they are consumed and report that separately
> from NAPI work. Use the descriptor count to drive free queue refills.
> 
> 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 | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>

Reviewed-by: Joe Damato <joe@dama.to>

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

[Jakub Kicinski]

On Tue, 01 Sep 2026 19:04:56 +0200 Linus Walleij wrote:
> -	unsigned int received = 0;
> +	unsigned int work_done = 0;

Do you (Linus) think that renaming the variables all over the place in
a fix is necessary? Or just LLM did it and you didn't bother pausing to
think? 

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

[Jakub Kicinski]

On Tue, 1 Sep 2026 13:09:02 -0700 Joe Damato wrote:
> > +	if (work_done < budget) {
> >  		napi_gro_flush(napi, false);
> > -		napi_complete_done(napi, received);
> > +		napi_complete_done(napi, work_done);
> >  		gmac_enable_rx_irq(napi->dev, 1);
> >  		++port->rx_napi_exits;
> >  	}  
> 
> Hmm. I could be wrong, but I think the above code could be simplified by
> calling napi_complete_done and letting it do the gro_flush? You can check the
> return value and then optionally enable interrupts. That's what most other
> drivers do, IIRC.

Good suggestion, otherwise exiting busy poll will falsely enable IRQs.
But to be clear that should be separate patch

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

[Linus Walleij]

On Wed, Sep 2, 2026 at 12:37 AM Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 01 Sep 2026 19:04:56 +0200 Linus Walleij wrote:

> > -     unsigned int received = 0;
> > +     unsigned int work_done = 0;
>
> Do you (Linus) think that renaming the variables all over the place in
> a fix is necessary? Or just LLM did it and you didn't bother pausing to
> think?

Yeah a typical mistake on my side I think, over-confidence in
the tools' choice of variable name, :(

"received" is just as good a name, but I see the appeal for an
LLM to do this, since that is the name used in a plethora of the
drivers under drivers/net, it kind of shines through how this
thing does its thing.

Yours,
Linus Walleij