[PATCH net-next v2 0/2] pcnet32: switch to napi_alloc_skb() and napi_gro_receive()

[PATCH net-next v2 0/2] pcnet32: switch to napi_alloc_skb() and napi_gro_receive()

[Oscar Maes]

Use the newer and more efficient napi_alloc_skb() and napi_gro_receive()
APIs to improve RX throughput.

In my testing this resulted in up to a 25% RX throughput increase in iperf3
tests in both directions.

Changes in v2:
- Clarified why releasing the spinlock during napi_complete_done is legal

Previous version:
https://lore.kernel.org/netdev/20260520063329.6387-1-oscmaes92@gmail.com/

Oscar Maes (2):
  pcnet32: stop holding device spin lock during napi_complete_done
  pcnet32: switch to napi_alloc_skb() and napi_gro_receive()

 drivers/net/ethernet/amd/pcnet32.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.39.5

[PATCH net-next v2 1/2] pcnet32: stop holding device spin lock during napi_complete_done

[Oscar Maes]

napi_complete_done may call gro_flush_normal (though not currently, as GRO
is unsupported at the moment), which may result in packet TX. This will
eventually result in calling pcnet32_start_xmit - resulting in a deadlock
while trying to re-acquire the already locked spin lock.

It is safe to split the spinlock block into two, because the hardware
registers are still protected from concurrent access, and the two blocks
perform unrelated operations that don't need to happen atomically. The call
to napi_complete_done does not require external locking.

Signed-off-by: Oscar Maes <oscmaes92@gmail.com>
---
 drivers/net/ethernet/amd/pcnet32.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index 911808ab13a7..4f3076d4ea34 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1407,8 +1407,10 @@ static int pcnet32_poll(struct napi_struct *napi, int budget)
 		pcnet32_restart(dev, CSR0_START);
 		netif_wake_queue(dev);
 	}
+	spin_unlock_irqrestore(&lp->lock, flags);
 
 	if (work_done < budget && napi_complete_done(napi, work_done)) {
+		spin_lock_irqsave(&lp->lock, flags);
 		/* clear interrupt masks */
 		val = lp->a->read_csr(ioaddr, CSR3);
 		val &= 0x00ff;
@@ -1416,9 +1418,9 @@ static int pcnet32_poll(struct napi_struct *napi, int budget)
 
 		/* Set interrupt enable. */
 		lp->a->write_csr(ioaddr, CSR0, CSR0_INTEN);
+		spin_unlock_irqrestore(&lp->lock, flags);
 	}
 
-	spin_unlock_irqrestore(&lp->lock, flags);
 	return work_done;
 }
 
-- 
2.39.5

[PATCH net-next v2 2/2] pcnet32: switch to napi_alloc_skb() and napi_gro_receive()

[Oscar Maes]

Use the newer and more efficient napi_alloc_skb() and napi_gro_receive()
APIs to improve RX throughput.

Also removes the manual calls to skb_reserve(NET_IP_ALIGN), since
napi_alloc_skb() already does this internally.

In my testing this resulted in up to a 25% RX throughput increase in iperf3
tests in both directions.

Signed-off-by: Oscar Maes <oscmaes92@gmail.com>
---
 drivers/net/ethernet/amd/pcnet32.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index 4f3076d4ea34..06d5432bb464 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1220,13 +1220,12 @@ static void pcnet32_rx_entry(struct net_device *dev,
 		struct sk_buff *newskb;
 		dma_addr_t new_dma_addr;
 
-		newskb = netdev_alloc_skb(dev, PKT_BUF_SKB);
+		newskb = napi_alloc_skb(&lp->napi, PKT_BUF_SIZE);
 		/*
 		 * map the new buffer, if mapping fails, drop the packet and
 		 * reuse the old buffer
 		 */
 		if (newskb) {
-			skb_reserve(newskb, NET_IP_ALIGN);
 			new_dma_addr = dma_map_single(&lp->pci_dev->dev,
 						      newskb->data,
 						      PKT_BUF_SIZE,
@@ -1251,14 +1250,13 @@ static void pcnet32_rx_entry(struct net_device *dev,
 		} else
 			skb = NULL;
 	} else
-		skb = netdev_alloc_skb(dev, pkt_len + NET_IP_ALIGN);
+		skb = napi_alloc_skb(&lp->napi, pkt_len);
 
 	if (!skb) {
 		dev->stats.rx_dropped++;
 		return;
 	}
 	if (!rx_in_place) {
-		skb_reserve(skb, NET_IP_ALIGN);
 		skb_put(skb, pkt_len);	/* Make room */
 		dma_sync_single_for_cpu(&lp->pci_dev->dev,
 					lp->rx_dma_addr[entry], pkt_len,
@@ -1272,7 +1270,7 @@ static void pcnet32_rx_entry(struct net_device *dev,
 	}
 	dev->stats.rx_bytes += skb->len;
 	skb->protocol = eth_type_trans(skb, dev);
-	netif_receive_skb(skb);
+	napi_gro_receive(&lp->napi, skb);
 	dev->stats.rx_packets++;
 }
 
-- 
2.39.5

Re: [PATCH net-next v2 1/2] pcnet32: stop holding device spin lock during napi_complete_done

[Andrew Lunn]

On Mon, May 25, 2026 at 02:54:36PM +0200, Oscar Maes wrote:
> napi_complete_done may call gro_flush_normal (though not currently, as GRO
> is unsupported at the moment), which may result in packet TX. This will
> eventually result in calling pcnet32_start_xmit - resulting in a deadlock
> while trying to re-acquire the already locked spin lock.
> 
> It is safe to split the spinlock block into two, because the hardware
> registers are still protected from concurrent access, and the two blocks
> perform unrelated operations that don't need to happen atomically. The call
> to napi_complete_done does not require external locking.
> 
> Signed-off-by: Oscar Maes <oscmaes92@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 2/2] pcnet32: switch to napi_alloc_skb() and napi_gro_receive()

[Jakub Kicinski]

On Mon, 25 May 2026 14:54:37 +0200 Oscar Maes wrote:
> In my testing this resulted in up to a 25% RX throughput increase in iperf3
> tests in both directions.

On which platform?

Re: [PATCH net-next v2 1/2] pcnet32: stop holding device spin lock during napi_complete_done

[Jakub Kicinski]

On Mon, 25 May 2026 14:54:36 +0200 Oscar Maes wrote:
> napi_complete_done may call gro_flush_normal (though not currently, as GRO
> is unsupported at the moment), which may result in packet TX. This will
> eventually result in calling pcnet32_start_xmit - resulting in a deadlock
> while trying to re-acquire the already locked spin lock.

Looks like a legit bug fix, please add a Fixes tag and repost for net
(rather than net-next)