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

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

[Linus Walleij]

Finish RX updates before releasing NAPI ownership, report actual NAPI
work, charge dropped frames to the poll budget, and drive free-queue
refills from consumed RX descriptors.

Track RX drop state across descriptor chains so discarded frames are
counted exactly once.

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>
---
Changes in v2:
- Finish RX statistics and free-queue updates before NAPI completion.
- Enable RX interrupts only after successful NAPI completion.
- Drop AI slop renaming of received->work_done.
- Link to v1: https://patch.msgid.link/20260901-gemini-ethernet-fixes-v1-0-ee6b09675876@kernel.org

---
Linus Walleij (5):
      net: ethernet: cortina: Fix budget accounting
      net: ethernet: cortina: Finish RX updates before NAPI completion
      net: ethernet: cortina: Count dropped frames as NAPI work
      net: ethernet: cortina: Count RX drops once per frame
      net: ethernet: cortina: Count RX descriptors for freeq refill

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

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

[PATCH v2 1/5] 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
Reviewed-by: Joe Damato <joe@dama.to>
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 v2 2/5] net: ethernet: cortina: Finish RX updates before NAPI completion

[Linus Walleij]

napi_complete_done() releases ownership of the NAPI instance, but the
Gemini poll keeps the RX statistics writer section open and updates the
free queue after calling it. A new poll can therefore start while the old
writer is still active.

Finish the statistics and free queue updates before releasing ownership.
Only re-enable RX interrupts when napi_complete_done() reports successful
completion.

Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Suggested-by: Joe Damato <joe@dama.to>
Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/ethernet/cortina/gemini.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 1d9824d1716c..6502220362cb 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1581,12 +1581,10 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
 	u64_stats_update_begin(&port->rx_stats_syncp);
 
 	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);
+	if (received < budget)
 		++port->rx_napi_exits;
-	}
+
+	u64_stats_update_end(&port->rx_stats_syncp);
 
 	port->freeq_refill += received;
 	if (port->freeq_refill > freeq_threshold) {
@@ -1594,7 +1592,9 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
 		geth_fill_freeq(geth, true);
 	}
 
-	u64_stats_update_end(&port->rx_stats_syncp);
+	if (received < budget && napi_complete_done(napi, received))
+		gmac_enable_rx_irq(napi->dev, 1);
+
 	return received;
 }
 

-- 
2.55.0

[PATCH v2 3/5] 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 | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 6502220362cb..33e9763b32fe 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -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,6 +1560,13 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 			put_page(page);
 
 		port->stats.rx_dropped++;
+
+next_desc:
+		/* Final or single-descriptor fragment, advance things */
+		if (word3.bits32 & EOF_BIT) {
+			budget--;
+			received++;
+		}
 	}
 
 	port->rx_skb = skb;

-- 
2.55.0

[PATCH v2 4/5] net: ethernet: cortina: Count RX drops once per frame

[Linus Walleij]

The absence of a partial skb means either that the driver is not
assembling a frame or that the current frame was already dropped.
Consequently, repeated descriptor errors can increment rx_dropped more
than once, while an orphaned descriptor chain can reach EOF without being
counted at all.

