Re: [PATCH v5 bpf-next 11/14] bpf: cpumap: introduce xdp multi-buff support

[Shay Agroskin <shayagr@amazon.com>]

Lorenzo Bianconi <lorenzo@kernel.org> writes:

> Introduce __xdp_build_skb_from_frame and 
> xdp_build_skb_from_frame
> utility routines to build the skb from xdp_frame.
> Add xdp multi-buff support to cpumap
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  include/net/xdp.h   |  5 ++++
>  kernel/bpf/cpumap.c | 45 +---------------------------
>  net/core/xdp.c      | 73 
>  +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 79 insertions(+), 44 deletions(-)
>
[...]
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 6c8e743ad03a..55f3e9c69427 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -597,3 +597,76 @@ void xdp_warn(const char *msg, const char 
> *func, const int line)
>  	WARN(1, "XDP_WARN: %s(line:%d): %s\n", func, line, msg);
>  };
>  EXPORT_SYMBOL_GPL(xdp_warn);
> +
> +struct sk_buff *__xdp_build_skb_from_frame(struct xdp_frame 
> *xdpf,
> +					   struct sk_buff *skb,
> +					   struct net_device *dev)
> +{
> +	unsigned int headroom = sizeof(*xdpf) + xdpf->headroom;
> +	void *hard_start = xdpf->data - headroom;
> +	skb_frag_t frag_list[MAX_SKB_FRAGS];
> +	struct xdp_shared_info *xdp_sinfo;
> +	int i, num_frags = 0;
> +
> +	xdp_sinfo = xdp_get_shared_info_from_frame(xdpf);
> +	if (unlikely(xdpf->mb)) {
> +		num_frags = xdp_sinfo->nr_frags;
> +		memcpy(frag_list, xdp_sinfo->frags,
> +		       sizeof(skb_frag_t) * num_frags);
> +	}

nit, can you please move the xdp_sinfo assignment inside this 'if' 
? This would help to emphasize that regarding xdp_frame tailroom 
as xdp_shared_info struct (rather than skb_shared_info) is correct 
only when the mb bit is set

thanks,
Shay

> +
> +	skb = build_skb_around(skb, hard_start, xdpf->frame_sz);
> +	if (unlikely(!skb))
> +		return NULL;
[...]