netdev - Re: [net-next PATCH v2 3/5] virtio

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:   Tue, 22 Nov 2016 00:27:03 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     daniel@...earbox.net, eric.dumazet@...il.com, kubakici@...pl,
        shm@...ulusnetworks.com, davem@...emloft.net,
        alexei.starovoitov@...il.com, netdev@...r.kernel.org,
        bblanco@...mgrid.com, john.r.fastabend@...el.com,
        brouer@...hat.com, tgraf@...g.ch
Subject: Re: [net-next PATCH v2 3/5] virtio_net: Add XDP support

On 16-11-21 03:20 PM, Michael S. Tsirkin wrote:
> On Sat, Nov 19, 2016 at 06:50:33PM -0800, John Fastabend wrote:
>> From: Shrijeet Mukherjee <shrijeet@...il.com>
>>
>> This adds XDP support to virtio_net. Some requirements must be
>> met for XDP to be enabled depending on the mode. First it will
>> only be supported with LRO disabled so that data is not pushed
>> across multiple buffers. The MTU must be less than a page size
>> to avoid having to handle XDP across multiple pages.
>>
>> If mergeable receive is enabled this first series only supports
>> the case where header and data are in the same buf which we can
>> check when a packet is received by looking at num_buf. If the
>> num_buf is greater than 1 and a XDP program is loaded the packet
>> is dropped and a warning is thrown. When any_header_sg is set this
>> does not happen and both header and data is put in a single buffer
>> as expected so we check this when XDP programs are loaded. Note I
>> have only tested this with Linux vhost backend.
>>
>> If big packets mode is enabled and MTU/LRO conditions above are
>> met then XDP is allowed.
>>
>> A follow on patch can be generated to solve the mergeable receive
>> case with num_bufs equal to 2. Buffers greater than two may not
>> be handled has easily.
> 
> 
> I would very much prefer support for other layouts without drops
> before merging this.
> header by itself can certainly be handled by skipping it.
> People wanted to use that e.g. for zero copy.

OK fair enough I'll do this now rather than push it out.

> 
> Anything else can be handled by copying the packet.

This though I'm not so sure about. The copy is going to be slow and
I wonder if someone could craft a packet to cause this if it could
be used to slow down a system.

Also I can't see what would cause this to happen. With mergeable
buffers and LRO off the num_bufs is either 1 or 2 depending on where
the header is. Otherwise with LRO off it should be in a single page.
At least this is the Linux vhost implementation, I guess other
implementation might meet spec but use num_buf > 2 or multiple pages
even in the non LRO case.

I tend to think dropping the packet out right is better than copying
it around. At very least if we do this we need to put in warnings so
users can see something is mis-configured.

.John