Date:   Tue, 12 May 2020 14:41:45 +0200
From:   Jürgen Groß <jgross@...e.com>
To:     Denis Kirjanov <kda@...ux-powerpc.org>
Cc:     paul@....org, netdev@...r.kernel.org, brouer@...hat.com,
        wei.liu@...nel.org, ilias.apalodimas@...aro.org
Subject: Re: [PATCH net-next v9 1/2] xen networking: add basic XDP support for
 xen-netfront

On 12.05.20 14:27, Denis Kirjanov wrote:
> On 5/12/20, Jürgen Groß <jgross@...e.com> wrote:
>> On 11.05.20 19:27, Denis Kirjanov wrote:
>>> On 5/11/20, Jürgen Groß <jgross@...e.com> wrote:
>>>> On 11.05.20 12:22, 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.
>>>>>
>>>>> synchronization between frontend and backend parts is done
>>>>> by using xenbus state switching:
>>>>> Reconfiguring -> Reconfigured- > Connected
>>>>>
>>>>> UDP packets drop rate using xdp program is around 310 kpps
>>>>> using ./pktgen_sample04_many_flows.sh and 160 kpps without the patch.
>>>>
>>>> I'm still not seeing proper synchronization between frontend and
>>>> backend when an XDP program is activated.
>>>>
>>>> Consider the following:
>>>>
>>>> 1. XDP program is not active, so RX responses have no XDP headroom
>>>> 2. netback has pushed one (or more) RX responses to the ring page
>>>> 3. XDP program is being activated -> Reconfiguring
>>>> 4. netback acknowledges, will add XDP headroom for following RX
>>>>       responses
>>>> 5. netfront reads RX response (2.) without XDP headroom from ring page
>>>> 6. boom!
>>>
>>> One thing that could be easily done is to set the offset on  xen-netback
>>> side
>>> in  xenvif_rx_data_slot().  Are you okay with that?
>>
>> How does this help in above case?
>>
>> I think you haven't understood the problem I'm seeing.
>>
>> There can be many RX responses in the ring page which haven't been
>> consumed by the frontend yet. You are doing the switch to XDP via a
>> different communication channel (Xenstore), so you need some way to
>> synchronize both communication channels.
>>
>> Either you make sure you have read all RX responses before doing the
>> switch (this requires stopping netback to push out more RX responses),
>> or you need to have a flag in the RX responses indicating whether XDP
>> headroom is provided or not (requires an addition to the Xen netif
>> protocol).
> Hi Jürgen,
> 
> I see your point that we can have a shared ring with mixed RX responses offset.
> Since the offset field is set always  to 0 on netback side we can
> adjust it and thus mark that a response has the offset adjusted or
> it's not (if the offset filed is set to 0).

For one I don't see your code in netfront to test this condition.

And I don't think this is a guaranteed interface. Have you checked all
netback versions in older kernels, in qemu, and in BSD?

BTW, I'm pretty sure the old xen-linux netback sometimes used an offset
not being 0. And yes, those kernels are still active in some cases (e.g.
SLES11-SP4 is still supported for customers having a long time service
agreement and this version is based on xen-linux).

> 
> In this case we have to run an xdp program on netfront side only for a
> response with offset set to xdp headroom.
> 
> I don't see a race in the scenario above.

I do.


Juergen

> 
> Or I'm completely wrong and this can not happen due to the
>> way XDP programs work, but you didn't provide any clear statement this
>> being the case.
>>
>>
>> Juergen
>>

Powered by blists - more mailing lists