Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

From: Denis Kirjanov <kda@linux-powerpc.org>
To: "Jürgen Groß" <jgross@suse.com>
Cc: netdev@vger.kernel.org, ilias.apalodimas@linaro.org,
	wei.liu@kernel.org, paul@xen.org
Subject: Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront
Date: Wed, 18 Mar 2020 15:50:19 +0300	[thread overview]
Message-ID: <CAOJe8K3gDJrdKz9zVZNj=N76GygMnPbCKM0-kVfoV53fASAefg@mail.gmail.com> (raw)
In-Reply-To: <f75365c7-a3ca-cf12-b2fc-e48652071795@suse.com>

On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
> On 16.03.20 14:09, Denis Kirjanov wrote:
>> The patch adds a basic XDP processing to xen-netfront driver.
>>
>> We ran an XDP program for an RX response received from netback
>> driver. Also we request xen-netback to adjust data offset for
>> bpf_xdp_adjust_head() header space for custom headers.
>
> This is in no way a "verbose patch descriprion".
>
> I'm missing:
>
> - Why are you doing this. "Add XDP support" is not enough, for such
>    a change I'd like to see some performance numbers to get an idea
>    of the improvement to expect, or which additional functionality
>    for the user is available.
Ok, I'll try to measure  some numbers.

>
> - A short description for me as a Xen maintainer with only basic
>    networking know-how, what XDP programs are about (a link to some
>    more detailed doc is enough, of course) and how the interface
>    is working (especially for switching between XDP mode and normal
>    SKB processing).

You can search for the "A practical introduction to XDP" tutorial.
Actually there is a lot of information available regarding XDP, you
can easily find it.

>
> - A proper description of the netfront/netback communication when
>    enabling or disabling XDP mode (who is doing what, is silencing
>    of the virtual adapter required, ...).
Currently we need only a header offset from netback driver so that we can put
custom encapsulation header if required and that's done using xen bus
state switching,
so that:
- netback tells that it can adjust the header offset
- netfront part reads it
>
> - Reasoning why the suggested changes of frontend and backend state
>    are no problem for special cases like hot-remove of an interface or
>    live migration or suspend of the guest.

I've put the code to talk_to_netback which is called "when first
setting up, and when resuming"
If you see a problem with that please share.

>
> Finally I'd like to ask you to split up the patch into a netfront and
> a netback one.

Ok, will do.

Thanks!
>
>
> Juergen
>