Track the dropping state across NAPI polls. Clear it at frame boundaries
and route mapping failures and orphaned continuations through the common
drop path so each discarded frame is counted exactly once.

Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Reported-by: Joe Damato <joe@dama.to>
Closes: https://lore.kernel.org/netdev/apdK5aMmvYssz35F@devvm20253.cco0.facebook.com/
Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/ethernet/cortina/gemini.c | 41 ++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 33e9763b32fe..9ba8524fa371 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -124,6 +124,7 @@ struct gemini_ethernet_port {
 	unsigned int		rx_coalesce_nsecs;
 	struct sk_buff		*rx_skb;
 	unsigned int		rx_frag_nr;
+	bool			rx_dropping;
 
 	unsigned int		freeq_refill;
 	struct gmac_txq		txq[TX_QUEUE_NUM];
@@ -1451,6 +1452,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 	struct gmac_rxdesc *rx = NULL;
 	struct gmac_queue_page *gpage;
 	unsigned int received = 0;
+	bool dropping = port->rx_dropping;
 	union gmac_rxdesc_0 word0;
 	union gmac_rxdesc_1 word1;
 	union gmac_rxdesc_3 word3;
@@ -1472,6 +1474,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 	w = rw.bits.wptr;
 
 	while (budget && w != r) {
+		page = NULL;
 		rx = port->rxq_ring + r;
 		word0 = rx->word0;
 		word1 = rx->word1;
@@ -1485,6 +1488,16 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 		frame_len = word1.bits.byte_count;
 		page_offs = mapping & ~PAGE_MASK;
 
+		if (word3.bits32 & SOF_BIT) {
+			if (skb) {
+				napi_free_frags(&port->napi);
+				port->stats.rx_dropped++;
+				skb = NULL;
+				frag_nr = 0;
+			}
+			dropping = false;
+		}
+
 		if (!mapping) {
 			netdev_err(netdev,
 				   "rxq[%u]: HW BUG: zero DMA desc\n", r);
@@ -1495,24 +1508,11 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 		gpage = gmac_get_queue_page(geth, port, mapping + PAGE_SIZE);
 		if (!gpage) {
 			dev_err(geth->dev, "could not find mapping\n");
-			port->stats.rx_dropped++;
-			if (skb) {
-				napi_free_frags(&port->napi);
-				skb = NULL;
-				frag_nr = 0;
-			}
-			goto next_desc;
+			goto err_drop;
 		}
 		page = gpage->page;
 
 		if (word3.bits32 & SOF_BIT) {
-			if (skb) {
-				napi_free_frags(&port->napi);
-				port->stats.rx_dropped++;
-				skb = NULL;
-				frag_nr = 0;
-			}
-
 			skb = gmac_skb_if_good_frame(port, word0, frame_len);
 			if (!skb)
 				goto err_drop;
@@ -1522,8 +1522,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 			frag_nr = 0;
 
 		} else if (!skb) {
-			put_page(page);
-			goto next_desc;
+			goto err_drop;
 		}
 
 		if (word3.bits32 & EOF_BIT)
@@ -1556,21 +1555,26 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 			frag_nr = 0;
 		}
 
-		if (mapping)
+		if (page)
 			put_page(page);
 
-		port->stats.rx_dropped++;
+		if (!dropping) {
+			port->stats.rx_dropped++;
+			dropping = true;
+		}
 
 next_desc:
 		/* Final or single-descriptor fragment, advance things */
 		if (word3.bits32 & EOF_BIT) {
 			budget--;
 			received++;
+			dropping = false;
 		}
 	}
 
 	port->rx_skb = skb;
 	port->rx_frag_nr = frag_nr;
+	port->rx_dropping = dropping;
 	writew(r, ptr_reg);
 	return received;
 }
@@ -1900,6 +1904,7 @@ static int gmac_stop(struct net_device *netdev)
 	napi_disable(&port->napi);
 	port->rx_skb = NULL;
 	port->rx_frag_nr = 0;
+	port->rx_dropping = false;
 
 	gmac_enable_irq(netdev, 0);
 	gmac_cleanup_rxq(netdev);

-- 
2.55.0

[PATCH v2 5/5] 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
Reviewed-by: Joe Damato <joe@dama.to>
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 9ba8524fa371..f08de623e6f7 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1440,7 +1440,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;
@@ -1448,6 +1449,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;
@@ -1483,6 +1485,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;
@@ -1575,6 +1578,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 	port->rx_skb = skb;
 	port->rx_frag_nr = frag_nr;
 	port->rx_dropping = dropping;
+	*freeq_consumed = consumed;
 	writew(r, ptr_reg);
 	return received;
 }
@@ -1584,18 +1588,19 @@ 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 received;
 
 	freeq_threshold = 1 << (geth->freeq_order - 1);
 	u64_stats_update_begin(&port->rx_stats_syncp);
 
-	received = gmac_rx(napi->dev, budget);
+	received = gmac_rx(napi->dev, budget, &freeq_consumed);
 	if (received < budget)
 		++port->rx_napi_exits;
 
 	u64_stats_update_end(&port->rx_stats_syncp);
 
-	port->freeq_refill += received;
+	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 v2 1/5] net: ethernet: cortina: Fix budget accounting

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 (4) · Low: 0

- [Medium] At this commit, gmac_rx()
  (drivers/net/ethernet/cortina/gemini.c:1443-1571) increments the new…

Pre-existing issues:
- [Medium] gmac_napi_poll() at drivers/net/ethernet/cortina/gemini.c:1591
  does `port->freeq_refill += received;` and compares the result against…
- [Medium] gmac_napi_poll()
  (drivers/net/ethernet/cortina/gemini.c:1584-1589) discards the return…
