Re: [PATCH net-next v3 3/8] net: macb: Add page pool support handle multi-descriptor frame rx

[Paolo Abeni <pabeni@redhat.com>]

On 3/2/26 12:52 PM, Paolo Valerio wrote:
> Use the page pool allocator for the data buffers and enable skb
> recycling support, instead of relying on netdev_alloc_skb
> allocating the entire skb during the refill.
> 
> The patch also add support for receiving network frames that span
> multiple DMA descriptors in the Cadence MACB/GEM Ethernet driver.
> 
> The patch removes the requirement that limited frame reception to
> a single descriptor (RX_SOF && RX_EOF), also avoiding potential
> contiguous multi-page allocation for large frames.
> 
> Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
> ---
>  drivers/net/ethernet/cadence/Kconfig     |   1 +
>  drivers/net/ethernet/cadence/macb.h      |   5 +
>  drivers/net/ethernet/cadence/macb_main.c | 401 ++++++++++++++++-------
>  3 files changed, 283 insertions(+), 124 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 5b2a461dfd28..ae500f717433 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -25,6 +25,7 @@ config MACB
>  	depends on PTP_1588_CLOCK_OPTIONAL
>  	select PHYLINK
>  	select CRC32
> +	select PAGE_POOL
>  	help
>  	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
>  	  AT91 parts.  This driver also supports the Cadence GEM (Gigabit
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 3b184e9ac771..a78ad00f53b1 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -14,6 +14,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/phy/phy.h>
>  #include <linux/workqueue.h>
> +#include <net/page_pool/helpers.h>
>  
>  #define MACB_GREGS_NBR 16
>  #define MACB_GREGS_VERSION 2
> @@ -1266,6 +1267,8 @@ struct macb_queue {
>  	void			*rx_buffers;
>  	struct napi_struct	napi_rx;
>  	struct queue_stats stats;
> +	struct page_pool	*page_pool;
> +	struct sk_buff		*skb;
>  };
>  
>  struct ethtool_rx_fs_item {
> @@ -1289,6 +1292,8 @@ struct macb {
>  	struct macb_dma_desc	*rx_ring_tieoff;
>  	dma_addr_t		rx_ring_tieoff_dma;
>  	size_t			rx_buffer_size;
> +	size_t			rx_headroom;
> +	unsigned int		rx_ip_align;
>  
>  	unsigned int		rx_ring_size;
>  	unsigned int		tx_ring_size;
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index fbd4872901c2..621bca2e1844 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1263,14 +1263,54 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
>  	return packets;
>  }
>  
> -static int gem_rx_refill(struct macb_queue *queue)
> +static inline int gem_rx_data_len(struct macb *bp, struct macb_queue *queue,
> +				  u32 desc_ctrl, bool rx_sof, bool rx_eof)

Please no static inline in c files.

> +{
> +	int len;
> +
> +	if (unlikely(!rx_sof && !queue->skb)) {
> +		netdev_err(bp->dev,
> +			   "Received non-starting frame while expecting a starting one\n");

The existing code already had a few of them, but this
netdev_err()/netdev_warn() messages on per packet events could DoS the
host should they ever trigger.

Please switch to _once() or _ratelimited() variant when the
warning/error could be triggered by an incoming packet.

/P