On Sat, 20 Jun 2020 00:57:17 +0200
Lorenzo Bianconi <lore...@kernel.org> wrote:
> From: David Ahern <dah...@digitalocean.com>
>
> Move the guts of xdp_convert_buff_to_frame to a new helper,
> xdp_update_frame_from_buff so it can be reused removing code duplication
>
> Suggested-by: Jesper Dangaard Brouer <bro...@redhat.com>
> Co-developed-by: Lorenzo Bianconi <lore...@kernel.org>
> Signed-off-by: Lorenzo Bianconi <lore...@kernel.org>
> Signed-off-by: David Ahern <dah...@digitalocean.com>
> ---
> include/net/xdp.h | 35 ++++++++++++++++++++++-------------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 609f819ed08b..ab1c503808a4 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -121,39 +121,48 @@ void xdp_convert_frame_to_buff(struct xdp_frame *frame,
> struct xdp_buff *xdp)
> xdp->frame_sz = frame->frame_sz;
> }
>
> -/* Convert xdp_buff to xdp_frame */
> static inline
> -struct xdp_frame *xdp_convert_buff_to_frame(struct xdp_buff *xdp)
> +int xdp_update_frame_from_buff(struct xdp_buff *xdp,
> + struct xdp_frame *xdp_frame)
> {
> - struct xdp_frame *xdp_frame;
> - int metasize;
> - int headroom;
> -
> - if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
> - return xdp_convert_zc_to_xdp_frame(xdp);
> + int metasize, headroom;
>
> /* Assure headroom is available for storing info */
> headroom = xdp->data - xdp->data_hard_start;
> metasize = xdp->data - xdp->data_meta;
> metasize = metasize > 0 ? metasize : 0;
> if (unlikely((headroom - metasize) < sizeof(*xdp_frame)))
> - return NULL;
> + return -ENOMEM;
IMHO I think ENOMEM is reserved for memory allocations failures.
I think ENOSPC will be more appropriate here (or EOVERFLOW).
>
> /* Catch if driver didn't reserve tailroom for skb_shared_info */
> if (unlikely(xdp->data_end > xdp_data_hard_end(xdp))) {
> XDP_WARN("Driver BUG: missing reserved tailroom");
> - return NULL;
> + return -ENOMEM;
Same here.
> }
>
> - /* Store info in top of packet */
> - xdp_frame = xdp->data_hard_start;
> -
> xdp_frame->data = xdp->data;
> xdp_frame->len = xdp->data_end - xdp->data;
> xdp_frame->headroom = headroom - sizeof(*xdp_frame);
> xdp_frame->metasize = metasize;
> xdp_frame->frame_sz = xdp->frame_sz;
>
> + return 0;
> +}
> +
> +/* Convert xdp_buff to xdp_frame */
> +static inline
> +struct xdp_frame *xdp_convert_buff_to_frame(struct xdp_buff *xdp)
> +{
> + struct xdp_frame *xdp_frame;
> +
> + if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
> + return xdp_convert_zc_to_xdp_frame(xdp);
> +
> + /* Store info in top of packet */
> + xdp_frame = xdp->data_hard_start;
> + if (unlikely(xdp_update_frame_from_buff(xdp, xdp_frame) < 0))
> + return NULL;
> +
> /* rxq only valid until napi_schedule ends, convert to xdp_mem_info */
> xdp_frame->mem = xdp->rxq->mem;
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
