netdev - Re: [bpf-next, 01/11] xdp: implement convert_to_xdp_frame for MEM_TYPE_ZERO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20180829180634.8330-1-macfij7@wp.pl>
Date:   Wed, 29 Aug 2018 20:06:34 +0200
From:   Maciek Fijalkowski <macfij7@...pl>
To:     bjorn.topel@...il.com, magnus.karlsson@...el.com,
        magnus.karlsson@...il.com, alexander.h.duyck@...el.com,
        alexander.duyck@...il.com, ast@...nel.org, brouer@...hat.com,
        daniel@...earbox.net, netdev@...r.kernel.org,
        jesse.brandeburg@...el.com, anjali.singhai@...el.com,
        peter.waskiewicz.jr@...el.com
Cc:     michael.lundkvist@...csson.com, willemdebruijn.kernel@...il.com,
        john.fastabend@...il.com, jakub.kicinski@...ronome.com,
        neerav.parikh@...el.com, mykyta.iziumtsev@...aro.org,
        francois.ozog@...aro.org, ilias.apalodimas@...aro.org,
        brian.brooks@...aro.org, u9012063@...il.com, pavel@...tnetmon.com,
        qi.z.zhang@...el.com
Subject: Re: [bpf-next, 01/11] xdp: implement convert_to_xdp_frame for MEM_TYPE_ZERO_COPY

From: Maciej Fijalkowski <macfij7@...pl>

> From: Björn Töpel <bjorn.topel@...el.com>
> 
> This commit adds proper MEM_TYPE_ZERO_COPY support for
> convert_to_xdp_frame. Converting a MEM_TYPE_ZERO_COPY xdp_buff to an
> xdp_frame is done by transforming the MEM_TYPE_ZERO_COPY buffer into a
> MEM_TYPE_PAGE_ORDER0 frame. This is costly, and in the future it might
> make sense to implement a more sophisticated thread-safe alloc/free
> scheme for MEM_TYPE_ZERO_COPY, so that no allocation and copy is
> required in the fast-path.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@...el.com>
> ---
>  include/net/xdp.h |  5 +++--
>  net/core/xdp.c    | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 76b95256c266..0d5c6fb4b2e2 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -91,6 +91,8 @@ static inline void xdp_scrub_frame(struct xdp_frame *frame)
>  	frame->dev_rx = NULL;
>  }
>  
> +struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp);
> +
>  /* Convert xdp_buff to xdp_frame */
>  static inline
>  struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
> @@ -99,9 +101,8 @@ struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
>  	int metasize;
>  	int headroom;
>  
> -	/* TODO: implement clone, copy, use "native" MEM_TYPE */
>  	if (xdp->rxq->mem.type == MEM_TYPE_ZERO_COPY)
> -		return NULL;
> +		return xdp_convert_zc_to_xdp_frame(xdp);
>  
>  	/* Assure headroom is available for storing info */
>  	headroom = xdp->data - xdp->data_hard_start;
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 89b6785cef2a..be6cb2f0e722 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -398,3 +398,42 @@ void xdp_attachment_setup(struct xdp_attachment_info *info,
>  	info->flags = bpf->flags;
>  }
>  EXPORT_SYMBOL_GPL(xdp_attachment_setup);
> +
> +struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp)
> +{
> +	unsigned int metasize, headroom, totsize;
> +	void *addr, *data_to_copy;
> +	struct xdp_frame *xdpf;
> +	struct page *page;
> +
> +	/* Clone into a MEM_TYPE_PAGE_ORDER0 xdp_frame. */
> +	metasize = xdp_data_meta_unsupported(xdp) ? 0 :
> +		   xdp->data - xdp->data_meta;
> +	headroom = xdp->data - xdp->data_hard_start;

You actually don't use the headroom calculated here;
xdpf->headroom is assigned to 0 - was this your intention?
If so, the local variable can be removed.

> +	totsize = xdp->data_end - xdp->data + metasize;
> +
> +	if (sizeof(*xdpf) + totsize > PAGE_SIZE)
> +		return NULL;
> +
> +	page = dev_alloc_page();
> +	if (!page)
> +		return NULL;
> +
> +	addr = page_to_virt(page);
> +	xdpf = addr;
> +	memset(xdpf, 0, sizeof(*xdpf));
> +
> +	addr += sizeof(*xdpf);
> +	data_to_copy = metasize ? xdp->data_meta : xdp->data;
> +	memcpy(addr, data_to_copy, totsize);
> +
> +	xdpf->data = addr + metasize;
> +	xdpf->len = totsize - metasize;
> +	xdpf->headroom = 0;
> +	xdpf->metasize = metasize;
> +	xdpf->mem.type = MEM_TYPE_PAGE_ORDER0;
> +
> +	xdp_return_buff(xdp);
> +	return xdpf;
> +}
> +EXPORT_SYMBOL_GPL(xdp_convert_zc_to_xdp_frame);