netdev - Re: [PATCH net-next v9 1/2] xen networking: add basic XDP support for xen-netfront

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOJe8K3EHevDJ+3P59=F6AU7dVBvubR4-yUbeLGQ1WbFK5icZg@mail.gmail.com>
Date:   Tue, 12 May 2020 16:21:46 +0300
From:   Denis Kirjanov <kda@...ux-powerpc.org>
To:     Jürgen Groß <jgross@...e.com>
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 5/12/20, Jürgen Groß <jgross@...e.com> wrote:
> 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.

Right, it's not in the current version.

>
> 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).

I see, good to know.
I think that I can add a new flag like XEN_NETRXF_xdp_headroom in this case

>>
>> 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
>>>
>
>