netdev - Re: [RFC PATCH] netfilter: remove the duplicate tables

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: <1290166180.3034.119.camel@edumazet-laptop>
Date:	Fri, 19 Nov 2010 12:29:40 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Jan Engelhardt <jengelh@...ozas.de>
Cc:	Changli Gao <xiaosuo@...il.com>, Patrick McHardy <kaber@...sh.net>,
	"David S. Miller" <davem@...emloft.net>,
	netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
	Stephen Hemminger <shemminger@...tta.com>
Subject: Re: [RFC PATCH] netfilter: remove the duplicate tables

Le vendredi 19 novembre 2010 à 12:15 +0100, Jan Engelhardt a écrit :

> Was it? Quoting Patrick from 24h prior to this post:
> 
> |so patches to get rid of the table duplication are highly welcome.
> 
> >still you post a patch that needs our review and time ? This is crazy.
> 
> You do not need to do it, but I will happily look at this.
> Of course my observations are the same as yours:
> 
> >Your way of allocating a percpu counter for each counter is a pure TLB
> >and cache line blower (up to two cache lines per counter), not counting
> >the time needed to load a new table with 10.000 entries. Some people
> >still use scripts with hundred of calls to iptables.
> 
> The two are statistically independent though. Even for a loaded 
> ruleset, the TLB/DC miss accumulation will be desastrous - as I've found 
> with linked-list rules/small allocs.
> 
> >Allocating one contiguous percpu var for all counters is a must.
> >
> >Problem is : percpu alloc doesnt allow big allocations.
> >
> >#define PCPU_MIN_UNIT_SIZE      PFN_ALIGN(32 << 10)
> >
> >So max allocation is 32 Kbytes, thats 2048 'xt_counters' only.
> >-> cannot really use pcpu-alloc, but a kmalloc_node() or vmalloc_node()
> >per cpu
> 
> .. as is already done for jumpstack ;-)

IMHO, the real problem is not the table duplication. We know that adding
a level of indirection is going to hurt a lot because of cache misses.

Its the atomic op (spinlock) done for every packet, entering every
filter, with the conditional branch we do because of possible recursion.

per cpu variable, and spinlock... its really expensive.

Stephen tried an RCU conversion some time ago, that aborted because of
RCU drawbacks (too much memory was possibly waiting to be freed after a
grace period). Maybe RCU infrastructure is now ready to try again.

We should do what we did for u64 stats counters in network stack, using
the u64_stats_sync.h infrastructure. No more synchro between the threads
running through rules, and one gathering counters. Better latencies in
particular.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html