Den tis 28 aug. 2018 kl 16:11 skrev Jesper Dangaard Brouer <bro...@redhat.com>:
>
> On Tue, 28 Aug 2018 14:44:25 +0200
> Björn Töpel <bjorn.to...@gmail.com> wrote:
>
> > From: Björn Töpel <bjorn.to...@intel.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.
>
> This is going to be slow. Especially the dev_alloc_page() call, which
> for small frames is likely going to be slower than the data copy.
> I guess this is a good first step, but I do hope we will circle back and
> optimize this later. (It would also be quite easy to use
> MEM_TYPE_PAGE_POOL instead to get page recycling in devmap redirect case).
>
Yes, slow. :-( Still, I think this is a good starting point, and then
introduce a page pool in later performance oriented series to make XDP
faster for the AF_XDP scenario.
But I'm definitely on your side here; This need to be addressed -- but
not now IMO.
And thanks for spending time on the series!
Björn
> I would have liked the MEM_TYPE_ZERO_COPY frame to travel one level
> deeper into the redirect-core code. Allowing devmap to send these
> frame without copy, and allow cpumap to do the dev_alloc_page() call
> (+copy) on the remote CPU.
>
>
> > Signed-off-by: Björn Töpel <bjorn.to...@intel.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;
> > + 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);
>
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
