Re: [PATCH net-next v1 07/11] xsk: Deliver provider chunks without copying

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Tue, Nov 10, 2026 at 11:20:28AM +0100, Björn Töpel wrote:
> A provider socket still receives through the copy path, which takes
> FILL entries without the provider's ownership checks: a chunk that a
> page pool holds could be handed out twice. No driver opts in yet.
> 
> Put packets whose buffers all belong to the socket's provider on the
> RX ring as they are, and copy others into chunks the provider claims.
> Only the queue NAPI may produce to such a socket, so generic XDP and
> CPUMAP redirects to it are dropped.
> 
> A device may place payload at chunk offset 0, as fbnic does, so only
> the first buffer of a packet keeps the UMEM headroom. Such buffers may
> be sent back as they are, so only descriptors with XDP_TX_METADATA
> need room for TX metadata in front.
> 
> Signed-off-by: Björn Töpel <bjorn@kernel.org>
> ---
>  Documentation/networking/af_xdp.rst          |   6 +-
>  Documentation/networking/xsk-tx-metadata.rst |   3 +-
>  include/net/page_pool/memory_provider.h      |   2 +
>  net/core/page_pool.c                         |  34 +++-
>  net/xdp/xsk.c                                |  82 ++++++++-
>  net/xdp/xsk.h                                |   5 +
>  net/xdp/xsk_pp.c                             | 165 +++++++++++++++++++
>  net/xdp/xsk_queue.h                          |  18 +-
>  8 files changed, 304 insertions(+), 11 deletions(-)
> 

(...)

> +
>  static int xsk_pp_nl_fill(void *mp_priv, struct sk_buff *rsp,
>  			  struct netdev_rx_queue *rxq)
>  {
> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index 1bc42c8902f4..b9464b917a88 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h
> @@ -156,17 +156,24 @@ static inline bool xp_unused_options_set(u32 options)
>  	return options & ~(XDP_PKT_CONTD | XDP_TX_METADATA);
>  }
>  
> +static inline u32 xp_desc_meta_len(struct xsk_buff_pool *pool,
> +				   struct xdp_desc *desc)
> +{
> +	return desc->options & XDP_TX_METADATA ? pool->tx_metadata_len : 0;

How is it related to this patch? I know I trimmed, but I didn't see any
references above to tx_metadata_len, only some documentation bump.

Besides, the amount of branches introduced to existing hot path is
worrying. On next revision, could you provide numbers from xdpsock before
and after this set run on existing zc driver?

> +}
> +
>  static inline bool xp_aligned_validate_desc(struct xsk_buff_pool *pool,
>  					    struct xdp_desc *desc)
>  {
> +	u32 meta = xp_desc_meta_len(pool, desc);
>  	u64 len = desc->len;
>  	u64 addr, offset;
>  
>  	if (!len)
>  		return false;
>  
> -	/* Can overflow if desc->addr < pool->tx_metadata_len */
> -	if (check_sub_overflow(desc->addr, pool->tx_metadata_len, &addr))
> +	/* Can overflow if desc->addr < meta */
> +	if (check_sub_overflow(desc->addr, meta, &addr))
>  		return false;
>  
>  	offset = addr & (pool->chunk_size - 1);
> @@ -176,7 +183,7 @@ static inline bool xp_aligned_validate_desc(struct xsk_buff_pool *pool,
>  	 * (pool->chunk_size is ``u32``), @len is guaranteed
>  	 * to be <= ``U32_MAX``.
>  	 */
> -	if (offset + len + pool->tx_metadata_len > pool->chunk_size)
> +	if (offset + len + meta > pool->chunk_size)
>  		return false;
>  
>  	if (addr >= pool->addrs_cnt)
> @@ -191,6 +198,7 @@ static inline bool xp_aligned_validate_desc(struct xsk_buff_pool *pool,
>  static inline bool xp_unaligned_validate_desc(struct xsk_buff_pool *pool,
>  					      struct xdp_desc *desc)
>  {
> +	u32 meta = xp_desc_meta_len(pool, desc);
>  	u64 len = desc->len;
>  	u64 addr, end;
>  
> @@ -198,13 +206,13 @@ static inline bool xp_unaligned_validate_desc(struct xsk_buff_pool *pool,
>  		return false;
>  
>  	/* Can't overflow: @len is guaranteed to be <= ``U32_MAX`` */
> -	len += pool->tx_metadata_len;
> +	len += meta;
>  	if (len > pool->chunk_size)
>  		return false;
>  
>  	/* Can overflow if desc->addr is close to 0 */
>  	if (check_sub_overflow(xp_unaligned_add_offset_to_addr(desc->addr),
> -			       pool->tx_metadata_len, &addr))
> +			       meta, &addr))
>  		return false;
>  
>  	if (addr >= pool->addrs_cnt)
> -- 
> 2.56.0
>