On Tue, Oct 15, 2024 at 04:53:47PM +0200, Alexander Lobakin wrote:
> Same as with converting &xdp_buff to skb on Rx, the code which allocates
> a new skb and copies the XSk frame there is identical across the
> drivers, so make it generic. This includes copying all the frags if they
> are present in the original buff.
> System percpu Page Pools help here a lot: when available, allocate pages
> from there instead of the MM layer. This greatly improves XDP_PASS
> performance on XSk: instead of page_alloc() + page_free(), the net core
> recycles the same pages, so the only overhead left is memcpy()s.
> Note that the passed buff gets freed if the conversion is done w/o any
> error, assuming you don't need this buffer after you convert it to an
> skb.
AFAICT looks good.
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx>
I have to switch the context now so I'll finish reviewing the remainder of
this set on monday.
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
> ---
> include/net/xdp.h | 1 +
> net/core/xdp.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 139 insertions(+)
>
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 83e3f4648caa..69728b2d75d5 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -331,6 +331,7 @@ void xdp_warn(const char *msg, const char *func, const int line);
> #define XDP_WARN(msg) xdp_warn(msg, __func__, __LINE__)
>
> struct sk_buff *xdp_build_skb_from_buff(const struct xdp_buff *xdp);
> +struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp);
> struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp);
> struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
> struct sk_buff *skb,
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 371c26c203b2..116153b88d26 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -22,6 +22,8 @@
> #include <trace/events/xdp.h>
> #include <net/xdp_sock_drv.h>
>
> +#include "dev.h"
> +
> #define REG_STATE_NEW 0x0
> #define REG_STATE_REGISTERED 0x1
> #define REG_STATE_UNREGISTERED 0x2
> @@ -682,6 +684,142 @@ struct sk_buff *xdp_build_skb_from_buff(const struct xdp_buff *xdp)
> }
> EXPORT_SYMBOL_GPL(xdp_build_skb_from_buff);
>
> +/**
> + * xdp_copy_frags_from_zc - copy the frags from an XSk buff to an skb
> + * @skb: skb to copy frags to
> + * @xdp: XSk &xdp_buff from which the frags will be copied
> + * @pp: &page_pool backing page allocation, if available
> + *
> + * Copy all frags from an XSk &xdp_buff to an skb to pass it up the stack.
> + * Allocate a new page / page frag for each frag, copy it and attach to
> + * the skb.
> + *
> + * Return: true on success, false on page allocation fail.
> + */
> +static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
> + const struct xdp_buff *xdp,
> + struct page_pool *pp)
> +{
> + const struct skb_shared_info *xinfo;
> + struct skb_shared_info *sinfo;
> + u32 nr_frags, ts;
> +
> + xinfo = xdp_get_shared_info_from_buff(xdp);
> + nr_frags = xinfo->nr_frags;
> + sinfo = skb_shinfo(skb);
> +
> +#if IS_ENABLED(CONFIG_PAGE_POOL)
> + ts = 0;
> +#else
> + ts = xinfo->xdp_frags_truesize ? : nr_frags * xdp->frame_sz;
> +#endif
> +
> + for (u32 i = 0; i < nr_frags; i++) {
> + u32 len = skb_frag_size(&xinfo->frags[i]);
> + void *data;
> +#if IS_ENABLED(CONFIG_PAGE_POOL)
> + u32 truesize = len;
> +
> + data = page_pool_dev_alloc_va(pp, &truesize);
> + ts += truesize;
> +#else
> + data = napi_alloc_frag(len);
> +#endif
> + if (unlikely(!data))
> + return false;
> +
> + memcpy(data, skb_frag_address(&xinfo->frags[i]),
> + LARGEST_ALIGN(len));
> + __skb_fill_page_desc(skb, sinfo->nr_frags++,
> + virt_to_page(data),
> + offset_in_page(data), len);
> + }
> +
> + xdp_update_skb_shared_info(skb, nr_frags, xinfo->xdp_frags_size,
> + ts, false);
> +
> + return true;
> +}
> +
> +/**
> + * xdp_build_skb_from_zc - create an skb from an XSk &xdp_buff
> + * @xdp: source XSk buff
> + *
> + * Similar to xdp_build_skb_from_buff(), but for XSk frames. Allocate an skb
> + * head, new page for the head, copy the data and initialize the skb fields.
> + * If there are frags, allocate new pages for them and copy.
> + * If Page Pool is available, the function allocates memory from the system
> + * percpu pools to try recycling the pages, otherwise it uses the NAPI page
> + * frag caches.
> + * If new skb was built successfully, @xdp is returned to XSk pool's freelist.
> + * On error, it remains untouched and the caller must take care of this.
> + *
> + * Return: new &sk_buff on success, %NULL on error.
> + */
> +struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
> +{
> + const struct xdp_rxq_info *rxq = xdp->rxq;
> + u32 len = xdp->data_end - xdp->data_meta;
> + struct page_pool *pp;
> + struct sk_buff *skb;
> + int metalen;
> +#if IS_ENABLED(CONFIG_PAGE_POOL)
> + u32 truesize;
> + void *data;
> +
> + pp = this_cpu_read(system_page_pool);
> + truesize = xdp->frame_sz;
> +
> + data = page_pool_dev_alloc_va(pp, &truesize);
> + if (unlikely(!data))
> + return NULL;
> +
> + skb = napi_build_skb(data, truesize);
> + if (unlikely(!skb)) {
> + page_pool_free_va(pp, data, true);
> + return NULL;
> + }
> +
> + skb_mark_for_recycle(skb);
> + skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
> +#else /* !CONFIG_PAGE_POOL */
> + struct napi_struct *napi;
> +
> + pp = NULL;
> + napi = napi_by_id(rxq->napi_id);
> + if (likely(napi))
> + skb = napi_alloc_skb(napi, len);
> + else
> + skb = __netdev_alloc_skb_ip_align(rxq->dev, len,
> + GFP_ATOMIC | __GFP_NOWARN);
> + if (unlikely(!skb))
> + return NULL;
> +#endif /* !CONFIG_PAGE_POOL */
> +
> + memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));
> +
> + metalen = xdp->data - xdp->data_meta;
> + if (metalen > 0) {
> + skb_metadata_set(skb, metalen);
> + __skb_pull(skb, metalen);
> + }
> +
> + skb_record_rx_queue(skb, rxq->queue_index);
> +
> + if (unlikely(xdp_buff_has_frags(xdp)) &&
> + unlikely(!xdp_copy_frags_from_zc(skb, xdp, pp))) {
> + napi_consume_skb(skb, true);
> + return NULL;
> + }
> +
> + xsk_buff_free(xdp);
> +
> + skb->protocol = eth_type_trans(skb, rxq->dev);
> +
> + return skb;
> +}
> +EXPORT_SYMBOL_GPL(xdp_build_skb_from_zc);
> +
> struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame *xdpf,
> struct sk_buff *skb,
> struct net_device *dev)
> --
> 2.46.2
>