- [Medium] gmac_napi_poll()
  (drivers/net/ethernet/cortina/gemini.c:1581-1598) opens the…
- [Medium] Duplicate view of the same ordering defect from a different
  stage: gmac_napi_poll()…

--

net: ethernet: cortina: Fix budget accounting

gmac_rx() used to return the leftover NAPI budget while gmac_napi_poll()
treated the value as a packet count, so an idle poll reported a full
budget. The patch adds a separate received counter, returns it, and
changes the free queue refill credit to add that count directly.

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

> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 4c762229ce420..1d9824d1716cf 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;

[Severity: Medium]
Should the drop paths in gmac_rx() also count towards received and
budget?  This is the only site that touches either counter, and it is
inside the successful GRO branch.

Descriptors that end up on a drop path still advance the ring read
pointer r but contribute nothing:

  - gmac_skb_if_good_frame() returning NULL (descriptor status error,
    short frame, RX status or checksum error, fragment allocation
    failure) jumps to err_drop
  - a zero DMA mapping jumps to err_drop
  - gmac_get_queue_page() failure does "continue"
  - a non-SOF descriptor with no in-progress skb does put_page() and
    "continue"
  - frag_nr == MAX_SKB_FRAGS jumps to err_drop

Since the loop condition is:

	while (budget && w != r) {

a sustained stream of malformed frames, or frag allocation failures
under memory pressure, lets one poll walk the whole available RX ring
while returning little or no work.  Does that defeat budget enforcement
in gmac_napi_poll() and allow softirq monopolization?

The later patch f27033c2d9b5 ("net: ethernet: cortina: Count dropped
frames as NAPI work") moves budget--/received++ to a common next_desc
EOF path, which appears to address exactly this, so this is noted for
the record.

> @@ -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;
>  	}
                ^^^

[Severity: Medium]
This is a pre-existing issue and not introduced here, but the branch
above discards the return value of napi_complete_done() and then
re-arms the hardware RX interrupt unconditionally:

	if (received < budget) {
		napi_gro_flush(napi, false);
		napi_complete_done(napi, received);
		gmac_enable_rx_irq(napi->dev, 1);
		++port->rx_napi_exits;
	}

The kernel-doc in include/linux/netdevice.h says "Return: false if
device should avoid rearming interrupts", which is how
defer_hard_irqs / gro_flush_timeout / prefer_busy_poll deferral is
implemented in net/core/dev.c:napi_complete_done().

Does this patch make it more visible?  Before the change the branch was
entered based on the inverted value, whereas now every non-budget
exhausting poll, including a fully idle one, calls
napi_complete_done(napi, 0) and re-enables the IRQ regardless of the
answer.

The conditional form arrives later in the series in 8dd58e3d1305
("net: ethernet: cortina: Finish RX updates before NAPI completion") as
"if (received < budget && napi_complete_done(napi, received))", so this
is for the record.

>  
> -	port->freeq_refill += (budget - received);
> +	port->freeq_refill += received;
        ^^^

[Severity: Medium]
This isn't a regression from this patch since the old expression
evaluated to the same frame count, but are the units on the two sides
of this comparison the same?

	freeq_threshold = 1 << (geth->freeq_order - 1);
	...
	port->freeq_refill += received;
	if (port->freeq_refill > freeq_threshold) {

freeq_threshold is expressed in hardware free queue fragment entries,
and geth_fill_freeq() accounts one fragment reference per entry
("count += 1 << fpp_order").  Meanwhile received counts completed
frames only.

Each loop iteration in gmac_rx() consumes exactly one RX descriptor,
and therefore one free queue fragment, as the read pointer shows:

	r++;
	r &= m;
	...
	writew(r, ptr_reg);

Non-final fragments of multi-descriptor frames (freeq_frag_order is
fixed at DEFAULT_RX_BUF_ORDER, about 2 KB, and does not scale with MTU)
and all descriptors taken by the drop paths credit zero refill.  Can
that make the proactive half-empty refill fire late, or effectively
never during a fragment or drop storm, leaving only the SWFQ_EMPTY
threshold interrupt as a backstop and producing avoidable RX drops plus
"HW BUG: zero DMA desc" errors?

The later patch 230757ee03c5 ("net: ethernet: cortina: Count RX
descriptors for freeq refill") adds a freeq_consumed out-parameter that
counts every consumed descriptor, so this is noted for the record.

[Severity: Medium]
This is also pre-existing rather than introduced here, but the port
state updates around this line run after NAPI ownership has been
released.  gmac_napi_poll() opens the writer section first:

	u64_stats_update_begin(&port->rx_stats_syncp);

	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 += received;
	...
	u64_stats_update_end(&port->rx_stats_syncp);

Once napi_complete_done() clears NAPI_STATE_SCHED and
gmac_enable_rx_irq() unmasks the RX interrupt, can a new interrupt on
another CPU start a second poll for the same port while the first is
still in its epilogue?

CPU0 gmac_napi_poll()
	napi_complete_done()	/* clears NAPI_STATE_SCHED */
	gmac_enable_rx_irq()
	/* still inside the rx_stats_syncp write section */

CPU1 gmac_irq()
	napi_schedule(&port->napi)
	  -> net_rx_action -> gmac_napi_poll()
	       u64_stats_update_begin(&port->rx_stats_syncp);

u64_stats_sync requires writers to be mutually exclusive per syncp, and
this driver runs on 32-bit ARM where u64_stats_update_begin() is a real
write_seqcount_begin().  With two overlapping writers, can
gmac_get_stats64() / gmac_get_ethtool_stats() observe an even sequence
mid-update and return torn 64-bit counters?  And can the non-atomic
read-modify-write on port->freeq_refill lose updates and skip the
refill this patch is trying to preserve?

This patch does not create the structure, but it makes the completion
branch run on every non-budget-exhausting poll, including previously
skipped idle polls.  The ordering is corrected later in the series by
8dd58e3d1305 ("net: ethernet: cortina: Finish RX updates before NAPI
completion"), where the stats section and the freeq_refill block
complete before napi_complete_done().

[Severity: Medium]
Same pre-existing ordering point seen from the NAPI documentation side,
not something this patch introduces: the NAPI documentation asks drivers
not to touch their own data structures after releasing ownership,
because another poll instance or napi_disable() may proceed.  Here
port->rx_napi_exits, port->freeq_refill and the closing
u64_stats_update_end() all run after napi_complete_done() and
gmac_enable_rx_irq().  Would it be safer to finish all per-port
accounting before releasing NAPI ownership, as 8dd58e3d1305 ("net:
ethernet: cortina: Finish RX updates before NAPI completion") later
does?

>  	if (port->freeq_refill > freeq_threshold) {
>  		port->freeq_refill -= freeq_threshold;
>  		geth_fill_freeq(geth, true);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-gemini-ethernet-fixes-v2-0-2bbbd598ca6e%40kernel.org

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Incomplete fix: in gmac_rx()
  (drivers/net/ethernet/cortina/gemini.c) the new common accounting…

--

net: ethernet: cortina: Count dropped frames as NAPI work

This moves the budget-- and received++ accounting in gmac_rx() out of the
successful delivery path and into a new common next_desc label, so that
frames which are dropped also consume NAPI budget. The error paths now
jump to next_desc instead of using continue.

> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 6502220362cbd..33e9763b32fe8 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c

[ ... ]

> @@ -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,6 +1560,13 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
>  			put_page(page);
>  
>  		port->stats.rx_dropped++;
> +
> +next_desc:
> +		/* Final or single-descriptor fragment, advance things */
> +		if (word3.bits32 & EOF_BIT) {
> +			budget--;
> +			received++;
> +		}

[Severity: Medium]
Since the accounting is gated on EOF_BIT, does this fully close the case
described in the commit message ("a stream of bad frames can process the
entire receive ring in one poll")?

The loop guard is the only per-descriptor limiter:

	while (budget && w != r) {
		...
		r++;
		r &= m;

so any descriptor that reaches next_desc with EOF_BIT clear still advances
r at no cost to budget. Paths that can do that in gmac_rx() are:

	if (!mapping) {
		netdev_err(netdev,
			   "rxq[%u]: HW BUG: zero DMA desc\n", r);
		goto err_drop;
	}

Here the driver has just concluded the device never wrote the descriptor,
and word3 comes from the same dma_alloc_coherent()-zeroed slot, so EOF_BIT
is clear and err_drop falls through to next_desc. Each such descriptor also
emits one unratelimited netdev_err() from softirq context.

The other uncharged paths are the "could not find mapping" gpage failure on
a non-final fragment, the orphan-continuation case:

		} else if (!skb) {
			put_page(page);
			goto next_desc;
		}

and the frag_nr == MAX_SKB_FRAGS drop when the descriptor is not the final
fragment.

Also, is the frame abandoned by an unexpected SOF counted anywhere?

		if (word3.bits32 & SOF_BIT) {
			if (skb) {
				napi_free_frags(&port->napi);
				port->stats.rx_dropped++;

That bumps rx_dropped and frees the partial frame, so a frame was
definitively dropped, but neither budget nor received changes, which reads
differently from "counts each completed frame as NAPI work whether it was
delivered or dropped".

For a descriptor stream that never presents EOF (zeroed or never-written
descriptors, repeated SOF without EOF), one poll can still walk the whole
snapshot, up to 1 << port->rxq_order entries: 512 with
DEFAULT_GMAC_RXQ_ORDER of 9, and up to 32768 after

		port->rxq_order = min(15, ilog2(rp->rx_pending - 1) + 1);

in gmac_set_ringparam(). The loop does terminate since r converges on the
snapshotted w, and received == 0 lets gmac_napi_poll() call
napi_complete_done(), so this is softirq residency and log flooding rather
than a hang.

Would charging work per descriptor consumed be more robust than inferring
frame completion from a field of a descriptor that may itself be invalid?
Note the later patch in this series that adds the freeq_consumed counter
already tracks a per-descriptor count, and the EOF_BIT gating at next_desc
is still present at the end of the series (230757ee03c5).

The related concern about port->freeq_refill += received diverging from the
number of descriptors the MAC consumed is addressed by the later commit
"net: ethernet: cortina: Count RX descriptors for freeq refill", so that
part is not raised here.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-gemini-ethernet-fixes-v2-0-2bbbd598ca6e%40kernel.org

Re: [PATCH v2 0/5] net: ethernet: cortina: Fix RX budget accounting

[Paolo Abeni]

On 9/3/26 11:45 PM, Linus Walleij wrote:
> Finish RX updates before releasing NAPI ownership, report actual NAPI
> work, charge dropped frames to the poll budget, and drive free-queue
> refills from consumed RX descriptors.
> 
> Track RX drop state across descriptor chains so discarded frames are
> counted exactly once.
> 
> 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>
Sashiko has a couple of comments, but the first one is a giant "this is
solved by the next patch" and I read the 2nd alike "if H/W is broken s/w
accounting could be inaccurate" which in turn is not a big deal.

@Linus, note that the current expectation is for the patch submitter to
provide something alike the above feedback.

/P

Re: [PATCH v2 0/5] net: ethernet: cortina: Fix RX budget accounting

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu, 03 Sep 2026 23:45:28 +0200 you wrote:
> Finish RX updates before releasing NAPI ownership, report actual NAPI
> work, charge dropped frames to the poll budget, and drive free-queue
> refills from consumed RX descriptors.
> 
> Track RX drop state across descriptor chains so discarded frames are
> counted exactly once.
> 
> [...]

Here is the summary with links:
  - [v2,1/5] net: ethernet: cortina: Fix budget accounting
    https://git.kernel.org/netdev/net/c/a0de06d0da78
  - [v2,2/5] net: ethernet: cortina: Finish RX updates before NAPI completion
    https://git.kernel.org/netdev/net/c/baa26841cb9a
  - [v2,3/5] net: ethernet: cortina: Count dropped frames as NAPI work
    https://git.kernel.org/netdev/net/c/b856c552f556
  - [v2,4/5] net: ethernet: cortina: Count RX drops once per frame
    https://git.kernel.org/netdev/net/c/6520198c430c
  - [v2,5/5] net: ethernet: cortina: Count RX descriptors for freeq refill
    https://git.kernel.org/netdev/net/c/e89e88ad41d9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v2 0/5] net: ethernet: cortina: Fix RX budget accounting

[Linus Walleij]

On Tue, Sep 8, 2026 at 12:33 PM Paolo Abeni <pabeni@redhat.com> wrote:

> Sashiko has a couple of comments, but the first one is a giant "this is
> solved by the next patch" and I read the 2nd alike "if H/W is broken s/w
> accounting could be inaccurate" which in turn is not a big deal.
>
> @Linus, note that the current expectation is for the patch submitter to
> provide something alike the above feedback.

Yeah I understood that, I was just not fast enough.

You analysis is correct AFAICT, I will however comb over
the feedback trying to fix up also some of the newly discovered
latent issues ... it's a bit painful and fun at the same time.

Thanks!
Linus Walleij