Use RCU to protect the 'helpers' linked list instead of spinlocks. Signed-off-by: Martin Josefsson --- include/net/netfilter/nf_conntrack_core.h | 4 + include/net/netfilter/nf_conntrack_helper.h | 8 -- net/netfilter/nf_conntrack_core.c | 37 ++++------- net/netfilter/nf_conntrack_helper.c | 93 +++++++++++----------------- net/netfilter/nf_conntrack_netlink.c | 10 --- net/netfilter/xt_helper.c | 4 - 6 files changed, 64 insertions(+), 92 deletions(-) Index: linux-2.6.23-rc6.quilt/net/netfilter/nf_conntrack_core.c =================================================================== --- linux-2.6.23-rc6.quilt.orig/net/netfilter/nf_conntrack_core.c 2007-09-12 11:26:07.000000000 +0200 +++ linux-2.6.23-rc6.quilt/net/netfilter/nf_conntrack_core.c 2007-09-14 18:26:02.000000000 +0200 @@ -387,7 +387,7 @@ __nf_conntrack_confirm(struct sk_buff ** NF_CT_STAT_INC(insert); spin_unlock_bh(&nf_conntrack_lock); help = nfct_help(ct); - if (help && help->helper) + if (help && rcu_dereference(help->helper)) nf_conntrack_event_cache(IPCT_HELPER, ct); #ifdef CONFIG_NF_NAT_NEEDED if (test_bit(IPS_SRC_NAT_DONE_BIT, &ct->status) || @@ -548,15 +548,19 @@ init_conntrack(const struct nf_conntrack spin_lock_bh(&nf_conntrack_lock); exp = nf_ct_find_expectation(tuple); if (exp) { + struct nf_conntrack_helper *helper; + pr_debug("conntrack: expectation arrives ct=%p exp=%p\n", conntrack, exp); /* Welcome, Mr. Bond. We've been expecting you... */ __set_bit(IPS_EXPECTED_BIT, &conntrack->status); conntrack->master = exp->master; - if (exp->helper) { + + helper = rcu_dereference(exp->helper); + if (helper) { help = nf_ct_helper_ext_add(conntrack, GFP_ATOMIC); if (help) - rcu_assign_pointer(help->helper, exp->helper); + rcu_assign_pointer(help->helper, helper); } #ifdef CONFIG_NF_CONNTRACK_MARK @@ -570,7 +574,7 @@ init_conntrack(const struct nf_conntrack } else { struct nf_conntrack_helper *helper; - helper = __nf_ct_helper_find(&repl_tuple); + helper = nf_ct_helper_find(&repl_tuple); if (helper) { help = nf_ct_helper_ext_add(conntrack, GFP_ATOMIC); if (help) @@ -750,8 +754,9 @@ void nf_conntrack_alter_reply(struct nf_ struct nf_conn_help *help = nfct_help(ct); struct nf_conntrack_helper *helper; - spin_lock_bh(&nf_conntrack_lock); - /* Should be unconfirmed, so not in hash table yet */ + /* Should be unconfirmed, so not in hash table yet. + No need to use locks since we are the only one + able to see this. */ NF_CT_ASSERT(!nf_ct_is_confirmed(ct)); pr_debug("Altering reply tuple of %p to ", ct); @@ -759,26 +764,24 @@ void nf_conntrack_alter_reply(struct nf_ ct->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply; if (ct->master || (help && help->expecting != 0)) - goto out; + return; - helper = __nf_ct_helper_find(newreply); + helper = nf_ct_helper_find(newreply); if (helper == NULL) { if (help) rcu_assign_pointer(help->helper, NULL); - goto out; + return; } if (help == NULL) { help = nf_ct_helper_ext_add(ct, GFP_ATOMIC); if (help == NULL) - goto out; + return; } else { memset(&help->help, 0, sizeof(help->help)); } rcu_assign_pointer(help->helper, helper); -out: - spin_unlock_bh(&nf_conntrack_lock); } EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply); @@ -906,16 +909,8 @@ void __nf_conntrack_attach(struct sk_buf } EXPORT_SYMBOL_GPL(__nf_conntrack_attach); -static inline int -do_iter(const struct nf_conntrack_tuple_hash *i, - int (*iter)(struct nf_conn *i, void *data), - void *data) -{ - return iter(nf_ct_tuplehash_to_ctrack(i), data); -} - /* Bring out ya dead! */ -static struct nf_conn * +struct nf_conn * get_next_corpse(int (*iter)(struct nf_conn *i, void *data), void *data, unsigned int *bucket) { Index: linux-2.6.23-rc6.quilt/net/netfilter/xt_helper.c =================================================================== --- linux-2.6.23-rc6.quilt.orig/net/netfilter/xt_helper.c 2007-09-12 11:25:07.000000000 +0200 +++ linux-2.6.23-rc6.quilt/net/netfilter/xt_helper.c 2007-09-12 11:26:07.000000000 +0200 @@ -56,8 +56,8 @@ match(const struct sk_buff *skb, if (info->name[0] == '\0') ret = !ret; else - ret ^= !strncmp(master_help->helper->name, info->name, - strlen(master_help->helper->name)); + ret ^= !strncmp(helper->name, info->name, + strlen(helper->name)); return ret; } Index: linux-2.6.23-rc6.quilt/net/netfilter/nf_conntrack_helper.c =================================================================== --- linux-2.6.23-rc6.quilt.orig/net/netfilter/nf_conntrack_helper.c 2007-09-12 11:26:07.000000000 +0200 +++ linux-2.6.23-rc6.quilt/net/netfilter/nf_conntrack_helper.c 2007-09-14 18:25:37.000000000 +0200 @@ -42,8 +42,9 @@ static unsigned int helper_hash(const st (__force __u16)tuple->src.u.all) % nf_ct_helper_hsize; } +/* Must hold rcu_read_lock() when calling this function */ struct nf_conntrack_helper * -__nf_ct_helper_find(const struct nf_conntrack_tuple *tuple) +nf_ct_helper_find(const struct nf_conntrack_tuple *tuple) { struct nf_conntrack_helper *helper; struct nf_conntrack_tuple_mask mask = { .src.u.all = htons(0xFFFF) }; @@ -54,59 +55,31 @@ __nf_ct_helper_find(const struct nf_conn return NULL; h = helper_hash(tuple); - hlist_for_each_entry(helper, n, &nf_ct_helper_hash[h], hnode) { + hlist_for_each_entry_rcu(helper, n, &nf_ct_helper_hash[h], hnode) { if (nf_ct_tuple_src_mask_cmp(tuple, &helper->tuple, &mask)) return helper; } return NULL; } +EXPORT_SYMBOL_GPL(nf_ct_helper_find); +/* Must hold rcu_read_lock() when calling this function */ struct nf_conntrack_helper * -nf_ct_helper_find_get(const struct nf_conntrack_tuple *tuple) -{ - struct nf_conntrack_helper *helper; - - /* need nf_conntrack_lock to assure that helper exists until - * try_module_get() is called */ - spin_lock_bh(&nf_conntrack_lock); - - helper = __nf_ct_helper_find(tuple); - if (helper) { - /* need to increase module usage count to assure helper will - * not go away while the caller is e.g. busy putting a - * conntrack in the hash that uses the helper */ - if (!try_module_get(helper->me)) - helper = NULL; - } - - spin_unlock_bh(&nf_conntrack_lock); - - return helper; -} -EXPORT_SYMBOL_GPL(nf_ct_helper_find_get); - -void nf_ct_helper_put(struct nf_conntrack_helper *helper) -{ - module_put(helper->me); -} -EXPORT_SYMBOL_GPL(nf_ct_helper_put); - -struct nf_conntrack_helper * -__nf_conntrack_helper_find_byname(const char *name) +nf_conntrack_helper_find_byname(const char *name) { struct nf_conntrack_helper *h; struct hlist_node *n; unsigned int i; for (i = 0; i < nf_ct_helper_hsize; i++) { - hlist_for_each_entry(h, n, &nf_ct_helper_hash[i], hnode) { + hlist_for_each_entry_rcu(h, n, &nf_ct_helper_hash[i], hnode) { if (!strcmp(h->name, name)) return h; } } return NULL; } -EXPORT_SYMBOL_GPL(__nf_conntrack_helper_find_byname); +EXPORT_SYMBOL_GPL(nf_conntrack_helper_find_byname); struct nf_conn_help *nf_ct_helper_ext_add(struct nf_conn *ct, gfp_t gfp) { @@ -121,17 +94,12 @@ struct nf_conn_help *nf_ct_helper_ext_ad } EXPORT_SYMBOL_GPL(nf_ct_helper_ext_add); -static inline int unhelp(struct nf_conntrack_tuple_hash *i, - const struct nf_conntrack_helper *me) +static int helper_match(struct nf_conn *i, void *data) { - struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(i); - struct nf_conn_help *help = nfct_help(ct); + struct nf_conn_help *help = nfct_help(i); + struct nf_conntrack_helper *me = data; - if (help && help->helper == me) { - nf_conntrack_event_cache(IPCT_HELPER, ct); - rcu_assign_pointer(help->helper, NULL); - } - return 0; + return (help && help->helper == me); } int nf_conntrack_helper_register(struct nf_conntrack_helper *me) @@ -141,7 +109,7 @@ int nf_conntrack_helper_register(struct BUG_ON(me->timeout == 0); spin_lock_bh(&nf_conntrack_lock); - hlist_add_head(&me->hnode, &nf_ct_helper_hash[h]); + hlist_add_head_rcu(&me->hnode, &nf_ct_helper_hash[h]); nf_ct_helper_count++; spin_unlock_bh(&nf_conntrack_lock); @@ -151,15 +119,21 @@ EXPORT_SYMBOL_GPL(nf_conntrack_helper_re void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me) { - struct nf_conntrack_tuple_hash *h; + unsigned int i; struct nf_conntrack_expect *exp; struct hlist_node *n, *next; - unsigned int i; + struct nf_conn *ct; - /* Need write lock here, to delete helper. */ + /* Need lock here, to delete helper. */ spin_lock_bh(&nf_conntrack_lock); - hlist_del(&me->hnode); + hlist_del_rcu(&me->hnode); nf_ct_helper_count--; + spin_unlock_bh(&nf_conntrack_lock); + + /* Someone could be still looking at the helper in a bh. */ + synchronize_net(); + + spin_lock_bh(&nf_conntrack_lock); /* Get rid of expectations */ for (i = 0; i < nf_ct_expect_hsize; i++) { @@ -174,15 +148,22 @@ void nf_conntrack_helper_unregister(stru } } - /* Get rid of expecteds, set helpers to NULL. */ - hlist_for_each_entry(h, n, &unconfirmed, hnode) - unhelp(h, me); - for (i = 0; i < nf_conntrack_htable_size; i++) { - hlist_for_each_entry(h, n, &nf_conntrack_hash[i], hnode) - unhelp(h, me); - } spin_unlock_bh(&nf_conntrack_lock); + i = 0; + while ((ct = get_next_corpse(helper_match, me, &i)) != NULL) { + struct nf_conn_help *help = nfct_help(ct); + + nf_conntrack_event_cache(IPCT_HELPER, ct); + + spin_lock_bh(&nf_conntrack_lock); + help->helper = NULL; + spin_unlock_bh(&nf_conntrack_lock); + + /* get_next_corpse() increases the refcount */ + nf_ct_put(ct); + } + /* Someone could be still looking at the helper in a bh. */ synchronize_net(); } Index: linux-2.6.23-rc6.quilt/include/net/netfilter/nf_conntrack_core.h =================================================================== --- linux-2.6.23-rc6.quilt.orig/include/net/netfilter/nf_conntrack_core.h 2007-09-12 11:26:07.000000000 +0200 +++ linux-2.6.23-rc6.quilt/include/net/netfilter/nf_conntrack_core.h 2007-09-12 11:26:07.000000000 +0200 @@ -78,6 +78,10 @@ static inline int nf_conntrack_confirm(s extern void __nf_conntrack_attach(struct sk_buff *nskb, struct sk_buff *skb); +extern struct nf_conn * +get_next_corpse(int (*iter)(struct nf_conn *i, void *data), + void *data, unsigned int *bucket); + int print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple, struct nf_conntrack_l3proto *l3proto, Index: linux-2.6.23-rc6.quilt/include/net/netfilter/nf_conntrack_helper.h =================================================================== --- linux-2.6.23-rc6.quilt.orig/include/net/netfilter/nf_conntrack_helper.h 2007-09-12 11:25:07.000000000 +0200 +++ linux-2.6.23-rc6.quilt/include/net/netfilter/nf_conntrack_helper.h 2007-09-12 11:26:07.000000000 +0200 @@ -40,15 +40,11 @@ struct nf_conntrack_helper }; extern struct nf_conntrack_helper * -__nf_ct_helper_find(const struct nf_conntrack_tuple *tuple); +nf_ct_helper_find(const struct nf_conntrack_tuple *tuple); extern struct nf_conntrack_helper * -nf_ct_helper_find_get( const struct nf_conntrack_tuple *tuple); +nf_conntrack_helper_find_byname(const char *name); -extern struct nf_conntrack_helper * -__nf_conntrack_helper_find_byname(const char *name); - -extern void nf_ct_helper_put(struct nf_conntrack_helper *helper); extern int nf_conntrack_helper_register(struct nf_conntrack_helper *); extern void nf_conntrack_helper_unregister(struct nf_conntrack_helper *); Index: linux-2.6.23-rc6.quilt/net/netfilter/nf_conntrack_netlink.c =================================================================== --- linux-2.6.23-rc6.quilt.orig/net/netfilter/nf_conntrack_netlink.c 2007-09-12 11:26:07.000000000 +0200 +++ linux-2.6.23-rc6.quilt/net/netfilter/nf_conntrack_netlink.c 2007-09-14 18:25:39.000000000 +0200 @@ -856,7 +856,7 @@ ctnetlink_change_helper(struct nf_conn * return 0; } - helper = __nf_conntrack_helper_find_byname(helpname); + helper = nf_conntrack_helper_find_byname(helpname); if (helper == NULL) return -EINVAL; @@ -987,11 +987,10 @@ ctnetlink_create_conntrack(struct nfattr ct->mark = ntohl(*(__be32 *)NFA_DATA(cda[CTA_MARK-1])); #endif - helper = nf_ct_helper_find_get(rtuple); + helper = nf_ct_helper_find(rtuple); if (helper) { help = nf_ct_helper_ext_add(ct, GFP_KERNEL); if (help == NULL) { - nf_ct_helper_put(helper); err = -ENOMEM; goto err; } @@ -1002,9 +1001,6 @@ ctnetlink_create_conntrack(struct nfattr add_timer(&ct->timeout); nf_conntrack_hash_insert(ct); - if (helper) - nf_ct_helper_put(helper); - return 0; err: @@ -1399,7 +1395,7 @@ ctnetlink_del_expect(struct sock *ctnl, /* delete all expectations for this helper */ spin_lock_bh(&nf_conntrack_lock); - h = __nf_conntrack_helper_find_byname(name); + h = nf_conntrack_helper_find_byname(name); if (!h) { spin_unlock_bh(&nf_conntrack_lock); return -EINVAL;