netdev - Re: [PATCH net-next v4] 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]

Date:   Mon, 30 Mar 2020 14:55:02 +0200
From:   Jürgen Groß <jgross@...e.com>
To:     Denis Kirjanov <kda@...ux-powerpc.org>
Cc:     netdev@...r.kernel.org, ilias.apalodimas@...aro.org,
        wei.liu@...nel.org, paul@....org
Subject: Re: [PATCH net-next v4] xen networking: add basic XDP support for
 xen-netfront

On 30.03.20 14:16, Denis Kirjanov wrote:
> On 3/23/20, Jürgen Groß <jgross@...e.com> wrote:
>> On 23.03.20 11:49, Denis Kirjanov wrote:
>>> On 3/23/20, Jürgen Groß <jgross@...e.com> wrote:
>>>> On 23.03.20 11:15, Denis Kirjanov wrote:
>>>>> On 3/18/20, Jürgen Groß <jgross@...e.com> wrote:
>>>>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>>>>> On 3/18/20, Jürgen Groß <jgross@...e.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
>>>>>>
>>>>>> Yes, but how is this synchronized with currently running network load?
>>>>>> Assume you are starting without XDP being active and then you are
>>>>>> activating it. How is the synchronization done from which request on
>>>>>> the XDP headroom is available?
>>>>>
>>>>> Hi Jurgen,
>>>>>
>>>>> basically XDP is activated when you've assigned an xdp program to the
>>>>> networking device.
>>>>> Assigning an xdp program means that we have to adjust a pointer which
>>>>> is RCU protected.
>>>>
>>>> This doesn't answer my question.
>>>>
>>>> You have basically two communication channels: the state of the frontend
>>>> and backend for activation/deactivation of XDP, and the ring pages with
>>>> the rx and tx requests and responses. How is the synchronization between
>>>> those two channels done? So how does the other side know which of the
>>>> packets in flight will then have XDP on or off?
>>>
>>> Right,
>>> that's done in xen-netback using xenbus state:
>>> - in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback to
>>> adjust offset for an RX response.
>>> -xen-netback reads the value from xenstore and adjusts the offset for
>>> xen-netback
>>> in xenvif_rx_data_slot() using vif->xdp_enabled flag.
>>
>> And before that all in-flight requests in the ring pages are being
>> processed and no new requests are guaranteed to be enqueued?
> 
> Actually I don't see the need to sync these requests since that all we
> have to do is to copy
> data with specified offset:
> with xdp->enabled=1: copy with the offset XDP_PACKET_HEADROOM
> with xdd->enabled=0: copy without the offset

Isn't that racy?

In xennet_xdp_set() you set queue->xdp_prog and then you change the
state to Reconfiguring. From the time queue->xdp_prog is set you'll
do the Xdp processing in xennet_get_responses(), even if the response
you are working on doesn't have the headroom you need, as the backend
didn't create it with headroom (it needs some time until it has seen
the new state and could react on it by sending _new_ responses with
headroom).

Or am I missing something about Xdp programs?


Juergen