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 | 18 +---- net/netfilter/nf_conntrack_helper.c | 87 ++++++++++------------------ net/netfilter/nf_conntrack_netlink.c | 4 - net/netfilter/nf_conntrack_standalone.c | 5 - net/netfilter/xt_helper.c | 26 +++++--- 7 files changed, 65 insertions(+), 87 deletions(-) Index: linux-2.6.19-rc3-git4.quilt/net/netfilter/nf_conntrack_core.c =================================================================== --- linux-2.6.19-rc3-git4.quilt.orig/net/netfilter/nf_conntrack_core.c 2006-11-02 19:15:46.000000000 +0100 +++ linux-2.6.19-rc3-git4.quilt/net/netfilter/nf_conntrack_core.c 2006-11-02 19:15:54.000000000 +0100 @@ -609,12 +609,10 @@ __nf_conntrack_alloc(const struct nf_con /* find features needed by this conntrack. */ features = l3proto->get_features(orig); - /* FIXME: protect helper list per RCU */ - spin_lock_bh(&nf_conntrack_lock); - helper = __nf_ct_helper_find(repl); + rcu_read_lock(); + helper = nf_ct_helper_find(repl); if (helper) features |= NF_CT_F_HELP; - spin_unlock_bh(&nf_conntrack_lock); DEBUGP("nf_conntrack_alloc: features=0x%x\n", features); @@ -658,11 +656,13 @@ __nf_conntrack_alloc(const struct nf_con conntrack->l4proto = l4proto; read_unlock_bh(&nf_ct_cache_lock); + rcu_read_unlock(); return conntrack; out: read_unlock_bh(&nf_ct_cache_lock); atomic_dec(&nf_conntrack_count); + rcu_read_unlock(); return conntrack; } @@ -1018,16 +1018,8 @@ void __nf_conntrack_attach(struct sk_buf nf_conntrack_get(nskb->nfct); } -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.19-rc3-git4.quilt/net/netfilter/xt_helper.c =================================================================== --- linux-2.6.19-rc3-git4.quilt.orig/net/netfilter/xt_helper.c 2006-11-02 19:15:47.000000000 +0100 +++ linux-2.6.19-rc3-git4.quilt/net/netfilter/xt_helper.c 2006-11-02 19:15:54.000000000 +0100 @@ -99,38 +99,44 @@ match(const struct sk_buff *skb, const struct xt_helper_info *info = matchinfo; struct nf_conn *ct; struct nf_conn_help *master_help; + struct nf_conntrack_helper *helper; enum ip_conntrack_info ctinfo; int ret = info->invert; ct = nf_ct_get((struct sk_buff *)skb, &ctinfo); if (!ct) { DEBUGP("xt_helper: Eek! invalid conntrack?\n"); - return ret; + goto out; } if (!ct->master) { DEBUGP("xt_helper: conntrack %p has no master\n", ct); - return ret; + goto out; } - spin_lock_bh(&nf_conntrack_lock); master_help = nfct_help(ct->master); - if (!master_help || !master_help->helper) { + if (!master_help) { DEBUGP("xt_helper: master ct %p has no helper\n", exp->expectant); - goto out_unlock; + goto out; + } + + helper = master_help->helper; + if (!helper) { + DEBUGP("xt_helper: master ct %p has no helper\n", + exp->expectant); + goto out; } DEBUGP("master's name = %s , info->name = %s\n", - ct->master->helper->name, info->name); + helper->name, info->name); if (info->name[0] == '\0') ret ^= 1; else - ret ^= !strncmp(master_help->helper->name, info->name, - strlen(master_help->helper->name)); -out_unlock: - spin_unlock_bh(&nf_conntrack_lock); + ret ^= !strncmp(helper->name, info->name, + strlen(helper->name)); +out: return ret; } #endif Index: linux-2.6.19-rc3-git4.quilt/net/netfilter/nf_conntrack_helper.c =================================================================== --- linux-2.6.19-rc3-git4.quilt.orig/net/netfilter/nf_conntrack_helper.c 2006-11-02 19:14:56.000000000 +0100 +++ linux-2.6.19-rc3-git4.quilt/net/netfilter/nf_conntrack_helper.c 2006-11-02 19:15:54.000000000 +0100 @@ -32,70 +32,38 @@ static struct list_head helpers __read_mostly = { &(helpers), &(helpers) }; +/* 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 *h; - list_for_each_entry(h, &helpers, list) { + list_for_each_entry_rcu(h, &helpers, list) { if (nf_ct_tuple_mask_cmp(tuple, &h->tuple, &h->mask)) return h; } return NULL; } +/* 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; -} - -void nf_ct_helper_put(struct nf_conntrack_helper *helper) -{ - module_put(helper->me); -} - -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; - list_for_each_entry(h, &helpers, list) { + list_for_each_entry_rcu(h, &helpers, list) if (!strcmp(h->name, name)) return h; - } return NULL; } -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); - help->helper = NULL; - } - return 0; + return (help && help->helper == me); } int nf_conntrack_helper_register(struct nf_conntrack_helper *me) @@ -111,8 +79,9 @@ int nf_conntrack_helper_register(struct printk(KERN_ERR "nf_conntrack_helper_reigster: Unable to create slab cache for conntracks\n"); return ret; } + spin_lock_bh(&nf_conntrack_lock); - list_add(&me->list, &helpers); + list_add_rcu(&me->list, &helpers); spin_unlock_bh(&nf_conntrack_lock); return 0; @@ -120,13 +89,19 @@ int nf_conntrack_helper_register(struct void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me) { - unsigned int i; - struct nf_conntrack_tuple_hash *h; + unsigned int bucket = 0; struct nf_conntrack_expect *exp, *tmp; + struct nf_conn *ct; /* Need lock here, to delete helper. */ spin_lock_bh(&nf_conntrack_lock); - list_del(&me->list); + list_del_rcu(&me->list); + 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 */ list_for_each_entry_safe(exp, tmp, &nf_conntrack_expect_list, list) { @@ -137,15 +112,21 @@ void nf_conntrack_helper_unregister(stru } } - /* Get rid of expecteds, set helpers to NULL. */ - list_for_each_entry(h, &unconfirmed, list) - unhelp(h, me); - for (i = 0; i < nf_conntrack_htable_size; i++) { - list_for_each_entry(h, &nf_conntrack_hash[i], list) - unhelp(h, me); - } spin_unlock_bh(&nf_conntrack_lock); + while ((ct = get_next_corpse(helper_match, me, &bucket)) != 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.19-rc3-git4.quilt/include/net/netfilter/nf_conntrack_core.h =================================================================== --- linux-2.6.19-rc3-git4.quilt.orig/include/net/netfilter/nf_conntrack_core.h 2006-11-02 19:15:47.000000000 +0100 +++ linux-2.6.19-rc3-git4.quilt/include/net/netfilter/nf_conntrack_core.h 2006-11-02 19:15:54.000000000 +0100 @@ -73,6 +73,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.19-rc3-git4.quilt/include/net/netfilter/nf_conntrack_helper.h =================================================================== --- linux-2.6.19-rc3-git4.quilt.orig/include/net/netfilter/nf_conntrack_helper.h 2006-11-02 19:14:31.000000000 +0100 +++ linux-2.6.19-rc3-git4.quilt/include/net/netfilter/nf_conntrack_helper.h 2006-11-02 19:15:54.000000000 +0100 @@ -38,15 +38,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.19-rc3-git4.quilt/net/netfilter/nf_conntrack_standalone.c =================================================================== --- linux-2.6.19-rc3-git4.quilt.orig/net/netfilter/nf_conntrack_standalone.c 2006-11-02 19:15:46.000000000 +0100 +++ linux-2.6.19-rc3-git4.quilt/net/netfilter/nf_conntrack_standalone.c 2006-11-02 19:15:54.000000000 +0100 @@ -738,9 +738,8 @@ EXPORT_SYMBOL(nf_conntrack_alloc); EXPORT_SYMBOL(nf_conntrack_free); EXPORT_SYMBOL(nf_conntrack_flush); EXPORT_SYMBOL(nf_ct_remove_expectations); -EXPORT_SYMBOL(nf_ct_helper_find_get); -EXPORT_SYMBOL(nf_ct_helper_put); -EXPORT_SYMBOL(__nf_conntrack_helper_find_byname); +EXPORT_SYMBOL(nf_ct_helper_find); +EXPORT_SYMBOL(nf_conntrack_helper_find_byname); EXPORT_SYMBOL(__nf_conntrack_find); EXPORT_SYMBOL(nf_ct_unlink_expect); EXPORT_SYMBOL(nf_conntrack_hash_insert); Index: linux-2.6.19-rc3-git4.quilt/net/netfilter/nf_conntrack_netlink.c =================================================================== --- linux-2.6.19-rc3-git4.quilt.orig/net/netfilter/nf_conntrack_netlink.c 2006-11-02 19:15:47.000000000 +0100 +++ linux-2.6.19-rc3-git4.quilt/net/netfilter/nf_conntrack_netlink.c 2006-11-02 19:15:54.000000000 +0100 @@ -882,7 +882,7 @@ ctnetlink_change_helper(struct nf_conn * if (err < 0) return err; - helper = __nf_conntrack_helper_find_byname(helpname); + helper = nf_conntrack_helper_find_byname(helpname); if (!helper) { if (!strcmp(helpname, "")) helper = NULL; @@ -1410,7 +1410,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); err = -EINVAL;