netdev - Re: [RFC PATCH] ip: re-introduce fragments cache worker

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: <0419d5f8f551e481603b34f162299cc9961694fc.camel@redhat.com>
Date:   Fri, 20 Jul 2018 19:31:59 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Florian Westphal <fw@...len.de>, neilb@...e.com
Subject: Re: [RFC PATCH] ip: re-introduce fragments cache worker

On Fri, 2018-07-20 at 08:58 -0700, Eric Dumazet wrote:
> On Fri, Jul 20, 2018 at 7:48 AM Paolo Abeni <pabeni@...hat.com> wrote:
> > 
> > Hi,
> > 
> > On Mon, 2018-07-09 at 05:50 -0700, Eric Dumazet wrote:
> > > On 07/09/2018 04:39 AM, Eric Dumazet wrote:
> > > 
> > > > Alternatively, you could try to patch fq_codel to drop all frags of one UDP datagram
> > > > instead of few of them.
> > > 
> > > A first step would be to make sure fq_codel_hash() (using skb_get_hash(skb)) selects
> > > the same bucket for all frags of a datagram :/
> > 
> > I gave the above a shot and I have some non upstream ready but somewhat
> > working code. Anyway it has some issues I'm unable to solve:
> > * it's very invasive for fq_codel, because I need to parse each packet
> > looking for the fragment id
> > * the parsing overhead can't be easily avoided for non fragments
> 
> Have you tried using ip_defrag(net, skb,  IP_DEFRAG_QDISC) from fq_codel ?
> (adding a new value in ip_defrag_users enum)
> 
> if (skb->protocol == htons(ETH_P_IP) {
>       if (ip_is_fragment(ip_hdr(skb))) {
>            if ((ip_defrag(net, skb, IP_DEFRAG_QDISC))
>                 return 0;
> ...

Thank you for the feedback. I must admit this quite in the opposite
direction of what I have attempted so far. I'll try that.
Thanks.
Still for ipv6 it will require a litte more work inside fq_codel.

> > I tried also something hopefully along the same lines of your other
> > suggestion (drop eariler the fragment queues when above low threshold):
> > when allocating a new frag queue and the ipfrag mem is above the low
> > th, another frag queue is selected in a pseudorandom way and dropped.
> 
> The problem with any strategy like that, is that forthcoming fragments
> for this frag queue
> will create another frag queue, that will never have a chance to complete.
> 
> Some workloads might benefit, others might not.

Yes, of course: is an heuristic, but is cheap code-wise, and it can be
disabled setting low th >= high th, so that the kernel will behave
exactly as it does now, and the kind of workloads we could cope with
will increase without adding new knobs.

Cheers,

Paolo