Skip to contentSkip to conversation

net: ethernet: cortina: Fix RX budget accounting

PATCH Superseded Linus Walleij 3 participants 11 messages Sep 1 – Sep 2

Subsystems (3)
Networking Network drivers Arm/cortina systems gemini Arm architecture

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Linus Walleij

author maintainer
#0 lore raw

cover net: ethernet: cortina: Fix RX budget accounting

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 <[email protected]>

---
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,

Signature · 1 line
Linus Walleij <[email protected]>
author maintainer
#1 lore raw

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

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/[email protected] open here
Link https://lore.kernel.org/r/[email protected] open here
Assisted-by LLM
Signed-off-by Linus Walleij <[email protected]>
---
 drivers/net/ethernet/cortina/gemini.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/net/ethernet/cortina/gemini.c +5 -3
index 4c762229ce42..1d9824d1716c 100644
@@ -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);
Signature · 1 line
2.55.0
Reviewed-by on v1 1/3
#2 lore raw

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

Quoted 8 lines · 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.

Quoted 9 lines
Fixes 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Link https://lore.kernel.org/r/[email protected] open here
Link https://lore.kernel.org/r/[email protected] open here
Assisted-by LLM
Signed-off-by Linus Walleij <[email protected]>
---
 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 <[email protected]>
author maintainer
#3 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/net/ethernet/cortina/gemini.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)
drivers/net/ethernet/cortina/gemini.c +17 -13
index 1d9824d1716c..699354604b93 100644
@@ -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)
Signature · 1 line
2.55.0
#4 lore raw

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

Quoted 14 lines · 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 <[email protected]>
---
 drivers/net/ethernet/cortina/gemini.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

[...]

Quoted 10 lines
drivers/net/ethernet/cortina/gemini.c +1 -1
@@ -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);

drivers/net/ethernet/cortina/gemini.c static int gmac_napi_poll(struct napi_struct *napi, int budget) · line 1593

Quoted 10 lines
drivers/net/ethernet/cortina/gemini.c +3 -3
-	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.

maintainer
#5 lore raw

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

Quoted 4 lines · On Tue, 1 Sep 2026 13:09:02 -0700 Joe Damato wrote:
Quoted 4 lines
drivers/net/ethernet/cortina/gemini.c +2 -1
+	if (work_done < budget) {
 		napi_gro_flush(napi, false);
-		napi_complete_done(napi, received);
+		napi_complete_done(napi, work_done);
Quoted 8 lines

drivers/net/ethernet/cortina/gemini.c static int gmac_napi_poll(struct napi_struct *napi, int budget) · line 1593

Quoted 3 lines
drivers/net/ethernet/cortina/gemini.c +0 -0
 		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

#6 lore raw

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

In reply to Linus Walleij
Quoted 47 lines · 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 <[email protected]>
---
 drivers/net/ethernet/cortina/gemini.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)
drivers/net/ethernet/cortina/gemini.c +3 -3
index 1d9824d1716c..699354604b93 100644
@@ -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)

drivers/net/ethernet/cortina/gemini.c static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget) · line 1550

Quoted 8 lines
drivers/net/ethernet/cortina/gemini.c +1 -3
@@ -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.

drivers/net/ethernet/cortina/gemini.c static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget) · line 1568

Quoted 13 lines
drivers/net/ethernet/cortina/gemini.c +6 -0
 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:

Quoted 2 lines

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

maintainer
#7 lore raw

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

In reply to Linus Walleij

drivers/net/ethernet/cortina/gemini.c static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget) · line 1453

Quoted 2 lines · On Tue, 01 Sep 2026 19:04:56 +0200 Linus Walleij wrote:
drivers/net/ethernet/cortina/gemini.c +1 -1
-	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?

author maintainer
#8 lore raw

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

Quoted 8 lines · Jakub Kicinski

drivers/net/ethernet/cortina/gemini.c static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget) · line 1453

Quoted 2 lines · On Tue, 01 Sep 2026 19:04:56 +0200 Linus Walleij wrote:
drivers/net/ethernet/cortina/gemini.c +1 -1
-     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

author maintainer
#9 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/net/ethernet/cortina/gemini.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/net/ethernet/cortina/gemini.c +8 -3
index 699354604b93..c17b06159f55 100644
@@ -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);
Signature · 1 line
2.55.0
Reviewed-by on v1 3/3
#10 lore raw

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

Quoted 8 lines · 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.

Quoted 7 lines
Fixes 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Assisted-by LLM
Signed-off-by Linus Walleij <[email protected]>
---
 drivers/net/ethernet/cortina/gemini.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Reviewed-by Joe Damato <[email protected]>