[PATCH] iptables: xt_hashlimit fix
From: Eric Dumazet <hidden>
Date: 2009-02-20 18:10:54
Also in:
netfilter-devel
Subsystem:
netfilter, networking [general], the rest · Maintainers:
Pablo Neira Ayuso, Florian Westphal, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
Eric Dumazet a écrit :
Stephen Hemminger a écrit :quoted
The reader/writer lock in ip_tables is acquired in the critical path of processing packets and is one of the reasons just loading iptables can cause a 20% performance loss. The rwlock serves two functions: 1) it prevents changes to table state (xt_replace) while table is in use. This is now handled by doing rcu on the xt_table. When table is replaced, the new table(s) are put in and the old one table(s) are freed after RCU period. 2) it provides synchronization when accesing the counter values. This is now handled by swapping in new table_info entries for each cpu then summing the old values, and putting the result back onto one cpu. On a busy system it may cause sampling to occur at different times on each cpu, but no packet/byte counts are lost in the process. Signed-off-by: Stephen Hemminger <redacted>Acked-by: Eric Dumazet <redacted> Sucessfully tested on my dual quad core machine too, but iptables only (no ipv6 here) BTW, my new "tbench 8" result is 2450 MB/s, (it was 2150 MB/s not so long ago) Thanks Stephen, thats very cool stuff, yet another rwlock out of kernel :)
Damned this broke xt_hashlimit, version=0
Look file "net/netfilter/xt_hashlimit.c" line 706
/* Ugly hack: For SMP, we only want to use one set */
r->u.master = r;
File "include/linux/netfilter/xt_hashlimit.h"
struct xt_hashlimit_info {
char name [IFNAMSIZ]; /* name */
struct hashlimit_cfg cfg;
/* Used internally by the kernel */
struct xt_hashlimit_htable *hinfo;
union {
void *ptr;
struct xt_hashlimit_info *master;
} u;
};
So, it appears some modules are using pointers to themselves, what a hack :(
We probably need an audit of other modules.
(net/netfilter/xt_statistic.c, net/netfilter/xt_quota.c,
net/netfilter/xt_limit.c ...)
Unfortunatly I wont have time to do this in following days, any volunteer ?
Thank you
[PATCH] netfilter: xt_hashlimit fix
Commit 784544739a25c30637397ace5489eeb6e15d7d49
(netfilter: iptables: lock free counters) broke xt_hashlimit netfilter module :
This module was storing a pointer inside its xt_hashlimit_info, and this pointer
is not relocated when we temporarly switch tables (iptables -L).
This hack is not not needed at all (probably a leftover from
ancient time), as each cpu should and can access to its own copy.
Signed-off-by: Eric Dumazet <redacted>
---
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 2482055..a5b5369 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c@@ -565,8 +565,7 @@ hashlimit_init_dst(const struct xt_hashlimit_htable *hinfo, static bool hashlimit_mt_v0(const struct sk_buff *skb, const struct xt_match_param *par) { - const struct xt_hashlimit_info *r = - ((const struct xt_hashlimit_info *)par->matchinfo)->u.master; + const struct xt_hashlimit_info *r = par->matchinfo; struct xt_hashlimit_htable *hinfo = r->hinfo; unsigned long now = jiffies; struct dsthash_ent *dh;
@@ -702,8 +701,6 @@ static bool hashlimit_mt_check_v0(const struct xt_mtchk_param *par) } mutex_unlock(&hlimit_mutex); - /* Ugly hack: For SMP, we only want to use one set */ - r->u.master = r; return true; }