netdev - Re: [PATCH] net/udp: do not touch skb->peeked unless really needed

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: <1481034881.18162.577.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Tue, 06 Dec 2016 06:34:41 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH] net/udp: do not touch skb->peeked unless really needed

On Tue, 2016-12-06 at 10:53 +0100, Paolo Abeni wrote:
> Hi Eric,

> I don't understand why we can avoid setting skb->peek if len > 0. I
> think that will change the kernel behavior if:
> - peek with offset is set
> - 3 skbs with len > 0 are enqueued
> - the u/s peek (with offset) the second one
> - the u/s disable peeking with offset and peeks 2 more skbs.
> 
> With the current code in the last step the u/s is going to peek the 1#
> and the 3# skbs, after this patch will peek the 1# and the 2#. Am I
> missing something ? Probably the new behavior is more correct, but still
> is a change. 
> 
> I gave this a run in my test bed on top of your udp-related patches I
> see additional ~3 improvement in the udp flood scenario, and a bit more
> in the un-contended scenario.
> 
> Thank you,

MSG_PEEK always grab the first skb in queue, regardless of its
skb->peeked status.

Unless an offset (given in bytes) is given. Then we skip X bytes in the
queue, again regardless of skb->peeked bit.

skb->peeked is _needed_ to avoid peeking the same 0-byte skb over and
over, since user land could not skip it, as the offset to skip skbs is
computed by sum ( lengthes). An infinite loop of recvmsg() would happen,
stuck on the same skb.

For regular non 0-bytes payload, we can skip over them without even
looking at skb->peeked.

Initially, skb->peeked was only a (bad) way to make sure UDP would
increment its stats only for non peeked messages. We can implement that
using at MSG_PEEK flag.