Re: [PATCH net-next v2] xen-netfront: add basic XDP support

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Thu, 5 Mar 2020 15:31:14 +0200
Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:

> On Thu, Mar 05, 2020 at 04:23:31PM +0300, Denis Kirjanov wrote:
> > On 3/5/20, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:  
> > > Hi Denis,
> > >
> > > There's a bunch of things still missing from my remarks on V1.
> > > XDP is not supposed to allocate and free pages constantly as that's one of
> > > the things that's making it fast.  
> > 
> > Hi Ilias,
> > 
> > I've removed the copying to an allocated page so there is no page
> > allocation/free logic added.
> >   
> 
> Yea that has been removed. I am not familiar with the driver though, so i'll
> give you an example. 
> Let's say the BPF program says the packet must be dropped. What will happen to
> the page with the packet payload?
> Ideally on XDP we want that page recycled back into the device descriptors, so
> the driver won't have to allocate and map a fresh page.

I agree.  The main point with XDP is that we can do something
faster-than the normal network stack.  Especially in case of XDP_DROP,
we do driver specific recycling tricks, to avoid any allocations and
reinsert the RX-frame in RX-ring, and avoid overhead of SKB allocations.

Looking closer at your patch it seem you run XDP after the SKB alloc?!?

> >   
> > >
> > > You are also missing proper support for XDP_REDIRECT, ndo_xdp_xmit. We
> > > usually require the whole functionality to merge the driver.  

I agree, we have unfortunately seen drivers not getting completed if we
don't require full-XDP feature set.

> > 
> > I wanted to minimize changes and send follow-up patches
> >   
> 
> Adding XDP_REDIRECT is pretty trivial and the ndo_xdp_xmit should be very
> similar to XDP_TX. So assuming you'll fix XDP_TX adding the .ndo one will be
> relatively small amount of code.

You can have a patchset with more patches, if you prefer splitting this
up in multiple patches.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer