Name: Connection Tracking Helper Lock optimization Author: Rusty Russell Status: Trivial Depends: Netfilter/conntrack_proto_lock.patch.gz Depends: Netfilter/alter_reply_fix.patch.gz Depends: Netfilter/conntrack_expect_lock_removal.patch.gz D: This moves the connection tracking helper list out from under the D: ip_conntrack_lock, and uses the BR_NETPROTO_LOCK. diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .20863-2.4.19-conntrack_helper_lock.pre/net/ipv4/netfilter/ip_conntrack_core.c .20863-2.4.19-conntrack_helper_lock/net/ipv4/netfilter/ip_conntrack_core.c --- .20863-2.4.19-conntrack_helper_lock.pre/net/ipv4/netfilter/ip_conntrack_core.c 2002-08-22 14:15:32.000000000 +1000 +++ .20863-2.4.19-conntrack_helper_lock/net/ipv4/netfilter/ip_conntrack_core.c 2002-08-22 14:15:34.000000000 +1000 @@ -26,7 +26,7 @@ /* For ERR_PTR(). Yeah, I know... --RR */ #include -/* This rwlock protects the main hash table, protocol/helper/expected +/* This rwlock protects the main hash table, expected registrations, conntrack timers*/ #define ASSERT_READ_LOCK(x) MUST_BE_READ_LOCKED(&ip_conntrack_lock) #define ASSERT_WRITE_LOCK(x) MUST_BE_WRITE_LOCKED(&ip_conntrack_lock) @@ -47,8 +47,11 @@ DECLARE_RWLOCK(ip_conntrack_lock); void (*ip_conntrack_destroyed)(struct ip_conntrack *conntrack) = NULL; LIST_HEAD(expect_list); + +/* These two are protected by BR_NETPROTO_LOCK */ LIST_HEAD(protocol_list); static LIST_HEAD(helpers); + unsigned int ip_conntrack_htable_size = 0; static int ip_conntrack_max = 0; static atomic_t ip_conntrack_count = ATOMIC_INIT(0); @@ -449,12 +452,6 @@ static int early_drop(struct list_head * return dropped; } -static inline int helper_cmp(const struct ip_conntrack_helper *i, - const struct ip_conntrack_tuple *rtuple) -{ - return ip_ct_tuple_mask_cmp(rtuple, &i->tuple, &i->mask); -} - /* Compare parts depending on mask. */ static inline int expect_cmp(const struct ip_conntrack_expect *i, const struct ip_conntrack_tuple *tuple) @@ -470,6 +467,7 @@ init_conntrack(const struct ip_conntrack struct sk_buff *skb) { struct ip_conntrack *conntrack; + struct ip_conntrack_helper *helper; struct ip_conntrack_tuple repl_tuple; size_t hash, repl_hash; struct ip_conntrack_expect *expected; @@ -529,12 +527,19 @@ init_conntrack(const struct ip_conntrack /* Mark clearly that it's not in the hash table. */ conntrack->tuplehash[IP_CT_DIR_ORIGINAL].list.next = NULL; + /* We hold BR_NETPROTO_LOCK */ + list_for_each_entry(helper, &helpers, list) { + if (ip_ct_tuple_mask_cmp(&repl_tuple, + &helper->tuple, + &helper->mask)) { + conntrack->helper = helper; + break; + } + } + /* Write lock required for deletion of expected. Without this, a read-lock would do. */ WRITE_LOCK(&ip_conntrack_lock); - conntrack->helper = LIST_FIND(&helpers, helper_cmp, - struct ip_conntrack_helper *, - &repl_tuple); /* Need finding and deleting of expected ONLY if we win race */ expected = LIST_FIND(&expect_list, expect_cmp, struct ip_conntrack_expect *, tuple); @@ -764,7 +769,8 @@ void ip_conntrack_unexpect_related(struc void ip_conntrack_alter_reply(struct ip_conntrack *conntrack, const struct ip_conntrack_tuple *newreply) { - WRITE_LOCK(&ip_conntrack_lock); + struct ip_conntrack_helper *helper; + /* Should be unconfirmed, so not in hash table yet */ IP_NF_ASSERT(!is_confirmed(conntrack)); @@ -772,37 +778,39 @@ void ip_conntrack_alter_reply(struct ip_ DUMP_TUPLE(newreply); conntrack->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply; - conntrack->helper = LIST_FIND(&helpers, helper_cmp, - struct ip_conntrack_helper *, - newreply); - WRITE_UNLOCK(&ip_conntrack_lock); + + /* Find new helper: we hold BR_NETPROTO_LOCK */ + conntrack->helper = NULL; + list_for_each_entry(helper, &helpers, list) { + if (ip_ct_tuple_mask_cmp(newreply, + &helper->tuple, + &helper->mask)) { + conntrack->helper = helper; + break; + } + } } int ip_conntrack_helper_register(struct ip_conntrack_helper *me) { MOD_INC_USE_COUNT; - WRITE_LOCK(&ip_conntrack_lock); - list_prepend(&helpers, me); - WRITE_UNLOCK(&ip_conntrack_lock); + br_write_lock_bh(BR_NETPROTO_LOCK); + list_add(&me->list, &helpers); + br_write_unlock_bh(BR_NETPROTO_LOCK); return 0; } -static inline int unhelp(struct ip_conntrack_tuple_hash *i, - const struct ip_conntrack_helper *me) +static inline void unhelp(struct ip_conntrack_tuple_hash *i) { - if (i->ctrack->helper == me) { - i->ctrack->helper = NULL; - /* Get rid of any expected. */ - if (i->ctrack->expected.expectant) { - IP_NF_ASSERT(i->ctrack->expected.expectant - == i->ctrack); - LIST_DELETE(&expect_list, &i->ctrack->expected); - i->ctrack->expected.expectant = NULL; - } + i->ctrack->helper = NULL; + /* Get rid of any expected. */ + if (i->ctrack->expected.expectant) { + IP_NF_ASSERT(i->ctrack->expected.expectant == i->ctrack); + list_del(&i->ctrack->expected.list); + i->ctrack->expected.expectant = NULL; } - return 0; } void ip_conntrack_helper_unregister(struct ip_conntrack_helper *me) @@ -810,18 +818,21 @@ void ip_conntrack_helper_unregister(stru unsigned int i; /* Need write lock here, to delete helper. */ - WRITE_LOCK(&ip_conntrack_lock); - LIST_DELETE(&helpers, me); + br_write_lock_bh(BR_NETPROTO_LOCK); + list_del(&me->list); + br_write_unlock_bh(BR_NETPROTO_LOCK); /* Get rid of expecteds, set helpers to NULL. */ - for (i = 0; i < ip_conntrack_htable_size; i++) - LIST_FIND_W(&ip_conntrack_hash[i], unhelp, - struct ip_conntrack_tuple_hash *, me); - WRITE_UNLOCK(&ip_conntrack_lock); + for (i = 0; i < ip_conntrack_htable_size; i++) { + struct ip_conntrack_tuple_hash *h; - /* Someone could be still looking at the helper in a bh. */ - br_write_lock_bh(BR_NETPROTO_LOCK); - br_write_unlock_bh(BR_NETPROTO_LOCK); + spin_lock_bh(&ip_conntrack_hash[i].lock); + list_for_each_entry(h, &ip_conntrack_hash[i].list, list) { + if (h->ctrack->helper == me) + unhelp(h); + } + spin_unlock_bh(&ip_conntrack_hash[i].lock); + } MOD_DEC_USE_COUNT; }