netdev - Re: Question about ip

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: <20170830115820.GC9993@breakpoint.cc>
Date:   Wed, 30 Aug 2017 13:58:20 +0200
From:   Florian Westphal <fw@...len.de>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Florian Westphal <fw@...len.de>,
        "liujian (CE)" <liujian56@...wei.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Wangkefeng (Kevin)" <wangkefeng.wang@...wei.com>,
        "weiyongjun (A)" <weiyongjun1@...wei.com>
Subject: Re: Question about ip_defrag

Jesper Dangaard Brouer <brouer@...hat.com> wrote:
> > I take 2) back.  Its wrong to do this, for large NR_CPU values it
> > would even overflow.
> 
> Alternatively solution 3:
> Why do we want to maintain a (4MBytes) memory limit, across all CPUs?
> Couldn't we just allow each CPU to have a memory limit?

Consider ipv4, ipv6, nf ipv6 defrag, 6lowpan, and 8k cpus... This will
render any limit useless.

> > > To me it looks like we/I have been using the wrong API for comparing
> > > against percpu_counters.  I guess we should have used __percpu_counter_compare().  
> > 
> > Are you sure?  For liujian use case (64 cores) it looks like we would
> > always fall through to percpu_counter_sum() so we eat spinlock_irqsave
> > cost for all compares.
> > 
> > Before we entertain this we should consider reducing frag_percpu_counter_batch
> > to a smaller value.
> 
> Yes, I agree, we really need to lower/reduce the frag_percpu_counter_batch.
> As you say, else the __percpu_counter_compare() call will be useless
> (around systems with >= 32 CPUs).
> 
> I think the bug is in frag_mem_limit().  It just reads the global
> counter (fbc->count), without considering other CPUs can have upto 130K
> that haven't been subtracted yet (due to 3M low limit, become dangerous
> at >=24 CPUs).  The  __percpu_counter_compare() does the right thing,
> and takes into account the number of (online) CPUs and batch size, to
> account for this.

Right, I think we should at very least use __percpu_counter_compare
before denying a new frag queue allocation request.

I'll create a patch.