netfilter: conntrack: optimization, remove central spinlock
Subsystems (3)
v3 cover netfilter: conntrack: optimization, remove central spinlock
This patchset change the conntrack locking and provides a huge performance improvements.
This patchset is based upon Eric Dumazet's proposed patch:
http://thread.gmane.org/gmane.linux.network/268758/focus=47306
I have in agreement with Eric Dumazet, taken over this patch (and turned it into a entire patchset).
Primary focus is to remove the central spinlock nf_conntrack_lock. This requires several steps to be acheived.
Patch01: Trivial cleanups
Patch02: Moves the "special" dying/unconfirmed/template lists to use a
per cpu spinlock.
Patch03: Is preparing for patch04, as it address a race
condition. Doing this a seperate patch for reviewers sake.
Patch04: Seperates expect locking from nf_conntrack_lock. The expect
list is small (default max 256), this it just get a single lock.
Patch05: Finally can remove nf_conntrack_lock, and instead uses an
array of hashed spinlocks to protect insertions/deletions of conntracks into the hash table. While still allowing dynamic resizing of the hash table.
Testing
-------
For expectations I've mostly tested the FTP nf_conntrack_ftp helper module, by commands:
for x in seq 1 300; do \
echo $x; \
echo -e "USER anonymous\nPASS nothing\nPASV" | nc 192.168.42.129 21; \
done
wget ftp://192.168.42.129/pub/delete.me.4k -O /dev/null
For overload/DoS testing, I've primarily done, SYN-flood attack testing. Results on a 24-core E5-2695v2(ES) with 10Gbit/s ixgbe (with tool trafgen)
Base kernel : New 810.405 conntrack/sec
Fixed kernel: New 2.233.876 conntrack/sec
Notice other floods attack (SYN+ACK or ACK) can easily be deflected using:
# iptables -A INPUT -m state --state INVALID -j DROP # sysctl -w net/netfilter/nf_conntrack_tcp_loose=0
E.g. this machine can reflect 6.481.463 "invalid" conntrack/sec (from an ACK-flood).
Perf data:
----------
The nf_conntrack_lock is suffers from huge contention on current generation servers (8 or more core/threads). Data from under SYN-flooding (without a listen socket)
Perf locking congestion is very "visible" on a base kernel:
- 72.56% ksoftirqd/6 [kernel.kallsyms] [k] _raw_spin_lock_bh
- _raw_spin_lock_bh
+ 25.33% init_conntrack
+ 24.86% nf_ct_delete_from_lists
+ 24.62% __nf_conntrack_confirm
+ 24.38% destroy_conntrack
+ 0.70% tcp_packet
+ 2.21% ksoftirqd/6 [kernel.kallsyms] [k] fib_table_lookup + 1.15% ksoftirqd/6 [kernel.kallsyms] [k] __slab_free + 0.77% ksoftirqd/6 [kernel.kallsyms] [k] inet_getpeer + 0.70% ksoftirqd/6 [nf_conntrack] [k] nf_ct_delete + 0.55% ksoftirqd/6 [ip_tables] [k] ipt_do_table
Perf after the patchset (SYN-flood attack):
+ 9.62% ksoftirqd/6 [kernel.kallsyms] [k] fib_table_lookup
+ 3.78% ksoftirqd/6 [kernel.kallsyms] [k] __slab_free
+ 2.71% ksoftirqd/6 [kernel.kallsyms] [k] inet_getpeer
+ 2.55% ksoftirqd/6 [kernel.kallsyms] [k] check_leaf
+ 2.38% ksoftirqd/6 [ip_tables] [k] ipt_do_table
+ 2.06% ksoftirqd/6 [kernel.kallsyms] [k] __slab_alloc
+ 1.94% ksoftirqd/6 [nf_conntrack] [k] __nf_conntrack_alloc
- 1.94% ksoftirqd/6 [kernel.kallsyms] [k] _raw_spin_lock
- _raw_spin_lock
+ 90.32% nf_conntrack_double_lock
+ 3.61% get_partial_node
+ 1.81% nf_ct_delete_from_lists
+ 1.68% __nf_conntrack_confirm
+ 1.03% sch_direct_xmit
+ 0.52% scheduler_tick
+ 1.86% ksoftirqd/6 [kernel.kallsyms] [k] nf_iterate
+ 1.80% ksoftirqd/6 [nf_conntrack] [k] init_conntrack
+ 1.77% ksoftirqd/6 [kernel.kallsyms] [k] __neigh_event_send
- 1.70% ksoftirqd/6 [kernel.kallsyms] [k] _raw_spin_lock_bh
- _raw_spin_lock_bh
+ 32.55% nf_ct_del_from_dying_or_unconfirmed_list
+ 25.33% init_conntrack
+ 19.88% tcp_packet
+ 17.97% nf_ct_delete_from_lists
+ 1.62% nf_conntrack_in
+ 1.33% ixgbe_poll
+ 0.74% destroy_conntrack
+ 1.64% ksoftirqd/6 [nf_conntrack] [k] hash_conntrack_raw
+ 1.58% ksoftirqd/6 [kernel.kallsyms] [k] __netif_receive_skb_core
+ 1.51% ksoftirqd/6 [nf_conntrack] [k] __nf_conntrack_find_get
+ 1.48% ksoftirqd/6 [kernel.kallsyms] [k] __cmpxchg_double_slab
+ 1.46% ksoftirqd/6 [nf_conntrack] [k] nf_conntrack_in
+ 1.45% ksoftirqd/6 [kernel.kallsyms] [k] __local_bh_enable_ip---
Jesper Dangaard Brouer (5):
netfilter: conntrack: remove central spinlock nf_conntrack_lock
netfilter: conntrack: seperate expect locking from nf_conntrack_lock
netfilter: avoid race with exp->master ct
netfilter: conntrack: spinlock per cpu to protect special lists.
netfilter: trivial code cleanup and doc changes
include/net/netfilter/nf_conntrack.h | 11 + include/net/netfilter/nf_conntrack_core.h | 9 + include/net/netns/conntrack.h | 13 + net/netfilter/nf_conntrack_core.c | 432 ++++++++++++++++++++--------- net/netfilter/nf_conntrack_expect.c | 36 ++ net/netfilter/nf_conntrack_h323_main.c | 4 net/netfilter/nf_conntrack_helper.c | 41 ++- net/netfilter/nf_conntrack_netlink.c | 128 +++++---- net/netfilter/nf_conntrack_sip.c | 8 - 9 files changed, 461 insertions(+), 221 deletions(-)
PATCH v3 1/5 netfilter: trivial code cleanup and doc changes
Changes while reading through the netfilter code.
Added hint about how conntrack nf_conn refcnt is accessed. And renamed repl_hash to reply_hash for readability
---
V3: fix whitespace in nf_conntrack_expect.c
include/net/netfilter/nf_conntrack.h | 8 +++++++- net/netfilter/nf_conntrack_core.c | 20 ++++++++++---------- net/netfilter/nf_conntrack_expect.c | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-)
include/net/netfilter/nf_conntrack.h +7 -1
@@ -73,7 +73,13 @@ struct nf_conn_help {struct nf_conn {/* Usage count in here is 1 for hash table/destruct timer, 1 per skb, -plus 1 for any connection(s) we are `master' for */ +* plus 1 for any connection(s) we are `master' for +* +* Hint, SKB address this struct and refcnt via skb->nfct and +* helpers nf_conntrack_get() and nf_conntrack_put(). +* Helper nf_ct_put() equals nf_conntrack_put() by dec refcnt, +* beware nf_ct_get() is different and don't inc refcnt. +*/ struct nf_conntrack ct_general;spinlock_t lock;
net/netfilter/nf_conntrack_core.c +10 -10
@@ -408,21 +408,21 @@ EXPORT_SYMBOL_GPL(nf_conntrack_find_get);static void __nf_conntrack_hash_insert (struct nf_conn *ct,unsigned int hash, -unsigned int repl_hash) +unsigned int reply_hash) {struct net *net =nf_ct_net (ct);hlist_nulls_add_head_rcu (&ct->tuplehash[IP_CT_DIR_ORIGINAL ].hnnode, &net->ct.hash[hash]);hlist_nulls_add_head_rcu (&ct->tuplehash[IP_CT_DIR_REPLY ].hnnode, - &net->ct.hash[repl_hash]); + &net->ct.hash[reply_hash]); }int nf_conntrack_hash_check_insert (struct nf_conn *ct) {struct net *net =nf_ct_net (ct); -unsigned int hash, repl_hash; +unsigned int hash, reply_hash;struct nf_conntrack_tuple_hash *h;struct hlist_nulls_node *n;u16 zone;
@@ -430,7 +430,7 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct) zone =nf_ct_zone (ct); hash =hash_conntrack (net, zone, &ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple); - repl_hash =hash_conntrack (net, zone, + reply_hash =hash_conntrack (net, zone, &ct->tuplehash[IP_CT_DIR_REPLY ].tuple);spin_lock_bh (&nf_conntrack_lock);
@@ -441,7 +441,7 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct) &h->tuple) && zone == nf_ct_zone(nf_ct_tuplehash_to_ctrack(h))) goto out; -hlist_nulls_for_each_entry (h, n, &net->ct.hash[repl_hash], hnnode) +hlist_nulls_for_each_entry (h, n, &net->ct.hash[reply_hash], hnnode)if (nf_ct_tuple_equal (&ct->tuplehash[IP_CT_DIR_REPLY ].tuple, &h->tuple) && zone ==nf_ct_zone (nf_ct_tuplehash_to_ctrack (h)))
@@ -451,7 +451,7 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)smp_wmb ();/* The caller holds a reference to this object */ atomic_set (&ct->ct_general.use,2 ); -__nf_conntrack_hash_insert (ct, hash, repl_hash); +__nf_conntrack_hash_insert (ct, hash, reply_hash);NF_CT_STAT_INC (net, insert);spin_unlock_bh (&nf_conntrack_lock);
@@ -483,7 +483,7 @@ EXPORT_SYMBOL_GPL(nf_conntrack_tmpl_insert);int __nf_conntrack_confirm (struct sk_buff *skb) { -unsigned int hash, repl_hash; +unsigned int hash, reply_hash;struct nf_conntrack_tuple_hash *h;struct nf_conn *ct;struct nf_conn_help *help;
@@ -507,7 +507,7 @@ __nf_conntrack_confirm(struct sk_buff *skb)/* reuse the hash saved before */ hash = *(unsigned long *)&ct->tuplehash[IP_CT_DIR_REPLY ].hnnode.pprev; hash =hash_bucket (hash, net); - repl_hash =hash_conntrack (net, zone, + reply_hash =hash_conntrack (net, zone, &ct->tuplehash[IP_CT_DIR_REPLY ].tuple);/* We're not in hash table, and we refuse to set up related
@@ -540,7 +540,7 @@ __nf_conntrack_confirm(struct sk_buff *skb) &h->tuple) && zone == nf_ct_zone(nf_ct_tuplehash_to_ctrack(h))) goto out; -hlist_nulls_for_each_entry (h, n, &net->ct.hash[repl_hash], hnnode) +hlist_nulls_for_each_entry (h, n, &net->ct.hash[reply_hash], hnnode)if (nf_ct_tuple_equal (&ct->tuplehash[IP_CT_DIR_REPLY ].tuple, &h->tuple) && zone ==nf_ct_zone (nf_ct_tuplehash_to_ctrack (h)))
@@ -570,7 +570,7 @@ __nf_conntrack_confirm(struct sk_buff *skb)* guarantee that no other CPU can find the conntrack before the above * stores are visible. */ -__nf_conntrack_hash_insert (ct, hash, repl_hash); +__nf_conntrack_hash_insert (ct, hash, reply_hash);NF_CT_STAT_INC (net, insert);spin_unlock_bh (&nf_conntrack_lock);
net/netfilter/nf_conntrack_expect.c +1 -1
@@ -417,7 +417,7 @@ out:return ret; } -int nf_ct_expect_related_report (struct nf_conntrack_expect *expect, +int nf_ct_expect_related_report (struct nf_conntrack_expect *expect,u32 portid,int report) {int ret;
PATCH v3 2/5 netfilter: conntrack: spinlock per cpu to protect special lists.
In reply to Jesper Dangaard BrouerOne spinlock per cpu to protect dying/unconfirmed/template special lists. (These lists are now per cpu, a bit like the untracked ct) Add a @cpu field to nf_conn, to make sure we hold the appropriate spinlock at removal time.
---
V2: Address DaveM's concerns
* Use smp_processor_id() instead of raw_smp_processor_id()
* Make use of local_bh_disable/enable to
(1) make smp_process_id() safe,
(2) and save some spin_lock_bh() calls
include/net/netfilter/nf_conntrack.h | 3 - include/net/netns/conntrack.h | 11 ++- net/netfilter/nf_conntrack_core.c | 141 +++++++++++++++++++++++++--------- net/netfilter/nf_conntrack_helper.c | 11 ++- net/netfilter/nf_conntrack_netlink.c | 81 +++++++++++--------- 5 files changed, 168 insertions(+), 79 deletions(-)
include/net/netfilter/nf_conntrack.h +2 -1
@@ -82,7 +82,8 @@ struct nf_conn {*/ struct nf_conntrack ct_general; -spinlock_t lock; +spinlock_t lock; +u16 cpu;/* XXX should I move this to the tail ? - Y.K */ /* These are my tuples; original and reply */
include/net/netns/conntrack.h +8 -3
@@ -62,6 +62,13 @@ struct nf_ip_net {#endif }; +struct ct_pcpu { +spinlock_t lock; +struct hlist_nulls_head unconfirmed; +struct hlist_nulls_head dying; +struct hlist_nulls_head tmpl; +}; +struct netns_ct {atomic_t count;unsigned int expect_count;
@@ -86,9 +93,7 @@ struct netns_ct {struct kmem_cache *nf_conntrack_cachep;struct hlist_nulls_head *hash;struct hlist_head *expect_hash; -struct hlist_nulls_head unconfirmed; -struct hlist_nulls_head dying; -struct hlist_nulls_head tmpl; +struct ct_pcpu __percpu *pcpu_lists;struct ip_conntrack_stat __percpu *stat;struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;struct nf_exp_event_notifier __rcu *nf_expect_event_cb;
net/netfilter/nf_conntrack_core.c +103 -38
241 lines not on this page · raw message
net/netfilter/nf_conntrack_helper.c +9 -2
@@ -396,6 +396,7 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,const struct hlist_node *next;const struct hlist_nulls_node *nn;unsigned int i; +int cpu;/* Get rid of expectations */ for (i =0 ; i < nf_ct_expect_hsize; i++) {
@@ -414,8 +415,14 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me, }/* Get rid of expecteds, set helpers to NULL. */ -hlist_nulls_for_each_entry (h, nn, &net->ct.unconfirmed, hnnode) -unhelp (h, me); +for_each_possible_cpu (cpu) { +struct ct_pcpu *pcpu =per_cpu_ptr (net->ct.pcpu_lists, cpu); + +spin_lock_bh (&pcpu->lock); +hlist_nulls_for_each_entry (h, nn, &pcpu->unconfirmed, hnnode) +unhelp (h, me); +spin_unlock_bh (&pcpu->lock); + }for (i =0 ; i < net->ct.htable_size; i++) {hlist_nulls_for_each_entry (h, nn, &net->ct.hash[i], hnnode)unhelp (h, me);
net/netfilter/nf_conntrack_netlink.c +46 -35
114 lines not on this page · raw message
PATCH v3 3/5 netfilter: avoid race with exp->master ct
In reply to Jesper Dangaard BrouerPreparation for disconnecting the nf_conntrack_lock from the expectations code. Once the nf_conntrack_lock is lifted, a race condition is exposed.
The expectations master conntrack exp->master, can race with delete operations, as the refcnt increment happens too late in init_conntrack(). Race is against other CPUs invoking ->destroy() (destroy_conntrack()), or nf_ct_delete() (via timeout or early_drop()).
Avoid this race in nf_ct_find_expectation() by using atomic_inc_not_zero(), and checking if nf_ct_is_dying() (path via nf_ct_delete()).
---
V2: Address Florian Westphal concerns
* add check after !nf_ct_is_confirmed()
* handle exit case if del_timer(&exp->timeout) fails
net/netfilter/nf_conntrack_core.c | 2 +- net/netfilter/nf_conntrack_expect.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletions(-)
net/netfilter/nf_conntrack_core.c +1 -1
@@ -902,6 +902,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl, ct, exp);/* Welcome, Mr. Bond. We've been expecting you... */ __set_bit (IPS_EXPECTED_BIT , &ct->status); +/* exp->master safe, refcnt bumped in nf_ct_find_expectation */ ct->master = exp->master;if (exp->helper) { help = nf_ct_helper_ext_add(ct, exp->helper,
@@ -916,7 +917,6 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,#ifdef CONFIG_NF_CONNTRACK_SECMARK ct->secmark = exp->master->secmark;#endif -nf_conntrack_get (&ct->master->ct_general);NF_CT_STAT_INC (net, expect_new); }else {__nf_ct_try_assign_helper (ct, tmpl,GFP_ATOMIC );
net/netfilter/nf_conntrack_expect.c +14 -0
@@ -155,6 +155,18 @@ nf_ct_find_expectation(struct net *net, u16 zone,if (!nf_ct_is_confirmed (exp->master))return NULL ; +/* Avoid race with other CPUs, that for exp->master ct, is +* about to invoke ->destroy(), or nf_ct_delete() via timeout +* or early_drop(). +* +* The atomic_inc_not_zero() check tells: If that fails, we +* know that the ct is being destroyed. If it succeeds, we +* can be sure the ct cannot disappear underneath. +*/ +if (unlikely (nf_ct_is_dying (exp->master) || + !atomic_inc_not_zero (&exp->master->ct_general.use))) +return NULL ; +if (exp->flags &NF_CT_EXPECT_PERMANENT ) {atomic_inc (&exp->use);return exp;
@@ -162,6 +174,8 @@ nf_ct_find_expectation(struct net *net, u16 zone,nf_ct_unlink_expect (exp);return exp; } +/* Undo exp->master refcnt increase, if del_timer() failed */ +nf_ct_put (exp->master);return NULL ; }
PATCH v3 4/5 netfilter: conntrack: seperate expect locking from nf_conntrack_lock
In reply to Jesper Dangaard BrouerNetfilter expectations are protected with the same lock as conntrack entries (nf_conntrack_lock). This patch split out expectations locking to use it's own lock (nf_conntrack_expect_lock).
---
V3: Use rcu_dereference_raw() in unhelp
include/net/netfilter/nf_conntrack_core.h | 2 + net/netfilter/nf_conntrack_core.c | 60 ++++++++++++++++------------- net/netfilter/nf_conntrack_expect.c | 20 +++++----- net/netfilter/nf_conntrack_h323_main.c | 4 +- net/netfilter/nf_conntrack_helper.c | 22 +++++------ net/netfilter/nf_conntrack_netlink.c | 32 ++++++++------- net/netfilter/nf_conntrack_sip.c | 8 ++-- 7 files changed, 79 insertions(+), 69 deletions(-)
include/net/netfilter/nf_conntrack_core.h +2 -0
@@ -79,4 +79,6 @@ print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,extern spinlock_t nf_conntrack_lock ; +extern spinlock_t nf_conntrack_expect_lock; +#endif /* _NF_CONNTRACK_CORE_H */
net/netfilter/nf_conntrack_core.c +33 -27
109 lines not on this page · raw message
net/netfilter/nf_conntrack_expect.c +11 -9
@@ -66,9 +66,9 @@ static void nf_ct_expectation_timed_out(unsigned long ul_expect) {struct nf_conntrack_expect *exp = (void *)ul_expect; -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);nf_ct_unlink_expect (exp); -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);nf_ct_expect_put (exp); }
@@ -191,12 +191,14 @@ void nf_ct_remove_expectations(struct nf_conn *ct)if (!help)return ; +spin_lock_bh (&nf_conntrack_expect_lock);hlist_for_each_entry_safe (exp, next, &help->expectations, lnode) {if (del_timer (&exp->timeout)) {nf_ct_unlink_expect (exp);nf_ct_expect_put (exp); } } +spin_unlock_bh (&nf_conntrack_expect_lock); }EXPORT_SYMBOL_GPL (nf_ct_remove_expectations);
@@ -231,12 +233,12 @@ static inline int expect_matches(const struct nf_conntrack_expect *a,/* Generally a bad idea to call this: could have matched already. */ void nf_ct_unexpect_related (struct nf_conntrack_expect *exp) { -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);if (del_timer (&exp->timeout)) {nf_ct_unlink_expect (exp);nf_ct_expect_put (exp); } -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); }EXPORT_SYMBOL_GPL (nf_ct_unexpect_related);
@@ -349,7 +351,7 @@ static int nf_ct_expect_insert(struct nf_conntrack_expect *exp)setup_timer (&exp->timeout, nf_ct_expectation_timed_out, (unsigned long )exp); helper =rcu_dereference_protected (master_help->helper, -lockdep_is_held (&nf_conntrack_lock)); +lockdep_is_held (&nf_conntrack_expect_lock));if (helper) { exp->timeout.expires = jiffies + helper->expect_policy[exp->class].timeout *HZ ;
@@ -409,7 +411,7 @@ static inline int __nf_ct_expect_check(struct nf_conntrack_expect *expect) }/* Will be over limit? */ helper =rcu_dereference_protected (master_help->helper, -lockdep_is_held (&nf_conntrack_lock)); +lockdep_is_held (&nf_conntrack_expect_lock));if (helper) { p = &helper->expect_policy[expect->class]; if (p->max_expected &&
@@ -436,7 +438,7 @@ int nf_ct_expect_related_report(struct nf_conntrack_expect *expect, {int ret; -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock); ret =__nf_ct_expect_check (expect);if (ret <=0 ) goto out;
@@ -444,11 +446,11 @@ int nf_ct_expect_related_report(struct nf_conntrack_expect *expect, ret =nf_ct_expect_insert (expect);if (ret <0 ) goto out; -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);nf_ct_expect_event_report (IPEXP_NEW , expect, portid, report);return ret; out: -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);return ret; }EXPORT_SYMBOL_GPL (nf_ct_expect_related_report);
net/netfilter/nf_conntrack_h323_main.c +2 -2
@@ -1476,7 +1476,7 @@ static int process_rcf(struct sk_buff *skb, struct nf_conn *ct,nf_ct_refresh (ct, skb, info->timeout *HZ );/* Set expect timeout */ -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock); exp =find_expect (ct, &ct->tuplehash[dir].tuple.dst.u3, info->sig_port[!dir]);if (exp) {
@@ -1486,7 +1486,7 @@ static int process_rcf(struct sk_buff *skb, struct nf_conn *ct,nf_ct_dump_tuple (&exp->tuple);set_expect_timeout (exp, info->timeout); } -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); }return 0 ;
net/netfilter/nf_conntrack_helper.c +11 -11
@@ -250,16 +250,14 @@ out: }EXPORT_SYMBOL_GPL (__nf_ct_try_assign_helper); +/* appropiate ct lock protecting must be taken by caller */ static inline int unhelp (struct nf_conntrack_tuple_hash *i,const struct nf_conntrack_helper *me) {struct nf_conn *ct =nf_ct_tuplehash_to_ctrack (i);struct nf_conn_help *help =nfct_help (ct); -if (help &&rcu_dereference_protected ( - help->helper, -lockdep_is_held (&nf_conntrack_lock) - ) == me) { +if (help &&rcu_dereference_raw (help->helper) == me) {nf_conntrack_event (IPCT_HELPER , ct);RCU_INIT_POINTER (help->helper,NULL ); }
@@ -284,17 +282,17 @@ static LIST_HEAD(nf_ct_helper_expectfn_list);void nf_ct_helper_expectfn_register (struct nf_ct_helper_expectfn *n) { -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);list_add_rcu (&n->head, &nf_ct_helper_expectfn_list); -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); }EXPORT_SYMBOL_GPL (nf_ct_helper_expectfn_register);void nf_ct_helper_expectfn_unregister (struct nf_ct_helper_expectfn *n) { -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);list_del_rcu (&n->head); -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); }EXPORT_SYMBOL_GPL (nf_ct_helper_expectfn_unregister);
@@ -399,13 +397,14 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,int cpu;/* Get rid of expectations */ +spin_lock_bh (&nf_conntrack_expect_lock);for (i =0 ; i < nf_ct_expect_hsize; i++) {hlist_for_each_entry_safe (exp, next, &net->ct.expect_hash[i], hnode) {struct nf_conn_help *help =nfct_help (exp->master);if ((rcu_dereference_protected ( help->helper, -lockdep_is_held (&nf_conntrack_lock) +lockdep_is_held (&nf_conntrack_expect_lock) ) == me || exp->helper == me) &&del_timer (&exp->timeout)) {nf_ct_unlink_expect (exp);
@@ -413,6 +412,7 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me, } } } +spin_unlock_bh (&nf_conntrack_expect_lock);/* Get rid of expecteds, set helpers to NULL. */ for_each_possible_cpu (cpu) {
@@ -423,10 +423,12 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,unhelp (h, me);spin_unlock_bh (&pcpu->lock); } +spin_lock_bh (&nf_conntrack_lock);for (i =0 ; i < net->ct.htable_size; i++) {hlist_nulls_for_each_entry (h, nn, &net->ct.hash[i], hnnode)unhelp (h, me); } +spin_unlock_bh (&nf_conntrack_lock); }void nf_conntrack_helper_unregister (struct nf_conntrack_helper *me)
@@ -444,10 +446,8 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)synchronize_rcu ();rtnl_lock (); -spin_lock_bh (&nf_conntrack_lock);for_each_net (net)__nf_conntrack_helper_unregister (me, net); -spin_unlock_bh (&nf_conntrack_lock);rtnl_unlock (); }EXPORT_SYMBOL_GPL (nf_conntrack_helper_unregister);
net/netfilter/nf_conntrack_netlink.c +16 -16
@@ -1377,14 +1377,14 @@ ctnetlink_change_helper(struct nf_conn *ct, const struct nlattr * const cda[]) nf_ct_protonum(ct));if (helper ==NULL ) {#ifdef CONFIG_MODULES -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);if (request_module ("nfct-helper-%s" , helpname) <0 ) { -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);return -EOPNOTSUPP ; } -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock); helper =__nf_conntrack_helper_find (helpname,nf_ct_l3num (ct),nf_ct_protonum (ct));if (helper)
@@ -1822,9 +1822,9 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, err = -EEXIST ; ct =nf_ct_tuplehash_to_ctrack (h);if (!(nlh->nlmsg_flags &NLM_F_EXCL )) { -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock); err =ctnetlink_change_conntrack (ct, cda); -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);if (err ==0 ) { nf_conntrack_eventmask_report((1 << IPCT_REPLY) | (1 << IPCT_ASSURED) |
@@ -2153,9 +2153,9 @@ ctnetlink_nfqueue_parse(const struct nlattr *attr, struct nf_conn *ct)if (ret <0 )return ret; -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock); ret =ctnetlink_nfqueue_parse_ct ((const struct nlattr **)cda, ct); -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);return ret; }
@@ -2710,13 +2710,13 @@ ctnetlink_del_expect(struct sock *ctnl, struct sk_buff *skb, }/* after list removal, usage count == 1 */ -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);if (del_timer (&exp->timeout)) {nf_ct_unlink_expect_report (exp,NETLINK_CB (skb).portid,nlmsg_report (nlh));nf_ct_expect_put (exp); } -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);/* have to put what we 'get' above. * after this line usage count == 0 */ nf_ct_expect_put (exp);
@@ -2725,7 +2725,7 @@ ctnetlink_del_expect(struct sock *ctnl, struct sk_buff *skb,struct nf_conn_help *m_help;/* delete all expectations for this helper */ -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);for (i =0 ; i < nf_ct_expect_hsize; i++) { hlist_for_each_entry_safe(exp, next, &net->ct.expect_hash[i],
@@ -2740,10 +2740,10 @@ ctnetlink_del_expect(struct sock *ctnl, struct sk_buff *skb, } } } -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); }else {/* This basically means we have to flush everything*/ -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);for (i =0 ; i < nf_ct_expect_hsize; i++) { hlist_for_each_entry_safe(exp, next, &net->ct.expect_hash[i],
@@ -2756,7 +2756,7 @@ ctnetlink_del_expect(struct sock *ctnl, struct sk_buff *skb, } } } -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); }return 0 ;
@@ -2982,11 +2982,11 @@ ctnetlink_new_expect(struct sock *ctnl, struct sk_buff *skb,if (err <0 )return err; -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock); exp =__nf_ct_expect_find (net, zone, &tuple);if (!exp) { -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); err = -ENOENT ;if (nlh->nlmsg_flags &NLM_F_CREATE ) { err = ctnetlink_create_expect(net, zone, cda,
@@ -3000,7 +3000,7 @@ ctnetlink_new_expect(struct sock *ctnl, struct sk_buff *skb, err = -EEXIST ;if (!(nlh->nlmsg_flags &NLM_F_EXCL )) err =ctnetlink_change_expect (exp, cda); -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);return err; }
net/netfilter/nf_conntrack_sip.c +4 -4
@@ -800,7 +800,7 @@ static int refresh_signalling_expectation(struct nf_conn *ct,struct hlist_node *next;int found =0 ; -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);hlist_for_each_entry_safe (exp, next, &help->expectations, lnode) { if (exp->class != SIP_EXPECT_SIGNALLING || !nf_inet_addr_cmp(&exp->tuple.dst.u3, addr) ||
@@ -815,7 +815,7 @@ static int refresh_signalling_expectation(struct nf_conn *ct, found =1 ;break ; } -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock);return found; }
@@ -825,7 +825,7 @@ static void flush_expectations(struct nf_conn *ct, bool media)struct nf_conntrack_expect *exp;struct hlist_node *next; -spin_lock_bh (&nf_conntrack_lock); +spin_lock_bh (&nf_conntrack_expect_lock);hlist_for_each_entry_safe (exp, next, &help->expectations, lnode) {if ((exp->class !=SIP_EXPECT_SIGNALLING ) ^ media)continue ;
@@ -836,7 +836,7 @@ static void flush_expectations(struct nf_conn *ct, bool media)if (!media)break ; } -spin_unlock_bh (&nf_conntrack_lock); +spin_unlock_bh (&nf_conntrack_expect_lock); } static int set_expected_rtp_rtcp(struct sk_buff *skb, unsigned int protoff,
PATCH v3 5/5 netfilter: conntrack: remove central spinlock nf_conntrack_lock
In reply to Jesper Dangaard Brouernf_conntrack_lock is a monolithic lock and suffers from huge contention on current generation servers (8 or more core/threads).
Perf locking congestion is clear on base kernel:
- 72.56% ksoftirqd/6 [kernel.kallsyms] [k] _raw_spin_lock_bh
- _raw_spin_lock_bh
+ 25.33% init_conntrack
+ 24.86% nf_ct_delete_from_lists
+ 24.62% __nf_conntrack_confirm
+ 24.38% destroy_conntrack
+ 0.70% tcp_packet
+ 2.21% ksoftirqd/6 [kernel.kallsyms] [k] fib_table_lookup + 1.15% ksoftirqd/6 [kernel.kallsyms] [k] __slab_free + 0.77% ksoftirqd/6 [kernel.kallsyms] [k] inet_getpeer + 0.70% ksoftirqd/6 [nf_conntrack] [k] nf_ct_delete + 0.55% ksoftirqd/6 [ip_tables] [k] ipt_do_table
This patch change conntrack locking and provides a huge performance improvement. SYN-flood attack tested on a 24-core E5-2695v2(ES) with 10Gbit/s ixgbe (with tool trafgen):
Base kernel: 810.405 new conntrack/sec
After patch: 2.233.876 new conntrack/sec
Notice other floods attack (SYN+ACK or ACK) can easily be deflected using:
# iptables -A INPUT -m state --state INVALID -j DROP # sysctl -w net/netfilter/nf_conntrack_tcp_loose=0
Use an array of hashed spinlocks to protect insertions/deletions of conntracks into the hash table. 1024 spinlocks seem to give good results, at minimal cost (4KB memory). Due to lockdep max depth, 1024 becomes 8 if CONFIG_LOCKDEP=y
The hash resize is a bit tricky, because we need to take all locks in the array. A seqcount_t is used to synchronize the hash table users with the resizing process.
---
include/net/netfilter/nf_conntrack_core.h | 7 + include/net/netns/conntrack.h | 2 net/netfilter/nf_conntrack_core.c | 219 +++++++++++++++++++++-------- net/netfilter/nf_conntrack_helper.c | 12 +- net/netfilter/nf_conntrack_netlink.c | 15 ++ 5 files changed, 188 insertions(+), 67 deletions(-)
include/net/netfilter/nf_conntrack_core.h +6 -1
@@ -77,7 +77,12 @@ print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,const struct nf_conntrack_l3proto *l3proto, const struct nf_conntrack_l4proto *proto); -extern spinlock_t nf_conntrack_lock ; +#ifdef CONFIG_LOCKDEP +# define CONNTRACK_LOCKS 8 +#else +# define CONNTRACK_LOCKS 1024 +#endif +extern spinlock_t nf_conntrack_locks[CONNTRACK_LOCKS ];extern spinlock_t nf_conntrack_expect_lock;
include/net/netns/conntrack.h +2 -0
@@ -5,6 +5,7 @@#include <linux/list_nulls.h> #include <linux/atomic.h> #include <linux/netfilter/nf_conntrack_tcp.h> +#include <linux/seqlock.h> struct ctl_table_header ;struct nf_conntrack_ecache ;
@@ -90,6 +91,7 @@ struct netns_ct {int sysctl_checksum;unsigned int htable_size; +seqcount_t generation;struct kmem_cache *nf_conntrack_cachep;struct hlist_nulls_head *hash;struct hlist_head *expect_hash;
net/netfilter/nf_conntrack_core.c +159 -60
372 lines not on this page · raw message
net/netfilter/nf_conntrack_helper.c +8 -4
@@ -423,12 +423,16 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,unhelp (h, me);spin_unlock_bh (&pcpu->lock); } -spin_lock_bh (&nf_conntrack_lock); +local_bh_disable ();for (i =0 ; i < net->ct.htable_size; i++) { -hlist_nulls_for_each_entry (h, nn, &net->ct.hash[i], hnnode) -unhelp (h, me); +spin_lock (&nf_conntrack_locks[i %CONNTRACK_LOCKS ]); +if (i < net->ct.htable_size) { +hlist_nulls_for_each_entry (h, nn, &net->ct.hash[i], hnnode) +unhelp (h, me); + } +spin_unlock (&nf_conntrack_locks[i %CONNTRACK_LOCKS ]); } -spin_unlock_bh (&nf_conntrack_lock); +local_bh_enable (); }void nf_conntrack_helper_unregister (struct nf_conntrack_helper *me)
net/netfilter/nf_conntrack_netlink.c +13 -2
@@ -764,14 +764,23 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)struct nfgenmsg *nfmsg =nlmsg_data (cb->nlh);u_int8_t l3proto = nfmsg->nfgen_family;int res; +spinlock_t *lockp; +#ifdef CONFIG_NF_CONNTRACK_MARK const struct ctnetlink_dump_filter *filter = cb->data;#endif -spin_lock_bh (&nf_conntrack_lock); last = (struct nf_conn *)cb->args[1 ]; + +local_bh_disable ();for (; cb->args[0 ] < net->ct.htable_size; cb->args[0 ]++) { restart: + lockp = &nf_conntrack_locks[cb->args[0 ] %CONNTRACK_LOCKS ]; +spin_lock (lockp); +if (cb->args[0 ] >= net->ct.htable_size) { +spin_unlock (lockp); + goto out; + }hlist_nulls_for_each_entry (h, n, &net->ct.hash[cb->args[0 ]], hnnode) {if (NF_CT_DIRECTION (h) !=IP_CT_DIR_ORIGINAL )
@@ -803,16 +812,18 @@ restart:if (res <0 ) {nf_conntrack_get (&ct->ct_general); cb->args[1 ] = (unsigned long )ct; +spin_unlock (lockp); goto out; } } +spin_unlock (lockp);if (cb->args[1 ]) { cb->args[1 ] =0 ; goto restart; } } out: -spin_unlock_bh (&nf_conntrack_lock); +local_bh_enable ();if (last)nf_ct_put (last);
David Miller
maintainerFrom: Jesper Dangaard Brouer <[email protected]>
Date: Mon, 03 Mar 2014 14:44:16 +0100
Quoted 10 lines
This patchset change the conntrack locking and provides a huge performance improvements.
This patchset is based upon Eric Dumazet's proposed patch:
http://thread.gmane.org/gmane.linux.network/268758/focus=47306
I have in agreement with Eric Dumazet, taken over this patch (and turned it into a entire patchset).Primary focus is to remove the central spinlock
nf_conntrack_lock. This requires several steps to be acheived.
...
For series:
Quoted 4 lines · Jesper Dangaard Brouer
This patchset is based upon Eric Dumazet's proposed patch:
http://thread.gmane.org/gmane.linux.network/268758/focus=47306
I have in agreement with Eric Dumazet, taken over this patch (and turned it into a entire patchset).
FWIW I think that this series should be merged for 3.15, thus:
I see no more issues with these patches, thanks Jesper for resurrecting Erics work.
Quoted 6 lines · Florian Westphal
Quoted 4 lines · Jesper Dangaard Brouer
This patchset is based upon Eric Dumazet's proposed patch:
http://thread.gmane.org/gmane.linux.network/268758/focus=47306
I have in agreement with Eric Dumazet, taken over this patch (and turned it into a entire patchset).
Quoted 7 lines
FWIW I think that this series should be merged for 3.15, thus:
Reviewed-by Florian Westphal <[email protected]>I see no more issues with these patches, thanks Jesper for resurrecting Erics work.
--
Indeed, thanks so much Jesper !
Quoted 11 lines · David Miller
From: Jesper Dangaard Brouer <[email protected]>
Date: Mon, 03 Mar 2014 14:44:16 +0100Quoted 8 lines
This patchset change the conntrack locking and provides a huge performance improvements.
This patchset is based upon Eric Dumazet's proposed patch:
http://thread.gmane.org/gmane.linux.network/268758/focus=47306
I have in agreement with Eric Dumazet, taken over this patch (and turned it into a entire patchset).
Quoted 7 lines
Quoted 2 lines
Primary focus is to remove the central spinlock
nf_conntrack_lock. This requires several steps to be acheived....
For series:
Signed-off-by David S. Miller <[email protected]>
Applied, thanks everyone.