Netdev List
 help / color / mirror / Atom feed
* [PATCH net 0/4] net: ethernet: cortina: Fix various RX bugs
@ 2026-05-08 22:13 Linus Walleij
  2026-05-08 22:13 ` [PATCH net 1/4] net: ethernet: cortina: No mapping is a dropped rx Linus Walleij
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Linus Walleij @ 2026-05-08 22:13 UTC (permalink / raw)
  To: Andreas Haarmann-Thiemann, Hans Ulli Kroll, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michał Mirosław
  Cc: netdev, Linus Walleij

During review of a minor patch for a bug in the Cortina
ethernet driver, Sashiko jumped in and pointed out a number
of nasty bugs.

This series hopefully fixes all of them.

Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Linus Walleij (4):
      net: ethernet: cortina: No mapping is a dropped rx
      net: ethernet: cortina: Make RX SKB per-port
      net: ethernet: cortina: Carry over frag counter
      net: ethernet: cortina: Fix budget accounting

 drivers/net/ethernet/cortina/gemini.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)
---
base-commit: 18ae35be3f9ed26f9acff1dd96a9aed026421c5b
change-id: 20260508-gemini-ethernet-fixes-3ab203d28c75

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net 1/4] net: ethernet: cortina: No mapping is a dropped rx
  2026-05-08 22:13 [PATCH net 0/4] net: ethernet: cortina: Fix various RX bugs Linus Walleij
@ 2026-05-08 22:13 ` Linus Walleij
  2026-05-08 22:13 ` [PATCH net 2/4] net: ethernet: cortina: Make RX SKB per-port Linus Walleij
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2026-05-08 22:13 UTC (permalink / raw)
  To: Andreas Haarmann-Thiemann, Hans Ulli Kroll, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michał Mirosław
  Cc: netdev, Linus Walleij

Increase stats.rx_dropped++ even if this is the first fragment
(skb == NULL) so we are doing proper accounting.

Fixes: b266bacba796 ("net: ethernet: cortina: Drop half-assembled SKB")
Link: https://sashiko.dev/#/patchset/20260505-gemini-ethernet-fix-v2-1-997c31d06079%40kernel.org
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/ethernet/cortina/gemini.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 065cbbf52686..466445c9e08b 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1491,9 +1491,9 @@ 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);
-				port->stats.rx_dropped++;
 				skb = NULL;
 			}
 			continue;

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net 2/4] net: ethernet: cortina: Make RX SKB per-port
  2026-05-08 22:13 [PATCH net 0/4] net: ethernet: cortina: Fix various RX bugs Linus Walleij
  2026-05-08 22:13 ` [PATCH net 1/4] net: ethernet: cortina: No mapping is a dropped rx Linus Walleij
