Re: [PATCH net-next v6 1/7] net: bcmgenet: convert RX path to page_pool

Jakub Kicinski
2026-04-12T19:10:46Z

On Mon, 6 Apr 2026 10:35:25 +0200 Nicolai Buchwitz wrote:
> Replace the per-packet __netdev_alloc_skb() + dma_map_single() in the
> RX path with page_pool, which provides efficient page recycling and
> DMA mapping management. This is a prerequisite for XDP support (which
> requires stable page-backed buffers rather than SKB linear data).
>
> Key changes:
> - Create a page_pool per RX ring (PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV)
> - bcmgenet_rx_refill() allocates pages via page_pool_alloc_pages()
> - bcmgenet_desc_rx() builds SKBs from pages via napi_build_skb() with
> skb_mark_for_recycle() for automatic page_pool return
> - Buffer layout reserves XDP_PACKET_HEADROOM (256 bytes) before the HW
> RSB (64 bytes) + alignment pad (2 bytes) for future XDP headroom

some nits here, since I have more "real" comments on later patches

> +/* Page pool RX buffer layout:
> + * XDP_PACKET_HEADROOM | RSB(64) + pad(2) | frame data | skb_shared_info
> + * The HW writes the 64B RSB + 2B alignment padding before the frame.
> + */
> +#define GENET_XDP_HEADROOM XDP_PACKET_HEADROOM

subjective but IDK what value this define adds vs using
XDP_PACKET_HEADROOM directly.

> +#define GENET_RSB_PAD (sizeof(struct status_64) + 2)
> +#define GENET_RX_HEADROOM (GENET_XDP_HEADROOM + GENET_RSB_PAD)

> +static int bcmgenet_rx_refill(struct bcmgenet_rx_ring *ring,
> + struct enet_cb *cb)
> {
> - struct device *kdev = &priv->pdev->dev;
> - struct sk_buff *skb;
> - struct sk_buff *rx_skb;
> + struct bcmgenet_priv *priv = ring->priv;
> dma_addr_t mapping;
> + struct page *page;
>
> - /* Allocate a new Rx skb */
> - skb = __netdev_alloc_skb(priv->dev, priv->rx_buf_len + SKB_ALIGNMENT,
> - GFP_ATOMIC | __GFP_NOWARN);

page pool adds __GFP_NOWARN automatically, you can drop it now

> - if (!skb) {
> + page = page_pool_alloc_pages(ring->page_pool,
> + GFP_ATOMIC | __GFP_NOWARN);
> + if (!page) {
> priv->mib.alloc_rx_buff_failed++;
> netif_err(priv, rx_err, priv->dev,
> - "%s: Rx skb allocation failed\n", __func__);
> - return NULL;
> - }