[PATCH net-next 0/3] net: stmmac: page pool Tx buffer quirk

[PATCH net-next 0/3] net: stmmac: page pool Tx buffer quirk

[muhammad.nazim.amirul.nazle.asmade]

From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

This series adds optional use of page pool buffers for stmmac transmit
instead of per-packet streaming dma map/unmap.

On platforms with an IOMMU, the invalidate cost of map/unmap can dominate
small-packet Tx performance. Copying into recycled, already-mapped page
pool buffers can be cheaper. The feature is opt-in via a new DT flag
snps,pagepool-tx-buf-quirk, and is enabled for Agilex5 GMACs.

Page pool Tx buffers are only used for single-fragment, non-TSO frames.

Patches 1-2 are based on netdev/net-next. Patch 3 (DTS) is based on
Dinh Nguyen's socfpga for-next tree.

Nazim Amirul (3):
  dt-bindings: net: snps,dwmac: Add snps,pagepool-tx-buf-quirk
  net: stmmac: Use pagepool instead of streaming buffers for Tx
  arm64: dts: agilex5: Enable pagepool Tx buffer quirk

 .../devicetree/bindings/net/snps,dwmac.yaml   |  8 +++
 .../boot/dts/intel/socfpga_agilex5.dtsi       |  3 +
 .../net/ethernet/stmicro/stmmac/stmmac.h      |  3 +
 .../ethernet/stmicro/stmmac/stmmac_main.c     | 75 +++++++++++++++++--
 .../ethernet/stmicro/stmmac/stmmac_platform.c |  2 +
 include/linux/stmmac.h                        |  1 +
 6 files changed, 81 insertions(+), 11 deletions(-)

-- 
2.43.7

[PATCH net-next 1/3] dt-bindings: net: snps,dwmac: Add snps,pagepool-tx-buf-quirk

[muhammad.nazim.amirul.nazle.asmade]

From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

Add a DT flag to enable use of page pool buffers instead of streaming
skb buffers for transmit. This avoids IOMMU invalidate wait time on
platforms where that cost is high, with the trade-off of copying data
from the skb into dma-mapped page pool buffers.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 2449311c6d28..73045511446f 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -545,6 +545,14 @@ properties:
       Enables the TSO feature otherwise it will be managed by MAC HW capability
       register.
 
+  snps,pagepool-tx-buf-quirk:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Enable use of page pool buffers instead of streaming buffers for transmit.
+      This avoids IOMMU invalidate wait time on platforms where that cost is
+      high, with the trade-off of copying data from the skb into dma-mapped
+      page pool buffers.
+
   mdio:
     $ref: mdio.yaml#
     unevaluatedProperties: false
-- 
2.43.7

Re: [PATCH net-next 1/3] dt-bindings: net: snps,dwmac: Add snps,pagepool-tx-buf-quirk

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The property `snps,pagepool-tx-buf-quirk` encodes Linux-specific software implementation details ('page pool' and 'skb') and dictates software memory management policy, violating the core rule that Device Tree bindings must remain OS-agnostic and describe only hardware.
- [Low] The patch adds a new Device Tree flag (`snps,pagepool-tx-buf-quirk`) without any corresponding driver code in the series to parse or handle the property.
--

--- Patch [1]: [PATCH net-next 1/3] dt-bindings: net: snps,dwmac: Add snps,pagepool-tx-buf-quirk ---
commit 8d997cfca114fafc51861d3c95f903b66f45af7f
Author: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

dt-bindings: net: snps,dwmac: Add snps,pagepool-tx-buf-quirk

This commit adds a new device tree flag to enable the use of page pool buffers
instead of streaming skb buffers for transmit. This aims to avoid high IOMMU
invalidate wait times on certain platforms.

> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

[ ... ]

> @@ -545,6 +545,14 @@ properties:
>        Enables the TSO feature otherwise it will be managed by MAC HW capability
>        register.
>  
> +  snps,pagepool-tx-buf-quirk:
> +    $ref: /schemas/types.yaml#/definitions/flag

[Severity: Low]
Is this flag used by any driver code in this series? 

Adding a device tree binding without the corresponding driver implementation
makes it difficult to review how the property is actually consumed by the
kernel.

> +    description:
> +      Enable use of page pool buffers instead of streaming buffers for transmit.
> +      This avoids IOMMU invalidate wait time on platforms where that cost is
> +      high, with the trade-off of copying data from the skb into dma-mapped
> +      page pool buffers.