@ 2026-05-08 22:13 ` Linus Walleij
  2026-05-08 22:13 ` [PATCH net 3/4] net: ethernet: cortina: Carry over frag counter Linus Walleij
  2026-05-08 22:13 ` [PATCH net 4/4] net: ethernet: cortina: Fix budget accounting Linus Walleij
  3 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2026-05-08 22:13 UTC (permalink / raw)
  To: Andreas Haarmann-Thiemann, Hans Ulli Kroll, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michał Mirosław
  Cc: netdev, Linus Walleij

The SKB used to assemble packets from fragments in gmac_rx()
is static local, but the Gemini has two ethernet ports, meaning
there can be races between the ports on a bad day if a device
is using both.

Make the RX SKB a per-port variable and carry it over between
invocations in the port struct instead.

Zero the pointer once we call napi_gro_frags(), on error (after
calling napi_free_frags()) or if the port is stopped.

Zero it in some place where not strictly necessary just to
emphasize what is going on.

This was found by Sashiko during normal patch review.

Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Link: https://sashiko.dev/#/patchset/20260505-gemini-ethernet-fix-v2-1-997c31d06079%40kernel.org
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/ethernet/cortina/gemini.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 466445c9e08b..d5a56366fb43 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -122,6 +122,8 @@ struct gemini_ethernet_port {
 	struct napi_struct	napi;
 	struct hrtimer		rx_coalesce_timer;
 	unsigned int		rx_coalesce_nsecs;
+	struct sk_buff		*rx_skb;
+
 	unsigned int		freeq_refill;
 	struct gmac_txq		txq[TX_QUEUE_NUM];
 	unsigned int		txq_order;
@@ -1442,10 +1444,10 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 	unsigned short m = (1 << port->rxq_order) - 1;
 	struct gemini_ethernet *geth = port->geth;
 	void __iomem *ptr_reg = port->rxq_rwptr;
+	struct sk_buff *skb = port->rx_skb;
 	unsigned int frame_len, frag_len;
 	struct gmac_rxdesc *rx = NULL;
 	struct gmac_queue_page *gpage;
-	static struct sk_buff *skb;
 	union gmac_rxdesc_0 word0;
 	union gmac_rxdesc_1 word1;
 	union gmac_rxdesc_3 word3;
@@ -1504,6 +1506,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 			if (skb) {
 				napi_free_frags(&port->napi);
 				port->stats.rx_dropped++;
+				skb = NULL;
 			}
 
 			skb = gmac_skb_if_good_frame(port, word0, frame_len);
@@ -1554,6 +1557,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 		port->stats.rx_dropped++;
 	}
 
+	port->rx_skb = skb;
 	writew(r, ptr_reg);
 	return budget;
 }
@@ -1881,6 +1885,7 @@ static int gmac_stop(struct net_device *netdev)
 	gmac_disable_tx_rx(netdev);
 	gmac_stop_dma(port);
 	napi_disable(&port->napi);
+	port->rx_skb = NULL;
 
 	gmac_enable_irq(netdev, 0);
 	gmac_cleanup_rxq(netdev);

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net 3/4] net: ethernet: cortina: Carry over frag counter
  2026-05-08 22:13 [PATCH net 0/4] net: ethernet: cortina: Fix various RX bugs Linus Walleij
  2026-05-08 22:13 ` [PATCH net 1/4] net: ethernet: cortina: No mapping is a dropped rx Linus Walleij
  2026-05-08 22:13 ` [PATCH net 2/4] net: ethernet: cortina: Make RX SKB per-port Linus Walleij
@ 2026-05-08 22:13 ` Linus Walleij
  2026-05-08 22:13 ` [PATCH net 4/4] net: ethernet: cortina: Fix budget accounting Linus Walleij
  3 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2026-05-08 22:13 UTC (permalink / raw)
  To: Andreas Haarmann-Thiemann, Hans Ulli Kroll, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michał Mirosław
  Cc: netdev, Linus Walleij

The gmac_rx() NAPI poll function assembles packets in an
SKB from a ring buffer.

If the ring buffer gets completely emptied during a poll cycle,
we exit gmac_rx(), but the packet is not yet completely
assembled in the SKB, yet the fragment counter frag_nr is
reset to zero on the next invocation.

Solve this by making the RX fragment counter a part of the
port struct, and carry it over between invocations.

Reset the fragment counter only right after calling
napi_gro_frags(), on error (after calling napi_free_frags())
or if stopping the port.

Reset it in some place where not strictly necessary just to
emphasize what is going on.

This was found by Sashiko during normal patch review.

Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Link: https://sashiko.dev/#/patchset/20260505-gemini-ethernet-fix-v2-1-997c31d06079%40kernel.org
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/ethernet/cortina/gemini.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index d5a56366fb43..4c762229ce42 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -123,6 +123,7 @@ struct gemini_ethernet_port {
 	struct hrtimer		rx_coalesce_timer;
 	unsigned int		rx_coalesce_nsecs;
 	struct sk_buff		*rx_skb;
+	unsigned int		rx_frag_nr;
 
 	unsigned int		freeq_refill;
 	struct gmac_txq		txq[TX_QUEUE_NUM];
@@ -1444,6 +1445,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 	unsigned short m = (1 << port->rxq_order) - 1;
 	struct gemini_ethernet *geth = port->geth;
 	void __iomem *ptr_reg = port->rxq_rwptr;
+	unsigned int frag_nr = port->rx_frag_nr;
 	struct sk_buff *skb = port->rx_skb;
 	unsigned int frame_len, frag_len;
 	struct gmac_rxdesc *rx = NULL;
@@ -1457,7 +1459,6 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 	unsigned short r, w;
 	union dma_rwptr rw;
 	dma_addr_t mapping;
-	int frag_nr = 0;
 
 	spin_lock_irqsave(&geth->irq_lock, flags);
 	rw.bits32 = readl(ptr_reg);
@@ -1497,6 +1498,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 			if (skb) {
 				napi_free_frags(&port->napi);
 				skb = NULL;
+				frag_nr = 0;
 			}
 			continue;
 		}
@@ -1507,6 +1509,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 				napi_free_frags(&port->napi);
 				port->stats.rx_dropped++;
 				skb = NULL;
+				frag_nr = 0;
 			}
 
 			skb = gmac_skb_if_good_frame(port, word0, frame_len);
@@ -1541,6 +1544,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 		if (word3.bits32 & EOF_BIT) {
 			napi_gro_frags(&port->napi);
 			skb = NULL;
+			frag_nr = 0;
 			--budget;
 		}
 		continue;
@@ -1549,6 +1553,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget)
 		if (skb) {
 			napi_free_frags(&port->napi);
 			skb = NULL;
+			frag_nr = 0;
 		}
 
 		if (mapping)
@@ -1558,6 +1563,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;
 }
@@ -1886,6 +1892,7 @@ static int gmac_stop(struct net_device *netdev)
 	gmac_stop_dma(port);
 	napi_disable(&port->napi);
 	port->rx_skb = NULL;
+	port->rx_frag_nr = 0;
 
 	gmac_enable_irq(netdev, 0);
 	gmac_cleanup_rxq(netdev);

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net 4/4] net: ethernet: cortina: Fix budget accounting
  2026-05-08 22:13 [PATCH net 0/4] net: ethernet: cortina: Fix various RX bugs Linus Walleij
                   ` (2 preceding siblings ...)
  2026-05-08 22:13 ` [PATCH net 3/4] net: ethernet: cortina: Carry over frag counter Linus Walleij
@ 2026-05-08 22:13 ` Linus Walleij
  3 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2026-05-08 22:13 UTC (permalink / raw)
  To: Andreas Haarmann-Thiemann, Hans Ulli Kroll, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michał Mirosław
  Cc: netdev, Linus Walleij

The gmac_rx() function should return the number of packets received,
not the remaining budget, as it currently does.

In gmac_napi_poll(), this return value is assigned to received,
and if received < budget, the driver calls
napi_complete_done(napi, received).

Fix this by adding a new local variable for the number of received
packets in gmac_rx() and return that instead.

This was found by Sashiko during normal patch review.

Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Link: https://sashiko.dev/#/patchset/20260505-gemini-ethernet-fix-v2-1-997c31d06079%40kernel.org
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/net/ethernet/cortina/gemini.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-08 22:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 22:13 [PATCH net 0/4] net: ethernet: cortina: Fix various RX bugs Linus Walleij
2026-05-08 22:13 ` [PATCH net 1/4] net: ethernet: cortina: No mapping is a dropped rx Linus Walleij
2026-05-08 22:13 ` [PATCH net 2/4] net: ethernet: cortina: Make RX SKB per-port Linus Walleij
2026-05-08 22:13 ` [PATCH net 3/4] net: ethernet: cortina: Carry over frag counter Linus Walleij
2026-05-08 22:13 ` [PATCH net 4/4] net: ethernet: cortina: Fix budget accounting Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox