From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= Subject: Re: [PATCH bpf-next 01/11] xdp: implement convert_to_xdp_frame for MEM_TYPE_ZERO_COPY Date: Tue, 28 Aug 2018 19:42:57 +0200 Message-ID: References: <20180828124435.30578-1-bjorn.topel@gmail.com> <20180828124435.30578-2-bjorn.topel@gmail.com> <20180828161102.45a00204@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: "Karlsson, Magnus" , Magnus Karlsson , "Duyck, Alexander H" , Alexander Duyck , ast@kernel.org, Daniel Borkmann , Netdev , "Brandeburg, Jesse" , "Singhai, Anjali" , peter.waskiewicz.jr@intel.com, =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , michael.lundkvist@ericsson.com, Willem de Bruijn , John Fastabend , Jakub Kicinski , neerav.parikh@intel.com, MykytaI Iziumtsev , Francois Ozog , Ilias To: Jesper Dangaard Brouer Return-path: Received: from mail-qk0-f196.google.com ([209.85.220.196]:46490 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727020AbeH1Vfw (ORCPT ); Tue, 28 Aug 2018 17:35:52 -0400 Received: by mail-qk0-f196.google.com with SMTP id j7-v6so1542785qkd.13 for ; Tue, 28 Aug 2018 10:43:08 -0700 (PDT) In-Reply-To: <20180828161102.45a00204@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: Den tis 28 aug. 2018 kl 16:11 skrev Jesper Dangaard Brouer : > > On Tue, 28 Aug 2018 14:44:25 +0200 > Bj=C3=B6rn T=C3=B6pel wrote: > > > From: Bj=C3=B6rn T=C3=B6pel > > > > 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=C3=B6rn > 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=C3=B6rn T=C3=B6pel > > --- > > 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 =3D 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_bu= ff *xdp) > > int metasize; > > int headroom; > > > > - /* TODO: implement clone, copy, use "native" MEM_TYPE */ > > if (xdp->rxq->mem.type =3D=3D MEM_TYPE_ZERO_COPY) > > - return NULL; > > + return xdp_convert_zc_to_xdp_frame(xdp); > > > > /* Assure headroom is available for storing info */ > > headroom =3D 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_in= fo *info, > > info->flags =3D 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 =3D xdp_data_meta_unsupported(xdp) ? 0 : > > + xdp->data - xdp->data_meta; > > + headroom =3D xdp->data - xdp->data_hard_start; > > + totsize =3D xdp->data_end - xdp->data + metasize; > > + > > + if (sizeof(*xdpf) + totsize > PAGE_SIZE) > > + return NULL; > > + > > + page =3D dev_alloc_page(); > > + if (!page) > > + return NULL; > > + > > + addr =3D page_to_virt(page); > > + xdpf =3D addr; > > + memset(xdpf, 0, sizeof(*xdpf)); > > + > > + addr +=3D sizeof(*xdpf); > > + data_to_copy =3D metasize ? xdp->data_meta : xdp->data; > > + memcpy(addr, data_to_copy, totsize); > > + > > + xdpf->data =3D addr + metasize; > > + xdpf->len =3D totsize - metasize; > > + xdpf->headroom =3D 0; > > + xdpf->metasize =3D metasize; > > + xdpf->mem.type =3D 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