On Fri, 12 Jun 2020 16:54:08 +0800
Hangbin Liu <liuhangbin@xxxxxxxxx> wrote:
> On Wed, Jun 10, 2020 at 12:18:59PM +0200, Jesper Dangaard Brouer wrote:
> > On Tue, 26 May 2020 22:05:38 +0800
> > Hangbin Liu <liuhangbin@xxxxxxxxx> wrote:
> >
> > > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > > index 90f44f382115..acdc63833b1f 100644
> > > --- a/net/core/xdp.c
> > > +++ b/net/core/xdp.c
> > > @@ -475,3 +475,29 @@ 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 xdp_frame *xdpf_clone(struct xdp_frame *xdpf)
> > > +{
> > > + unsigned int headroom, totalsize;
> > > + struct xdp_frame *nxdpf;
> > > + struct page *page;
> > > + void *addr;
> > > +
> > > + headroom = xdpf->headroom + sizeof(*xdpf);
> > > + totalsize = headroom + xdpf->len;
> > > +
> > > + if (unlikely(totalsize > PAGE_SIZE))
> > > + return NULL;
> > > + page = dev_alloc_page();
> > > + if (!page)
> > > + return NULL;
> > > + addr = page_to_virt(page);
> > > +
> > > + memcpy(addr, xdpf, totalsize);
> >
> > I don't think this will work. You are assuming that the memory model
> > (xdp_mem_info) is the same.
> >
> > You happened to use i40, that have MEM_TYPE_PAGE_SHARED, and you should
> > have changed this to MEM_TYPE_PAGE_ORDER0, but it doesn't crash as they
> > are compatible. If you were using mlx5, I suspect that this would
> > result in memory leaking.
>
> Is there anything else I should do except add the following line?
> nxdpf->mem.type = MEM_TYPE_PAGE_ORDER0;
You do realize that you also have copied over the mem.id, right?
And as I wrote below you also need to update frame_sz.
> >
> > You also need to update xdpf->frame_sz, as you also cannot assume it is
> > the same.
>
> Won't the memcpy() copy xdpf->frame_sz to nxdpf?
You obviously cannot use the frame_sz from the existing frame, as you
just allocated a new page for the new xdp_frame, that have another size
(here PAGE_SIZE).
> And I didn't see xdpf->frame_sz is set in xdp_convert_zc_to_xdp_frame(),
> do we need a fix?
Good catch, that sounds like a bug, that should be fixed.
Will you send a fix?
> > > +
> > > + nxdpf = addr;
> > > + nxdpf->data = addr + headroom;
> > > +
> > > + return nxdpf;
> > > +}
> > > +EXPORT_SYMBOL_GPL(xdpf_clone);
> >
> >
> > struct xdp_frame {
> > void *data;
> > u16 len;
> > u16 headroom;
> > u32 metasize:8;
> > u32 frame_sz:24;
> > /* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,
> > * while mem info is valid on remote CPU.
> > */
> > struct xdp_mem_info mem;
> > struct net_device *dev_rx; /* used by cpumap */
> > };
> >
>
struct xdp_mem_info {
u32 type; /* 0 4 */
u32 id; /* 4 4 */
/* size: 8, cachelines: 1, members: 2 */
/* last cacheline: 8 bytes */
};
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
