From 47cbb071627549d604d4d4d4e915592d98620963 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Fri, 9 Apr 2010 12:25:30 +0200 Subject: [PATCH 1/2] xt_condition: remove unnecessary RCU protection The module does not use the RCU mechanism, so calling list_add_rcu/list_del_rcu does not make much sense either. --- doc/changelog.txt | 1 + extensions/xt_condition.c | 18 +++--------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/doc/changelog.txt b/doc/changelog.txt index 3e7f103..a2b0741 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -9,6 +9,7 @@ HEAD - TEE: do not retain iif and mark on cloned packet - TEE: new loop detection logic - TEE: use less expensive pskb_copy +- condition: remove unnecessary RCU protection Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_condition.c b/extensions/xt_condition.c index f5fb98d..ce2a365 100644 --- a/extensions/xt_condition.c +++ b/extensions/xt_condition.c @@ -100,13 +100,8 @@ condition_mt(const struct sk_buff *skb, const struct xt_match_param *par) { const struct xt_condition_mtinfo *info = par->matchinfo; const struct condition_variable *var = info->condvar; - bool x; - rcu_read_lock(); - x = rcu_dereference(var->enabled); - rcu_read_unlock(); - - return x ^ info->invert; + return var->enabled ^ info->invert; } static int condition_mt_check(const struct xt_mtchk_param *par) @@ -164,7 +159,7 @@ static int condition_mt_check(const struct xt_mtchk_param *par) wmb(); var->status_proc->read_proc = condition_proc_read; var->status_proc->write_proc = condition_proc_write; - list_add_rcu(&var->list, &conditions_list); + list_add(&var->list, &conditions_list); var->status_proc->uid = condition_uid_perms; var->status_proc->gid = condition_gid_perms; mutex_unlock(&proc_lock); @@ -179,16 +174,9 @@ static void condition_mt_destroy(const struct xt_mtdtor_param *par) mutex_lock(&proc_lock); if (--var->refcount == 0) { - list_del_rcu(&var->list); + list_del(&var->list); remove_proc_entry(var->status_proc->name, proc_net_condition); mutex_unlock(&proc_lock); - /* - * synchronize_rcu() would be good enough, but - * synchronize_net() guarantees that no packet - * will go out with the old rule after - * succesful removal. - */ - synchronize_net(); kfree(var); return; } From c6f8f72bf1ad71a3cb66fc165826b14fc895d396 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Fri, 9 Apr 2010 12:28:50 +0200 Subject: [PATCH 2/2] xt_condition: use non-interruptible check routine Patrick McHardy let's it be known: "No need for interruptible locking, the section is very short and usually there's only a single iptables process running at a time." --- extensions/xt_condition.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/extensions/xt_condition.c b/extensions/xt_condition.c index ce2a365..c4a54b9 100644 --- a/extensions/xt_condition.c +++ b/extensions/xt_condition.c @@ -56,7 +56,7 @@ struct condition_variable { /* proc_lock is a user context only semaphore used for write access */ /* to the conditions' list. */ -static struct mutex proc_lock; +static DEFINE_MUTEX(proc_lock); static LIST_HEAD(conditions_list); static struct proc_dir_entry *proc_net_condition; @@ -122,9 +122,7 @@ static int condition_mt_check(const struct xt_mtchk_param *par) * Let's acquire the lock, check for the condition and add it * or increase the reference counter. */ - if (mutex_lock_interruptible(&proc_lock) != 0) - return -EINTR; - + mutex_lock(&proc_lock); list_for_each_entry(var, &conditions_list, list) { if (strcmp(info->name, var->status_proc->name) == 0) { var->refcount++;