[Severity: Medium]
Does this description violate the rule that device tree bindings should remain
OS-agnostic? 

The terms page pool buffers and skb encode Linux-specific networking stack
implementation details. Should this property instead describe the underlying
hardware characteristics, such as the high IOMMU latency itself, rather than
dictating a software memory management policy?

> +
>    mdio:
>      $ref: mdio.yaml#
>      unevaluatedProperties: false

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803070513.4657-1-muhammad.nazim.amirul.nazle.asmade@altera.com?part=1

Re: [PATCH net-next 1/3] dt-bindings: net: snps,dwmac: Add snps,pagepool-tx-buf-quirk

[Krzysztof Kozlowski]

On Mon, Aug 03, 2026 at 12:05:11AM -0700, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> 
> Add a DT flag to enable use of page pool buffers instead of streaming
> skb buffers for transmit. This avoids IOMMU invalidate wait time on
> platforms where that cost is high, with the trade-off of copying data
> from the skb into dma-mapped page pool buffers.
> 
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 2449311c6d28..73045511446f 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -545,6 +545,14 @@ properties:
>        Enables the TSO feature otherwise it will be managed by MAC HW capability
>        register.
>  
> +  snps,pagepool-tx-buf-quirk:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:

You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.

> +      Enable use of page pool buffers instead of streaming buffers for transmit.
> +      This avoids IOMMU invalidate wait time on platforms where that cost is
> +      high, with the trade-off of copying data from the skb into dma-mapped

Best regards,
Krzysztof

[PATCH net-next 2/3] net: stmmac: Use pagepool instead of streaming buffers for Tx

[muhammad.nazim.amirul.nazle.asmade]

From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

Use page pool buffers instead of streaming skb buffers in stmmac_xmit
when snps,pagepool-tx-buf-quirk is enabled. As page pool buffers are
recycled, the overhead of dma map/unmap for each transmit can be
avoided. There is a trade-off of copying from the skb into the page
pool buffer, but this helps where the copy is cheaper than dma
map/unmap (for example with IOMMU invalidate cost).

