netdev - Re: [net-next PATCH v2 2/2] e1000: bundle xdp xmit routines

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: <CALx6S36dH87e6TPxEBGsy0F-vD0fZzKyP8tbuuaAfLNSF1DGpw@mail.gmail.com>
Date:   Fri, 9 Sep 2016 18:04:25 -0700
From:   Tom Herbert <tom@...bertland.com>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     Brenden Blanco <bblanco@...mgrid.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Cong Wang <xiyou.wangcong@...il.com>,
        intel-wired-lan <intel-wired-lan@...ts.osuosl.org>,
        u9012063@...il.com,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [net-next PATCH v2 2/2] e1000: bundle xdp xmit routines

On Fri, Sep 9, 2016 at 5:01 PM, John Fastabend <john.fastabend@...il.com> wrote:
> On 16-09-09 04:44 PM, Tom Herbert wrote:
>> On Fri, Sep 9, 2016 at 2:29 PM, John Fastabend <john.fastabend@...il.com> wrote:
>>> e1000 supports a single TX queue so it is being shared with the stack
>>> when XDP runs XDP_TX action. This requires taking the xmit lock to
>>> ensure we don't corrupt the tx ring. To avoid taking and dropping the
>>> lock per packet this patch adds a bundling implementation to submit
>>> a bundle of packets to the xmit routine.
>>>
>>> I tested this patch running e1000 in a VM using KVM over a tap
>>> device using pktgen to generate traffic along with 'ping -f -l 100'.
>>>
>> Hi John,
>>
>> How does this interact with BQL on e1000?
>>
>> Tom
>>
>
> Let me check if I have the API correct. When we enqueue a packet to
> be sent we must issue a netdev_sent_queue() call and then on actual
> transmission issue a netdev_completed_queue().
>
> The patch attached here missed a few things though.
>
> But it looks like I just need to call netdev_sent_queue() from the
> e1000_xmit_raw_frame() routine and then let the tx completion logic
> kick in which will call netdev_completed_queue() correctly.
>
> I'll need to add a check for the queue state as well. So if I do these
> three things,
>
>         check __QUEUE_STATE_XOFF before sending
>         netdev_sent_queue() -> on XDP_TX
>         netdev_completed_queue()
>
> It should work agree? Now should we do this even when XDP owns the
> queue? Or is this purely an issue with sharing the queue between
> XDP and stack.
>
But what is the action for XDP_TX if the queue is stopped? There is no
qdisc to back pressure in the XDP path. Would we just start dropping
packets then?

Tom

> .John
>