netdev - Re: [RFC PATCH] net: frag limit checks need to use percpu_counter

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:   Fri, 1 Sep 2017 09:16:41 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     "liujian (CE)" <liujian56@...wei.com>
Cc:     Michal Kubecek <mkubecek@...e.cz>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Florian Westphal <fw@...len.de>, brouer@...hat.com
Subject: Re: [RFC PATCH] net: frag limit checks need to use
 percpu_counter_compare


On Fri, 1 Sep 2017 02:25:32 +0000 "liujian (CE)" <liujian56@...wei.com> wrote:

> > -----Original Message-----
> > From: Michal Kubecek [mailto:mkubecek@...e.cz]
> > Sent: Friday, September 01, 2017 12:24 AM
> > To: Jesper Dangaard Brouer
> > Cc: liujian (CE); netdev@...r.kernel.org; Florian Westphal
> > Subject: Re: [RFC PATCH] net: frag limit checks need to use
> > percpu_counter_compare
> > 
> > On Thu, Aug 31, 2017 at 12:20:19PM +0200, Jesper Dangaard Brouer wrote:  
> > > To: Liujian can you please test this patch?
> > >  I want to understand if using __percpu_counter_compare() solves  the
> > > problem correctness wise (even-though this will be slower  than using
> > > a simple atomic_t on your big system).  
> 
> I have test the patch, it can work. 

Thanks for confirming this.

> 1. make sure frag_mem_limit reach to thresh
>   ===>FRAG: inuse 0 memory 0 frag_mem_limit 5386864  
> 2. change NIC rx irq's affinity to a fixed CPU

If you pin the NIC RX queue to a single CPU, then the error issue
basically cannot happen.  Different CPU need to have a chance to "own"
part of the percpu_counter.  I guess default setup with irqbalance
could eventually screw the percpu_counter enough given enough CPUs, or
a network load with enough different L2-headers to high different RX
queues.

> 3. iperf -u -c 9.83.1.41 -l 10000 -i 1 -t 1000 -P 10 -b 20M
>   And check /proc/net/snmp, there are no ReasmFails.

My quick check command is:
 nstat > /dev/null && sleep 1 && nstat && grep FRAG /proc/net/sockstat

> And I think it is a better way that adding some counter sync points
> as you said.

I've discussed this offlist with Florian, while it is doable, we are
adding too much complexity for something that can be solved much
simpler with an atomic_t (as before my patch).  Thus, I'm now looking
at reverting my original change (commit 6d7b857d541e ("net: use
lib/percpu_counter API for fragmentation mem accounting")).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer