Re: [PATCH net-next v9] net: stmmac: rework stmmac_rx to support XDP rx multi-buff

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 06 Nov 2026 11:54:19 +0100 Lorenzo Bianconi wrote:
> +static struct sk_buff *stmmac_build_skb(struct xdp_buff *xdp)
> +{
> +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> +	u32 metasize = xdp->data - xdp->data_meta;
> +	struct sk_buff *skb;
> +	u8 num_frags = 0;
> +
> +	if (unlikely(xdp_buff_has_frags(xdp)))
> +		num_frags = sinfo->nr_frags;
> +
> +	skb = napi_build_skb(xdp->data_hard_start, xdp->frame_sz);
> +	if (!skb)
> +		return NULL;
> +
> +	skb_mark_for_recycle(skb);
> +	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> +	skb_put(skb, xdp->data_end - xdp->data);
> +	if (metasize)
> +		skb_metadata_set(skb, metasize);
> +
> +	if (unlikely(xdp_buff_has_frags(xdp)))
> +		xdp_update_skb_frags_info(skb, num_frags, sinfo->xdp_frags_size,
> +					  sinfo->xdp_frags_truesize,
> +					  xdp_buff_get_skb_flags(xdp));
> +	return skb;
> +}

This is almost identical to xdp_build_skb_from_buff(), why not use the
common code?

> +static void
> +stmmac_xdp_put_buff(struct stmmac_rx_queue *rx_q, struct xdp_buff *xdp,
> +		    bool allow_direct)
> +{
> +	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
> +	int i;
> +
> +	if (likely(!xdp_buff_has_frags(xdp)))
> +		goto out;
> +
> +	for (i = 0; i < sinfo->nr_frags; i++)
> +		page_pool_put_full_page(rx_q->page_pool,
> +					skb_frag_page(&sinfo->frags[i]),
> +					allow_direct);
> +out:
> +	page_pool_put_full_page(rx_q->page_pool, virt_to_head_page(xdp->data),
> +				allow_direct);
> +}
> +
> +static int stmmac_xdp_shrink_tail(struct stmmac_rx_queue *rx_q,
> +				  struct xdp_buff *xdp, int offset)
> +{
> +	struct skb_shared_info *sinfo;
> +	int i;
> +
> +	if (unlikely(offset < 0 ||
> +		     offset > (int)xdp_get_buff_len(xdp) - ETH_HLEN))
> +		return -EINVAL;
> +
> +	if (likely(!xdp_buff_has_frags(xdp))) {
> +		xdp->data_end -= offset;
> +		return 0;
> +	}
> +
> +	sinfo = xdp_get_shared_info_from_buff(xdp);
> +	for (i = sinfo->nr_frags - 1; i >= 0 && offset > 0; i--) {
> +		skb_frag_t *frag = &sinfo->frags[i];
> +		int delta = min_t(int, offset, skb_frag_size(frag));
> +
> +		if (delta == skb_frag_size(frag)) {
> +			/* The whole frag is consumed by the strip: return it
> +			 * to the page pool right away. Its ring slot still
> +			 * carries the page DMA address until stmmac_rx_refill()
> +			 * re-arms it at the end of the NAPI poll. This is
> +			 * safe because HW cannot touch the slot until then.
> +			 */
> +			page_pool_put_full_page(rx_q->page_pool,
> +						skb_frag_page(frag), true);
> +			sinfo->nr_frags--;
> +			sinfo->xdp_frags_truesize -= xdp->frame_sz;
> +		} else {
> +			skb_frag_size_sub(frag, delta);
> +		}
> +
> +		sinfo->xdp_frags_size -= delta;
> +		offset -= delta;
> +	}
> +
> +	if (unlikely(!sinfo->nr_frags)) {
> +		xdp_buff_clear_frags_flag(xdp);
> +		xdp_buff_clear_frag_pfmemalloc(xdp);
> +		xdp->data_end -= offset;
> +	}
> +
> +	return 0;
> +}

These two OTOH could easily become new common helpers, they seem to do
nothing stmmac specific?