Currently page pool buffers are only used for single-fragment transmit
in the non-TSO path.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  3 +
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 75 ++++++++++++++++---
 .../ethernet/stmicro/stmmac/stmmac_platform.c |  2 +
 include/linux/stmmac.h                        |  1 +
 4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 8ba8f03e1ce0..093c052b4908 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -41,6 +41,7 @@ struct stmmac_resources {
 
 enum stmmac_txbuf_type {
 	STMMAC_TXBUF_T_SKB,
+	STMMAC_TXBUF_T_DMA,
 	STMMAC_TXBUF_T_XDP_TX,
 	STMMAC_TXBUF_T_XDP_NDO,
 	STMMAC_TXBUF_T_XSK_TX,
@@ -49,6 +50,7 @@ enum stmmac_txbuf_type {
 struct stmmac_tx_info {
 	struct xsk_tx_metadata_compl xsk_meta;
 	dma_addr_t buf;
+	struct page *page;
 	unsigned len;
 	enum stmmac_txbuf_type buf_type;
 	bool map_as_page;
@@ -65,6 +67,7 @@ struct stmmac_tx_queue {
 	int tbs;
 	struct hrtimer txtimer;
 	u32 queue_index;
+	struct page_pool *page_pool;
 	struct stmmac_priv *priv_data;
 	struct dma_extended_desc *dma_etx ____cacheline_aligned_in_smp;
 	struct dma_edesc *dma_entx;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ee44bd6f4d48..4aa6b1d1507b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1712,7 +1712,10 @@ static void stmmac_free_tx_buffer(struct stmmac_priv *priv,
 
 	if (tx_q->tx_skbuff_dma[i].buf &&
 	    tx_q->tx_skbuff_dma[i].buf_type != STMMAC_TXBUF_T_XDP_TX) {
-		if (tx_q->tx_skbuff_dma[i].map_as_page)
+		if (tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_DMA)
+			page_pool_recycle_direct(tx_q->page_pool,
+						 tx_q->tx_skbuff_dma[i].page);
+		else if (tx_q->tx_skbuff_dma[i].map_as_page)
 			dma_unmap_page(priv->device,
 				       tx_q->tx_skbuff_dma[i].buf,
 				       tx_q->tx_skbuff_dma[i].len,
@@ -1735,7 +1738,8 @@ static void stmmac_free_tx_buffer(struct stmmac_priv *priv,
 		tx_q->xsk_frames_done++;
 
 	if (tx_q->tx_skbuff[i] &&
-	    tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_SKB) {
+	    (tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_SKB ||
+	     tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_DMA)) {
 		dev_kfree_skb_any(tx_q->tx_skbuff[i]);
 		tx_q->tx_skbuff[i] = NULL;
 	}
@@ -2201,6 +2205,8 @@ static void __free_dma_tx_desc_resources(struct stmmac_priv *priv,
 
 	kfree(tx_q->tx_skbuff_dma);
 	kfree(tx_q->tx_skbuff);
+	if (tx_q->page_pool)
+		page_pool_destroy(tx_q->page_pool);
 }
 
 static void free_dma_tx_desc_resources(struct stmmac_priv *priv,
@@ -2335,12 +2341,32 @@ static int __alloc_dma_tx_desc_resources(struct stmmac_priv *priv,
 					 u32 queue)
 {
 	struct stmmac_tx_queue *tx_q = &dma_conf->tx_queue[queue];
+	struct page_pool_params pp_params = { 0 };
+	unsigned int num_pages;
 	size_t size;
 	void *addr;
+	int ret;
 
 	tx_q->queue_index = queue;
 	tx_q->priv_data = priv;
 
+	pp_params.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
+	pp_params.pool_size = dma_conf->dma_tx_size;
+	num_pages = DIV_ROUND_UP(dma_conf->dma_buf_sz, PAGE_SIZE);
+	pp_params.order = ilog2(num_pages);
+	pp_params.nid = dev_to_node(priv->device);
+	pp_params.dev = priv->device;
+	pp_params.dma_dir = DMA_BIDIRECTIONAL;
+	pp_params.offset = 0;
+	pp_params.max_len = num_pages * PAGE_SIZE;
+
+	tx_q->page_pool = page_pool_create(&pp_params);
+	if (IS_ERR(tx_q->page_pool)) {
+		ret = PTR_ERR(tx_q->page_pool);
+		tx_q->page_pool = NULL;
+		return ret;
+	}
+
 	tx_q->tx_skbuff_dma = kzalloc_objs(*tx_q->tx_skbuff_dma,
 					   dma_conf->dma_tx_size);
 	if (!tx_q->tx_skbuff_dma)
@@ -2836,7 +2862,8 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue,
 		    tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_XDP_NDO) {
 			xdpf = tx_q->xdpf[entry];
 			skb = NULL;
-		} else if (tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_SKB) {
+		} else if (tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_SKB ||
+			   tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_DMA) {
 			xdpf = NULL;
 			skb = tx_q->tx_skbuff[entry];
 		} else {
@@ -2884,7 +2911,10 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue,
 
 		if (likely(tx_q->tx_skbuff_dma[entry].buf &&
 			   tx_q->tx_skbuff_dma[entry].buf_type != STMMAC_TXBUF_T_XDP_TX)) {
-			if (tx_q->tx_skbuff_dma[entry].map_as_page)
+			if (tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_DMA)
+				page_pool_recycle_direct(tx_q->page_pool,
+							 tx_q->tx_skbuff_dma[entry].page);
+			else if (tx_q->tx_skbuff_dma[entry].map_as_page)
 				dma_unmap_page(priv->device,
 					       tx_q->tx_skbuff_dma[entry].buf,
 					       tx_q->tx_skbuff_dma[entry].len,
@@ -2922,7 +2952,8 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue,
 		if (tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_XSK_TX)
 			tx_q->xsk_frames_done++;
 
-		if (tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_SKB) {
+		if (tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_SKB ||
+		    tx_q->tx_skbuff_dma[entry].buf_type == STMMAC_TXBUF_T_DMA) {
 			if (likely(skb)) {
 				pkts_compl++;
 				bytes_compl += skb->len;
@@ -4742,12 +4773,14 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	bool enh_desc, has_vlan, set_ic, is_jumbo = false;
 	struct stmmac_priv *priv = netdev_priv(dev);
 	unsigned int nopaged_len = skb_headlen(skb);
+	gfp_t gfp = GFP_ATOMIC | __GFP_NOWARN;
 	u32 queue = skb_get_queue_mapping(skb);
 	int nfrags = skb_shinfo(skb)->nr_frags;
 	unsigned int first_entry, tx_packets;
 	struct stmmac_txq_stats *txq_stats;
 	struct dma_desc *desc, *first_desc;
 	struct stmmac_tx_queue *tx_q;
+	struct page *tx_buf_page;
 	int i, csum_insertion = 0;
 	int entry, first_tx;
 	dma_addr_t dma_addr;
@@ -4814,13 +4847,33 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	} else {
 		bool last_segment = (nfrags == 0);
 
-		dma_addr = dma_map_single(priv->device, skb->data,
-					  nopaged_len, DMA_TO_DEVICE);
-		if (dma_mapping_error(priv->device, dma_addr))
-			goto dma_map_err;
+		if (!(last_segment && priv->plat->tx_buf_quirk)) {
+			dma_addr = dma_map_single(priv->device, skb->data,
+						  nopaged_len, DMA_TO_DEVICE);
+			if (dma_mapping_error(priv->device, dma_addr))
+				goto dma_map_err;
 
-		stmmac_set_tx_skb_dma_entry(tx_q, first_entry, dma_addr,
-					    nopaged_len, false);
+			stmmac_set_tx_skb_dma_entry(tx_q, first_entry, dma_addr,
+						    nopaged_len, false);
+		} else {
+			tx_buf_page = page_pool_alloc_pages(tx_q->page_pool, gfp);
+			if (!tx_buf_page)
+				goto dma_map_err;
+
+			dma_addr = page_pool_get_dma_addr(tx_buf_page);
+			stmmac_set_tx_dma_entry(tx_q, first_entry,
+						STMMAC_TXBUF_T_DMA, dma_addr,
+						nopaged_len, false);
+			tx_q->tx_skbuff_dma[first_entry].page = tx_buf_page;
+
+			dma_sync_single_for_cpu(priv->device, dma_addr,
+						nopaged_len, DMA_BIDIRECTIONAL);
+			skb_copy_from_linear_data(skb, page_address(tx_buf_page),
+						  nopaged_len);
+			dma_sync_single_for_device(priv->device, dma_addr,
+						   nopaged_len,
+						   DMA_BIDIRECTIONAL);
+		}
 
 		stmmac_set_desc_addr(priv, first_desc, dma_addr);
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index dc5f951a311d..0f8d2fc61ce4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -575,6 +575,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 
 	of_property_read_u32(np, "snps,ps-speed", &plat->mac_port_sel_speed);
 
+	plat->tx_buf_quirk = of_property_read_bool(np, "snps,pagepool-tx-buf-quirk");
+
 	plat->axi = stmmac_axi_setup(pdev);
 
 	rc = stmmac_mtl_setup(pdev, plat);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 4430b967abde..d4c9d77a6f81 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -268,6 +268,7 @@ struct plat_stmmacenet_data {
 	u8 tx_queues_to_use;
 	u8 rx_sched_algorithm;
 	u8 tx_sched_algorithm;
+	bool tx_buf_quirk;
 	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
 	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
 	void (*get_interfaces)(struct stmmac_priv *priv, void *bsp_priv,
-- 
2.43.7

Re: [PATCH net-next 2/3] net: stmmac: Use pagepool instead of streaming buffers for Tx

[Mina Almasry]

On Mon, Aug 3, 2026 at 12:08 AM
<muhammad.nazim.amirul.nazle.asmade@altera.com> wrote:
>
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
>
> Use page pool buffers instead of streaming skb buffers in stmmac_xmit
> when snps,pagepool-tx-buf-quirk is enabled. As page pool buffers are
> recycled, the overhead of dma map/unmap for each transmit can be
> avoided. There is a trade-off of copying from the skb into the page
> pool buffer, but this helps where the copy is cheaper than dma
> map/unmap (for example with IOMMU invalidate cost).
>
> Currently page pool buffers are only used for single-fragment transmit
> in the non-TSO path.
>
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

Whoa!

I haven't looked at the patch deeply to be honest, but using the
page-pool for tx is a bit interesting. There is no other code that
uses it like that I think. I can't think of any blockers off the top
of my head, but I think there are places where we keep a global list
of created page_pools on the system/netdev and the code parses those
lists may be assuming it's a normal page-pool, which is a pp used for
rx and attached to the an rx queue.

Just Ccing the maintainers of pp to know about this use case. I can't
think of issues again, but I wonder if a more-expert has thoughts.

-- 
Thanks,
Mina

Re: [PATCH net-next 2/3] net: stmmac: Use pagepool instead of streaming buffers for Tx

[Mina Almasry]

On Mon, Aug 3, 2026 at 11:39 AM Mina Almasry <almasrymina@google.com> wrote:
>
> On Mon, Aug 3, 2026 at 12:08 AM
> <muhammad.nazim.amirul.nazle.asmade@altera.com> wrote:
> >
> > From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> >
> > Use page pool buffers instead of streaming skb buffers in stmmac_xmit
> > when snps,pagepool-tx-buf-quirk is enabled. As page pool buffers are
> > recycled, the overhead of dma map/unmap for each transmit can be
> > avoided. There is a trade-off of copying from the skb into the page
> > pool buffer, but this helps where the copy is cheaper than dma
> > map/unmap (for example with IOMMU invalidate cost).
> >
> > Currently page pool buffers are only used for single-fragment transmit
> > in the non-TSO path.
> >
> > Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
>
> Whoa!
>
> I haven't looked at the patch deeply to be honest, but using the
> page-pool for tx is a bit interesting. There is no other code that
> uses it like that I think. I can't think of any blockers off the top
> of my head, but I think there are places where we keep a global list
> of created page_pools on the system/netdev and the code parses those
> lists may be assuming it's a normal page-pool, which is a pp used for
> rx and attached to the an rx queue.
>
> Just Ccing the maintainers of pp to know about this use case. I can't
> think of issues again, but I wonder if a more-expert has thoughts.
>

Ah, yes, gemini reminded me of the most critical bit. The page_pool
actually doesn't do concurrency checks on allocation because it
assumes allocations only-ever come from the napi, and can't race with
allocations coming from another napi. Does your patch account for that
somehow? Or can allocations in tx come from multiple cpus in parallel?

Full feedback:

```
  ### 1. Concurrency & Memory Corruption (The Lockless Fast-Path)

  page_pool allocations (page_pool_alloc_pages()) are lockless in the
fast path (pool->alloc cache). They inherently assume exactly one
allocator context (which natively maps to a single
  RX NAPI softirq).
  If this driver allows multiple CPUs to concurrently enter the TX
routine and allocate from the same page_pool, the lockless cache will
immediately corrupt. To use it in TX, they must
  guarantee:

  • They allocate 1 dedicated page_pool per TX ring.
  • The TX ring is strictly protected by a spinlock (e.g.
__netif_tx_lock) during all allocations, or they are utilizing strict
CPU-bound XPS.

  ### 2. Slow-Path Contention (ptr_ring Thrashing)

  Even if the locking is safe, performance will likely tank due to
NAPI mismatches.
  page_pool keeps its speed because pages freed by the NAPI context
are dropped directly back into the lockless alloc array. If a page is
freed by a remote CPU or an unbound context, it
  falls back to a spinlock-protected ptr_ring.
  In a TX flow, if CPU A injects the packet (ndo_start_xmit) but CPU B
handles the hardware TX-completion IRQ, CPU B will constantly push
pages into the locked ptr_ring, and CPU A will
  constantly pull from it. You lose all the lockless benefits of
page_pool and just burn cycles on cross-CPU spinlocks.

  ### 3. Memory Waste (Page Splitting)

  TX injections are usually small (e.g., bouncing a fragmented
header). page_pool allocates strictly in PAGE_SIZE chunks unless you
heavily orchestrate page_pool_alloc_frag(). Standard
  SLUB kmalloc() or skb_alloc() is almost always cheaper, more memory
efficient, and more cache-friendly for TX bounce-buffers than spinning
up dedicated page pools.

  TL;DR: Unless the TX allocations and the TX completions are strictly
pinned to the identical NAPI context/CPU, and they are allocating
massive buffers, throwing page_pool at the TX
  path usually results in lock contention and memory inefficiency.
They are almost certainly better off using standard kernel allocators,
or at minimum mapping DMA_TO_DEVICE correctly
  over standard per-CPU locked pools.
```

-- 
Thanks,
Mina

Re: [PATCH net-next 2/3] net: stmmac: Use pagepool instead of streaming buffers for Tx

[sashiko-bot]

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

New issues:
- [High] `page_pool_recycle_direct()` is called outside of NAPI context, leading to potential lockless ring corruption.
- [Medium] Unnecessary `PP_FLAG_DMA_SYNC_DEV` flag causes redundant full-page DMA syncs, defeating performance goals.

Pre-existing issues:
- [High] Unconditional NULL pointer dereference in `dma_free_tx_skbufs` if `tx_skbuff_dma` allocation fails.
- [High] DMA mapping and `page_pool` memory leak when `skb_frag_dma_map()` fails during transmission.
- [High] Double free and Use-After-Free of TX queue resources due to dangling pointers on teardown.
--

--- Patch [2]: [PATCH net-next 2/3] net: stmmac: Use pagepool instead of streaming buffers for Tx ---
commit a91ef91a303112e59c32621c9138a02b37e37769
Author: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Subject: net: stmmac: Use pagepool instead of streaming buffers for Tx

Use page pool buffers instead of streaming skb buffers in stmmac_xmit
when snps,pagepool-tx-buf-quirk is enabled. As page pool buffers are
recycled, the overhead of dma map/unmap for each transmit can be avoided.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ee44bd6f4d48e..4aa6b1d1507bf 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1712,7 +1712,10 @@ static void stmmac_free_tx_buffer(struct stmmac_priv *priv,
>  
>  	if (tx_q->tx_skbuff_dma[i].buf &&
>  	    tx_q->tx_skbuff_dma[i].buf_type != STMMAC_TXBUF_T_XDP_TX) {

[Severity: High]
This is a pre-existing issue, but is it possible for this code to trigger a
NULL pointer dereference?

If the memory allocation for tx_q->tx_skbuff_dma fails in
__alloc_dma_tx_desc_resources() (in stmmac_main.c), it returns -ENOMEM. The
error path then calls free_dma_tx_desc_resources(), which loops over the
queues and calls __free_dma_tx_desc_resources(). This ultimately calls
dma_free_tx_skbufs(), which unconditionally indexes into
tx_q->tx_skbuff_dma[i] via stmmac_free_tx_buffer().

Since tx_q->tx_skbuff_dma is NULL, would this result in a panic during
device setup or XDP re-initialization on memory pressure?

> -		if (tx_q->tx_skbuff_dma[i].map_as_page)
> +		if (tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_DMA)
> +			page_pool_recycle_direct(tx_q->page_pool,
> +						 tx_q->tx_skbuff_dma[i].page);

[Severity: High]
Is it safe to call page_pool_recycle_direct() here outside of NAPI context?

The page_pool_recycle_direct() API requires execution in NAPI softirq context
on the same CPU, as it bypasses locking (allow_direct=true). However,
stmmac_free_tx_buffer() is called by dma_free_tx_skbufs() during device
teardown (process context) and by stmmac_tx_err(), which is triggered from
stmmac_napi_check() directly inside the hardirq handler when a TX error
occurs.

Could calling this lockless API from hardirq or process contexts race with
NAPI polling and lead to internal page pool ring corruption?

> +		else if (tx_q->tx_skbuff_dma[i].map_as_page)
>  			dma_unmap_page(priv->device,
>  				       tx_q->tx_skbuff_dma[i].buf,
>  				       tx_q->tx_skbuff_dma[i].len,
[ ... ]
> @@ -2201,6 +2205,8 @@ static void __free_dma_tx_desc_resources(struct stmmac_priv *priv,
>  
>  	kfree(tx_q->tx_skbuff_dma);
>  	kfree(tx_q->tx_skbuff);
> +	if (tx_q->page_pool)
> +		page_pool_destroy(tx_q->page_pool);

[Severity: High]
This is a pre-existing issue, but does this code leave dangling pointers that
could cause a double free or use-after-free on teardown?

Memory arrays (tx_q->tx_skbuff, tx_q->tx_skbuff_dma) and the newly added
tx_q->page_pool are freed or destroyed, but their pointers are not nullified.
If a subsequent resource allocation fails (e.g. during stmmac_xdp_open()),
the error path calls free_dma_tx_desc_resources(), which loops over all
queues.

For queues that were not successfully re-initialized, it re-executes kfree()
and page_pool_destroy() on the stale, dangling pointers.

>  }
>  
>  static void free_dma_tx_desc_resources(struct stmmac_priv *priv,
[ ... ]
> @@ -2335,12 +2341,32 @@ static int __alloc_dma_tx_desc_resources(struct stmmac_priv *priv,
>  					 u32 queue)
>  {
>  	struct stmmac_tx_queue *tx_q = &dma_conf->tx_queue[queue];
> +	struct page_pool_params pp_params = { 0 };
> +	unsigned int num_pages;
>  	size_t size;
>  	void *addr;
> +	int ret;
>  
>  	tx_q->queue_index = queue;
>  	tx_q->priv_data = priv;
>  
> +	pp_params.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;

[Severity: Medium]
Does using PP_FLAG_DMA_SYNC_DEV cause redundant full-page DMA syncs?

This flag instructs the page pool to automatically synchronize the entire
allocated size (up to 16KB) for the device. However, as seen later in
stmmac_xmit(), the driver manually calls dma_sync_single_for_cpu() and
dma_sync_single_for_device() on just the packet payload length (nopaged_len).

Would this result in double-syncing where the page pool redundantly syncs the
full page on every allocation, reducing the intended performance benefit?

> +	pp_params.pool_size = dma_conf->dma_tx_size;
> +	num_pages = DIV_ROUND_UP(dma_conf->dma_buf_sz, PAGE_SIZE);
[ ... ]
> @@ -4742,12 +4773,14 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	bool enh_desc, has_vlan, set_ic, is_jumbo = false;
>  	struct stmmac_priv *priv = netdev_priv(dev);
>  	unsigned int nopaged_len = skb_headlen(skb);
> +	gfp_t gfp = GFP_ATOMIC | __GFP_NOWARN;
>  	u32 queue = skb_get_queue_mapping(skb);
>  	int nfrags = skb_shinfo(skb)->nr_frags;
>  	unsigned int first_entry, tx_packets;
>  	struct stmmac_txq_stats *txq_stats;
>  	struct dma_desc *desc, *first_desc;
>  	struct stmmac_tx_queue *tx_q;
> +	struct page *tx_buf_page;
>  	int i, csum_insertion = 0;
>  	int entry, first_tx;
>  	dma_addr_t dma_addr;
> @@ -4814,13 +4847,33 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	} else {
>  		bool last_segment = (nfrags == 0);
>  
> -		dma_addr = dma_map_single(priv->device, skb->data,
> -					  nopaged_len, DMA_TO_DEVICE);
> -		if (dma_mapping_error(priv->device, dma_addr))
> -			goto dma_map_err;
> +		if (!(last_segment && priv->plat->tx_buf_quirk)) {
> +			dma_addr = dma_map_single(priv->device, skb->data,
> +						  nopaged_len, DMA_TO_DEVICE);
> +			if (dma_mapping_error(priv->device, dma_addr))
> +				goto dma_map_err;
>  
> -		stmmac_set_tx_skb_dma_entry(tx_q, first_entry, dma_addr,
> -					    nopaged_len, false);
> +			stmmac_set_tx_skb_dma_entry(tx_q, first_entry, dma_addr,
> +						    nopaged_len, false);
> +		} else {
> +			tx_buf_page = page_pool_alloc_pages(tx_q->page_pool, gfp);
> +			if (!tx_buf_page)
> +				goto dma_map_err;
> +
> +			dma_addr = page_pool_get_dma_addr(tx_buf_page);
> +			stmmac_set_tx_dma_entry(tx_q, first_entry,
> +						STMMAC_TXBUF_T_DMA, dma_addr,
> +						nopaged_len, false);
> +			tx_q->tx_skbuff_dma[first_entry].page = tx_buf_page;
> +
> +			dma_sync_single_for_cpu(priv->device, dma_addr,
> +						nopaged_len, DMA_BIDIRECTIONAL);
> +			skb_copy_from_linear_data(skb, page_address(tx_buf_page),
> +						  nopaged_len);
> +			dma_sync_single_for_device(priv->device, dma_addr,
> +						   nopaged_len,
> +						   DMA_BIDIRECTIONAL);
> +		}
>  
>  		stmmac_set_desc_addr(priv, first_desc, dma_addr);
>  

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
stmmac_xmit() leak DMA mappings and memory when skb_frag_dma_map() fails?

If mapping an SKB fragment fails lower down in stmmac_xmit() via
skb_frag_dma_map(), the code jumps to the dma_map_err label, frees the SKB,
and returns. However, it fails to unmap previously mapped fragments or the
linear segment, and does not advance the tx_q->cur_tx ring index.

Because the index is not advanced, stmmac_tx_clean() will never process these
leaked entries, and the next packet transmitted will silently overwrite
tx_q->tx_skbuff_dma[first_entry]. For the newly added page pool buffer, this
permanently leaks the underlying physical page allocated from
page_pool_alloc_pages(), which could deplete system memory over time.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803070513.4657-1-muhammad.nazim.amirul.nazle.asmade@altera.com?part=2

[PATCH 3/3] arm64: dts: agilex5: Enable pagepool Tx buffer quirk

[muhammad.nazim.amirul.nazle.asmade]

From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

Enable snps,pagepool-tx-buf-quirk for Agilex5 gmac0/1/2. These ports
use an IOMMU, where avoiding per-packet dma map/unmap invalidate cost
improves Tx performance.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
 arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
index f54767d1526e..4ce4fa091385 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
@@ -605,6 +605,7 @@ gmac0: ethernet@10810000 {
 			snps,mtl-tx-config = <&mtl_tx_emac0_setup>;
 			snps,pbl = <32>;
 			snps,tso;
+			snps,pagepool-tx-buf-quirk;
 			altr,sysmgr-syscon = <&sysmgr 0x44 0>;
 			snps,clk-csr = <0>;
 			iommus = <&smmu 1>;
@@ -751,6 +752,7 @@ gmac1: ethernet@10820000 {
 			snps,mtl-tx-config = <&mtl_tx_emac1_setup>;
 			snps,pbl = <32>;
 			snps,tso;
+			snps,pagepool-tx-buf-quirk;
 			altr,sysmgr-syscon = <&sysmgr 0x48 0>;
 			snps,clk-csr = <0>;
 			iommus = <&smmu 2>;
@@ -897,6 +899,7 @@ gmac2: ethernet@10830000 {
 			snps,mtl-tx-config = <&mtl_tx_emac2_setup>;
 			snps,pbl = <32>;
 			snps,tso;
+			snps,pagepool-tx-buf-quirk;
 			altr,sysmgr-syscon = <&sysmgr 0x4c 0>;
 			snps,clk-csr = <0>;
 			iommus = <&smmu 3>;
-- 
2.43.7

Re: [PATCH 3/3] arm64: dts: agilex5: Enable pagepool Tx buffer quirk

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unsafe use of page_pool_recycle_direct() from hardirq context causes fast-cache corruption.
- [Medium] Redundant full-page DMA sync due to PP_FLAG_DMA_SYNC_DEV on a TX bounce buffer.
- [Low] Encoding a software optimization policy into Devicetree.
--

--- Patch [3]: [PATCH 3/3] arm64: dts: agilex5: Enable pagepool Tx buffer quirk ---
commit 4b5b56ba27a76622b7269e1715c07a2f16e86382
Author: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>

arm64: dts: agilex5: Enable pagepool Tx buffer quirk

Enable snps,pagepool-tx-buf-quirk for Agilex5 gmac0/1/2. These ports
use an IOMMU, where avoiding per-packet dma map/unmap invalidate cost
improves Tx performance.

> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> index b06c6d5d60eed..525e6a2319c10 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> @@ -574,6 +574,7 @@ gmac0: ethernet@10810000 {
>  			snps,mtl-tx-config = <&mtl_tx_emac0_setup>;
>  			snps,pbl = <32>;
>  			snps,tso;
> +			snps,pagepool-tx-buf-quirk;

[Severity: Low]
Does a software optimization policy belong in Devicetree? Usually, Devicetree
describes hardware topology rather than software configuration like this quirk.

[Severity: High]
By enabling this quirk, does this expose a fast-cache corruption regression
in the driver?

When a hardware Tx error occurs, the hardirq handler calls stmmac_tx_err(),
which iterates buffers and eventually calls:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_free_tx_buffer()
			page_pool_recycle_direct(tx_q->page_pool,
						 tx_q->tx_skbuff_dma[i].page);

If this interrupts napi softirq running on the same CPU, or races with napi
on another CPU, won't calling the napi-only page_pool_recycle_direct()
without the Tx lock corrupt the lockless page_pool fast cache?

[Severity: Medium]
Will enabling this quirk introduce redundant performance overhead that defeats
its purpose?

The driver initializes the Tx page pool with PP_FLAG_DMA_SYNC_DEV:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:__alloc_dma_tx_desc_resources()
	pp_params.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;

This causes a full 4KB page sync on every recycle. However, the driver
already performs a precise sync manually:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_xmit()
			dma_sync_single_for_device(priv->device, dma_addr,
						   nopaged_len,
						   DMA_BIDIRECTIONAL);

Could the sync upon recycle be syncing stale data before the next packet is
written, making the PP_FLAG_DMA_SYNC_DEV flag redundant?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803070513.4657-1-muhammad.nazim.amirul.nazle.asmade@altera.com?part=3