* [PATCH 1/6] netfilter: xtables: replace XT_ENTRY_ITERATE macro
2010-02-12 10:20 ip_tables: foreachs and reentrancy Jan Engelhardt
@ 2010-02-12 10:20 ` Jan Engelhardt
2010-02-13 10:29 ` Amos Jeffries
2010-02-12 10:20 ` [PATCH 2/6] netfilter: xtables: optimize call flow around xt_entry_foreach Jan Engelhardt
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-12 10:20 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel
The macro is replaced by a list.h-like foreach loop. This makes
the code much more inspectable.
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
include/linux/netfilter/x_tables.h | 9 ++
include/linux/netfilter_arp/arp_tables.h | 10 +--
include/linux/netfilter_ipv4/ip_tables.h | 11 +--
include/linux/netfilter_ipv6/ip6_tables.h | 10 +--
net/ipv4/netfilter/arp_tables.c | 151 ++++++++++++++++++----------
net/ipv4/netfilter/ip_tables.c | 160 +++++++++++++++++++----------
net/ipv6/netfilter/ip6_tables.c | 160 +++++++++++++++++++----------
7 files changed, 321 insertions(+), 190 deletions(-)
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 3d39e6e..6a25875 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -140,6 +140,7 @@ struct xt_counters_info {
__ret; \
})
+#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define XT_ENTRY_ITERATE_CONTINUE(type, entries, size, n, fn, args...) \
({ \
@@ -164,6 +165,14 @@ struct xt_counters_info {
#define XT_ENTRY_ITERATE(type, entries, size, fn, args...) \
XT_ENTRY_ITERATE_CONTINUE(type, entries, size, 0, fn, args)
+#endif /* !__KERNEL__ */
+
+/* pos is normally a struct ipt_entry/ip6t_entry/etc. */
+#define xt_entry_foreach(pos, ehead, esize) \
+ for ((pos) = (typeof(pos))(ehead); \
+ (pos) < (typeof(pos))((char *)(ehead) + (esize)); \
+ (pos) = (typeof(pos))((char *)(pos) + (pos)->next_offset))
+
#ifdef __KERNEL__
#include <linux/netdevice.h>
diff --git a/include/linux/netfilter_arp/arp_tables.h b/include/linux/netfilter_arp/arp_tables.h
index 0b33980..e9948c0 100644
--- a/include/linux/netfilter_arp/arp_tables.h
+++ b/include/linux/netfilter_arp/arp_tables.h
@@ -211,9 +211,11 @@ static __inline__ struct arpt_entry_target *arpt_get_target(struct arpt_entry *e
return (void *)e + e->target_offset;
}
+#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define ARPT_ENTRY_ITERATE(entries, size, fn, args...) \
XT_ENTRY_ITERATE(struct arpt_entry, entries, size, fn, ## args)
+#endif
/*
* Main firewall chains definitions and global var's definitions.
@@ -291,14 +293,6 @@ compat_arpt_get_target(struct compat_arpt_entry *e)
#define COMPAT_ARPT_ALIGN(s) COMPAT_XT_ALIGN(s)
-/* fn returns 0 to continue iteration */
-#define COMPAT_ARPT_ENTRY_ITERATE(entries, size, fn, args...) \
- XT_ENTRY_ITERATE(struct compat_arpt_entry, entries, size, fn, ## args)
-
-#define COMPAT_ARPT_ENTRY_ITERATE_CONTINUE(entries, size, n, fn, args...) \
- XT_ENTRY_ITERATE_CONTINUE(struct compat_arpt_entry, entries, size, n, \
- fn, ## args)
-
#endif /* CONFIG_COMPAT */
#endif /*__KERNEL__*/
#endif /* _ARPTABLES_H */
diff --git a/include/linux/netfilter_ipv4/ip_tables.h b/include/linux/netfilter_ipv4/ip_tables.h
index 364973b..5b20ae7 100644
--- a/include/linux/netfilter_ipv4/ip_tables.h
+++ b/include/linux/netfilter_ipv4/ip_tables.h
@@ -227,9 +227,11 @@ ipt_get_target(struct ipt_entry *e)
#define IPT_MATCH_ITERATE(e, fn, args...) \
XT_MATCH_ITERATE(struct ipt_entry, e, fn, ## args)
+#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define IPT_ENTRY_ITERATE(entries, size, fn, args...) \
XT_ENTRY_ITERATE(struct ipt_entry, entries, size, fn, ## args)
+#endif
/*
* Main firewall chains definitions and global var's definitions.
@@ -317,15 +319,6 @@ compat_ipt_get_target(struct compat_ipt_entry *e)
#define COMPAT_IPT_MATCH_ITERATE(e, fn, args...) \
XT_MATCH_ITERATE(struct compat_ipt_entry, e, fn, ## args)
-/* fn returns 0 to continue iteration */
-#define COMPAT_IPT_ENTRY_ITERATE(entries, size, fn, args...) \
- XT_ENTRY_ITERATE(struct compat_ipt_entry, entries, size, fn, ## args)
-
-/* fn returns 0 to continue iteration */
-#define COMPAT_IPT_ENTRY_ITERATE_CONTINUE(entries, size, n, fn, args...) \
- XT_ENTRY_ITERATE_CONTINUE(struct compat_ipt_entry, entries, size, n, \
- fn, ## args)
-
#endif /* CONFIG_COMPAT */
#endif /*__KERNEL__*/
#endif /* _IPTABLES_H */
diff --git a/include/linux/netfilter_ipv6/ip6_tables.h b/include/linux/netfilter_ipv6/ip6_tables.h
index 8031eb4..8bb3f5b 100644
--- a/include/linux/netfilter_ipv6/ip6_tables.h
+++ b/include/linux/netfilter_ipv6/ip6_tables.h
@@ -284,9 +284,11 @@ ip6t_get_target(struct ip6t_entry *e)
#define IP6T_MATCH_ITERATE(e, fn, args...) \
XT_MATCH_ITERATE(struct ip6t_entry, e, fn, ## args)
+#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define IP6T_ENTRY_ITERATE(entries, size, fn, args...) \
XT_ENTRY_ITERATE(struct ip6t_entry, entries, size, fn, ## args)
+#endif
/*
* Main firewall chains definitions and global var's definitions.
@@ -345,14 +347,6 @@ compat_ip6t_get_target(struct compat_ip6t_entry *e)
#define COMPAT_IP6T_MATCH_ITERATE(e, fn, args...) \
XT_MATCH_ITERATE(struct compat_ip6t_entry, e, fn, ## args)
-/* fn returns 0 to continue iteration */
-#define COMPAT_IP6T_ENTRY_ITERATE(entries, size, fn, args...) \
- XT_ENTRY_ITERATE(struct compat_ip6t_entry, entries, size, fn, ## args)
-
-#define COMPAT_IP6T_ENTRY_ITERATE_CONTINUE(entries, size, n, fn, args...) \
- XT_ENTRY_ITERATE_CONTINUE(struct compat_ip6t_entry, entries, size, n, \
- fn, ## args)
-
#endif /* CONFIG_COMPAT */
#endif /*__KERNEL__*/
#endif /* _IP6_TABLES_H */
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 4db5c1e..f733886 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -641,8 +641,9 @@ static int translate_table(const char *name,
const unsigned int *hook_entries,
const unsigned int *underflows)
{
+ struct arpt_entry *iter;
unsigned int i;
- int ret;
+ int ret = 0;
newinfo->size = size;
newinfo->number = number;
@@ -657,12 +658,13 @@ static int translate_table(const char *name,
i = 0;
/* Walk through entries, checking offsets. */
- ret = ARPT_ENTRY_ITERATE(entry0, newinfo->size,
- check_entry_size_and_hooks,
- newinfo,
- entry0,
- entry0 + size,
- hook_entries, underflows, valid_hooks, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ ret = check_entry_size_and_hooks(iter, newinfo, entry0,
+ entry0 + size, hook_entries, underflows,
+ valid_hooks, &i);
+ if (ret != 0)
+ break;
+ }
duprintf("translate_table: ARPT_ENTRY_ITERATE gives %d\n", ret);
if (ret != 0)
return ret;
@@ -697,12 +699,16 @@ static int translate_table(const char *name,
/* Finally, each sanity check must pass */
i = 0;
- ret = ARPT_ENTRY_ITERATE(entry0, newinfo->size,
- find_check_entry, name, size, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ ret = find_check_entry(iter, name, size, &i);
+ if (ret != 0)
+ break;
+ }
if (ret != 0) {
- ARPT_ENTRY_ITERATE(entry0, newinfo->size,
- cleanup_entry, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size)
+ if (cleanup_entry(iter, &i) != 0)
+ break;
return ret;
}
@@ -739,6 +745,7 @@ static inline int set_entry_to_counter(const struct arpt_entry *e,
static void get_counters(const struct xt_table_info *t,
struct xt_counters counters[])
{
+ struct arpt_entry *iter;
unsigned int cpu;
unsigned int i;
unsigned int curcpu;
@@ -754,22 +761,18 @@ static void get_counters(const struct xt_table_info *t,
curcpu = smp_processor_id();
i = 0;
- ARPT_ENTRY_ITERATE(t->entries[curcpu],
- t->size,
- set_entry_to_counter,
- counters,
- &i);
+ xt_entry_foreach(iter, t->entries[curcpu], t->size)
+ if (set_entry_to_counter(iter, counters, &i) != 0)
+ break;
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
i = 0;
xt_info_wrlock(cpu);
- ARPT_ENTRY_ITERATE(t->entries[cpu],
- t->size,
- add_entry_to_counter,
- counters,
- &i);
+ xt_entry_foreach(iter, t->entries[cpu], t->size)
+ if (add_entry_to_counter(iter, counters, &i) != 0)
+ break;
xt_info_wrunlock(cpu);
}
local_bh_enable();
@@ -899,7 +902,9 @@ static int compat_calc_entry(const struct arpt_entry *e,
static int compat_table_info(const struct xt_table_info *info,
struct xt_table_info *newinfo)
{
+ struct arpt_entry *iter;
void *loc_cpu_entry;
+ int ret = 0;
if (!newinfo || !info)
return -EINVAL;
@@ -908,9 +913,12 @@ static int compat_table_info(const struct xt_table_info *info,
memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
newinfo->initial_entries = 0;
loc_cpu_entry = info->entries[raw_smp_processor_id()];
- return ARPT_ENTRY_ITERATE(loc_cpu_entry, info->size,
- compat_calc_entry, info, loc_cpu_entry,
- newinfo);
+ xt_entry_foreach(iter, loc_cpu_entry, info->size) {
+ ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
+ if (ret != 0)
+ break;
+ }
+ return ret;
}
#endif
@@ -1025,6 +1033,7 @@ static int __do_replace(struct net *net, const char *name,
struct xt_table_info *oldinfo;
struct xt_counters *counters;
void *loc_cpu_old_entry;
+ struct arpt_entry *iter;
ret = 0;
counters = vmalloc_node(num_counters * sizeof(struct xt_counters),
@@ -1068,8 +1077,9 @@ static int __do_replace(struct net *net, const char *name,
/* Decrease module usage counts and free resource */
loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
- ARPT_ENTRY_ITERATE(loc_cpu_old_entry, oldinfo->size, cleanup_entry,
- NULL);
+ xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
+ if (cleanup_entry(iter, NULL) != 0)
+ break;
xt_free_table_info(oldinfo);
if (copy_to_user(counters_ptr, counters,
@@ -1095,6 +1105,7 @@ static int do_replace(struct net *net, const void __user *user,
struct arpt_replace tmp;
struct xt_table_info *newinfo;
void *loc_cpu_entry;
+ struct arpt_entry *iter;
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
return -EFAULT;
@@ -1130,7 +1141,9 @@ static int do_replace(struct net *net, const void __user *user,
return 0;
free_newinfo_untrans:
- ARPT_ENTRY_ITERATE(loc_cpu_entry, newinfo->size, cleanup_entry, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
+ if (cleanup_entry(iter, NULL) != 0)
+ break;
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1163,6 +1176,7 @@ static int do_add_counters(struct net *net, const void __user *user,
const struct xt_table_info *private;
int ret = 0;
void *loc_cpu_entry;
+ struct arpt_entry *iter;
#ifdef CONFIG_COMPAT
struct compat_xt_counters_info compat_tmp;
@@ -1220,11 +1234,9 @@ static int do_add_counters(struct net *net, const void __user *user,
curcpu = smp_processor_id();
loc_cpu_entry = private->entries[curcpu];
xt_info_wrlock(curcpu);
- ARPT_ENTRY_ITERATE(loc_cpu_entry,
- private->size,
- add_counter_to_entry,
- paddc,
- &i);
+ xt_entry_foreach(iter, loc_cpu_entry, private->size)
+ if (add_counter_to_entry(iter, paddc, &i) != 0)
+ break;
xt_info_wrunlock(curcpu);
unlock_up_free:
local_bh_enable();
@@ -1388,8 +1400,10 @@ static int translate_compat_table(const char *name,
unsigned int i, j;
struct xt_table_info *newinfo, *info;
void *pos, *entry0, *entry1;
+ struct compat_arpt_entry *iter0;
+ struct arpt_entry *iter1;
unsigned int size;
- int ret;
+ int ret = 0;
info = *pinfo;
entry0 = *pentry0;
@@ -1406,11 +1420,13 @@ static int translate_compat_table(const char *name,
j = 0;
xt_compat_lock(NFPROTO_ARP);
/* Walk through entries, checking offsets. */
- ret = COMPAT_ARPT_ENTRY_ITERATE(entry0, total_size,
- check_compat_entry_size_and_hooks,
- info, &size, entry0,
- entry0 + total_size,
- hook_entries, underflows, &j, name);
+ xt_entry_foreach(iter0, entry0, total_size) {
+ ret = check_compat_entry_size_and_hooks(iter0, info, &size,
+ entry0, entry0 + total_size, hook_entries, underflows,
+ &j, name);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
goto out_unlock;
@@ -1451,9 +1467,12 @@ static int translate_compat_table(const char *name,
entry1 = newinfo->entries[raw_smp_processor_id()];
pos = entry1;
size = total_size;
- ret = COMPAT_ARPT_ENTRY_ITERATE(entry0, total_size,
- compat_copy_entry_from_user,
- &pos, &size, name, newinfo, entry1);
+ xt_entry_foreach(iter0, entry0, total_size) {
+ ret = compat_copy_entry_from_user(iter0, &pos,
+ &size, name, newinfo, entry1);
+ if (ret != 0)
+ break;
+ }
xt_compat_flush_offsets(NFPROTO_ARP);
xt_compat_unlock(NFPROTO_ARP);
if (ret)
@@ -1464,13 +1483,28 @@ static int translate_compat_table(const char *name,
goto free_newinfo;
i = 0;
- ret = ARPT_ENTRY_ITERATE(entry1, newinfo->size, compat_check_entry,
- name, &i);
+ xt_entry_foreach(iter1, entry1, newinfo->size) {
+ ret = compat_check_entry(iter1, name, &i);
+ if (ret != 0)
+ break;
+ }
if (ret) {
+ /*
+ * The first i matches need cleanup_entry (calls ->destroy)
+ * because they had called ->check already. The other j-i
+ * entries need only release.
+ */
+ int skip = i;
j -= i;
- COMPAT_ARPT_ENTRY_ITERATE_CONTINUE(entry0, newinfo->size, i,
- compat_release_entry, &j);
- ARPT_ENTRY_ITERATE(entry1, newinfo->size, cleanup_entry, &i);
+ xt_entry_foreach(iter0, entry0, newinfo->size) {
+ if (skip-- > 0)
+ continue;
+ if (compat_release_entry(iter0, &j) != 0)
+ break;
+ }
+ xt_entry_foreach(iter1, entry1, newinfo->size)
+ if (cleanup_entry(iter1, &i) != 0)
+ break;
xt_free_table_info(newinfo);
return ret;
}
@@ -1488,7 +1522,9 @@ static int translate_compat_table(const char *name,
free_newinfo:
xt_free_table_info(newinfo);
out:
- COMPAT_ARPT_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
+ xt_entry_foreach(iter0, entry0, total_size)
+ if (compat_release_entry(iter0, &j) != 0)
+ break;
return ret;
out_unlock:
xt_compat_flush_offsets(NFPROTO_ARP);
@@ -1515,6 +1551,7 @@ static int compat_do_replace(struct net *net, void __user *user,
struct compat_arpt_replace tmp;
struct xt_table_info *newinfo;
void *loc_cpu_entry;
+ struct arpt_entry *iter;
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
return -EFAULT;
@@ -1552,7 +1589,9 @@ static int compat_do_replace(struct net *net, void __user *user,
return 0;
free_newinfo_untrans:
- ARPT_ENTRY_ITERATE(loc_cpu_entry, newinfo->size, cleanup_entry, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
+ if (cleanup_entry(iter, NULL) != 0)
+ break;
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1636,6 +1675,7 @@ static int compat_copy_entries_to_user(unsigned int total_size,
int ret = 0;
void *loc_cpu_entry;
unsigned int i = 0;
+ struct arpt_entry *iter;
counters = alloc_counters(table);
if (IS_ERR(counters))
@@ -1645,9 +1685,12 @@ static int compat_copy_entries_to_user(unsigned int total_size,
loc_cpu_entry = private->entries[raw_smp_processor_id()];
pos = userptr;
size = total_size;
- ret = ARPT_ENTRY_ITERATE(loc_cpu_entry, total_size,
- compat_copy_entry_to_user,
- &pos, &size, counters, &i);
+ xt_entry_foreach(iter, loc_cpu_entry, total_size) {
+ ret = compat_copy_entry_to_user(iter, &pos,
+ &size, counters, &i);
+ if (ret != 0)
+ break;
+ }
vfree(counters);
return ret;
}
@@ -1843,13 +1886,15 @@ void arpt_unregister_table(struct xt_table *table)
struct xt_table_info *private;
void *loc_cpu_entry;
struct module *table_owner = table->me;
+ struct arpt_entry *iter;
private = xt_unregister_table(table);
/* Decrease module usage counts and free resources */
loc_cpu_entry = private->entries[raw_smp_processor_id()];
- ARPT_ENTRY_ITERATE(loc_cpu_entry, private->size,
- cleanup_entry, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, private->size)
+ if (cleanup_entry(iter, NULL) != 0)
+ break;
if (private->number > private->initial_entries)
module_put(table_owner);
xt_free_table_info(private);
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e94c18b..b43280a 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -288,6 +288,7 @@ static void trace_packet(const struct sk_buff *skb,
const void *table_base;
const struct ipt_entry *root;
const char *hookname, *chainname, *comment;
+ const struct ipt_entry *iter;
unsigned int rulenum = 0;
table_base = private->entries[smp_processor_id()];
@@ -296,10 +297,10 @@ static void trace_packet(const struct sk_buff *skb,
hookname = chainname = hooknames[hook];
comment = comments[NF_IP_TRACE_COMMENT_RULE];
- IPT_ENTRY_ITERATE(root,
- private->size - private->hook_entry[hook],
- get_chainname_rulenum,
- e, hookname, &chainname, &comment, &rulenum);
+ xt_entry_foreach(iter, root, private->size - private->hook_entry[hook])
+ if (get_chainname_rulenum(iter, e, hookname,
+ &chainname, &comment, &rulenum) != 0)
+ break;
nf_log_packet(AF_INET, hook, skb, in, out, &trace_loginfo,
"TRACE: %s:%s:%s:%u ",
@@ -826,8 +827,9 @@ translate_table(struct net *net,
const unsigned int *hook_entries,
const unsigned int *underflows)
{
+ struct ipt_entry *iter;
unsigned int i;
- int ret;
+ int ret = 0;
newinfo->size = size;
newinfo->number = number;
@@ -841,12 +843,13 @@ translate_table(struct net *net,
duprintf("translate_table: size %u\n", newinfo->size);
i = 0;
/* Walk through entries, checking offsets. */
- ret = IPT_ENTRY_ITERATE(entry0, newinfo->size,
- check_entry_size_and_hooks,
- newinfo,
- entry0,
- entry0 + size,
- hook_entries, underflows, valid_hooks, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ ret = check_entry_size_and_hooks(iter, newinfo, entry0,
+ entry0 + size, hook_entries, underflows,
+ valid_hooks, &i);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
return ret;
@@ -878,12 +881,16 @@ translate_table(struct net *net,
/* Finally, each sanity check must pass */
i = 0;
- ret = IPT_ENTRY_ITERATE(entry0, newinfo->size,
- find_check_entry, net, name, size, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ ret = find_check_entry(iter, net, name, size, &i);
+ if (ret != 0)
+ break;
+ }
if (ret != 0) {
- IPT_ENTRY_ITERATE(entry0, newinfo->size,
- cleanup_entry, net, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size)
+ if (cleanup_entry(iter, net, &i) != 0)
+ break;
return ret;
}
@@ -923,6 +930,7 @@ static void
get_counters(const struct xt_table_info *t,
struct xt_counters counters[])
{
+ struct ipt_entry *iter;
unsigned int cpu;
unsigned int i;
unsigned int curcpu;
@@ -938,22 +946,18 @@ get_counters(const struct xt_table_info *t,
curcpu = smp_processor_id();
i = 0;
- IPT_ENTRY_ITERATE(t->entries[curcpu],
- t->size,
- set_entry_to_counter,
- counters,
- &i);
+ xt_entry_foreach(iter, t->entries[curcpu], t->size)
+ if (set_entry_to_counter(iter, counters, &i) != 0)
+ break;
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
i = 0;
xt_info_wrlock(cpu);
- IPT_ENTRY_ITERATE(t->entries[cpu],
- t->size,
- add_entry_to_counter,
- counters,
- &i);
+ xt_entry_foreach(iter, t->entries[cpu], t->size)
+ if (add_entry_to_counter(iter, counters, &i) != 0)
+ break;
xt_info_wrunlock(cpu);
}
local_bh_enable();
@@ -1111,7 +1115,9 @@ static int compat_calc_entry(const struct ipt_entry *e,
static int compat_table_info(const struct xt_table_info *info,
struct xt_table_info *newinfo)
{
+ struct ipt_entry *iter;
void *loc_cpu_entry;
+ int ret = 0;
if (!newinfo || !info)
return -EINVAL;
@@ -1120,9 +1126,12 @@ static int compat_table_info(const struct xt_table_info *info,
memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
newinfo->initial_entries = 0;
loc_cpu_entry = info->entries[raw_smp_processor_id()];
- return IPT_ENTRY_ITERATE(loc_cpu_entry, info->size,
- compat_calc_entry, info, loc_cpu_entry,
- newinfo);
+ xt_entry_foreach(iter, loc_cpu_entry, info->size) {
+ ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
+ if (ret != 0)
+ break;
+ }
+ return ret;
}
#endif
@@ -1236,6 +1245,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
struct xt_table_info *oldinfo;
struct xt_counters *counters;
void *loc_cpu_old_entry;
+ struct ipt_entry *iter;
ret = 0;
counters = vmalloc(num_counters * sizeof(struct xt_counters));
@@ -1278,8 +1288,10 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
/* Decrease module usage counts and free resource */
loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
- IPT_ENTRY_ITERATE(loc_cpu_old_entry, oldinfo->size, cleanup_entry,
- net, NULL);
+ xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
+
xt_free_table_info(oldinfo);
if (copy_to_user(counters_ptr, counters,
sizeof(struct xt_counters) * num_counters) != 0)
@@ -1304,6 +1316,7 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
struct ipt_replace tmp;
struct xt_table_info *newinfo;
void *loc_cpu_entry;
+ struct ipt_entry *iter;
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
return -EFAULT;
@@ -1339,7 +1352,9 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
return 0;
free_newinfo_untrans:
- IPT_ENTRY_ITERATE(loc_cpu_entry, newinfo->size, cleanup_entry, net, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1373,6 +1388,7 @@ do_add_counters(struct net *net, const void __user *user,
const struct xt_table_info *private;
int ret = 0;
void *loc_cpu_entry;
+ struct ipt_entry *iter;
#ifdef CONFIG_COMPAT
struct compat_xt_counters_info compat_tmp;
@@ -1430,11 +1446,9 @@ do_add_counters(struct net *net, const void __user *user,
curcpu = smp_processor_id();
loc_cpu_entry = private->entries[curcpu];
xt_info_wrlock(curcpu);
- IPT_ENTRY_ITERATE(loc_cpu_entry,
- private->size,
- add_counter_to_entry,
- paddc,
- &i);
+ xt_entry_foreach(iter, loc_cpu_entry, private->size)
+ if (add_counter_to_entry(iter, paddc, &i) != 0)
+ break;
xt_info_wrunlock(curcpu);
unlock_up_free:
local_bh_enable();
@@ -1720,8 +1734,10 @@ translate_compat_table(struct net *net,
unsigned int i, j;
struct xt_table_info *newinfo, *info;
void *pos, *entry0, *entry1;
+ struct compat_ipt_entry *iter0;
+ struct ipt_entry *iter1;
unsigned int size;
- int ret;
+ int ret = 0;
info = *pinfo;
entry0 = *pentry0;
@@ -1738,11 +1754,13 @@ translate_compat_table(struct net *net,
j = 0;
xt_compat_lock(AF_INET);
/* Walk through entries, checking offsets. */
- ret = COMPAT_IPT_ENTRY_ITERATE(entry0, total_size,
- check_compat_entry_size_and_hooks,
- info, &size, entry0,
- entry0 + total_size,
- hook_entries, underflows, &j, name);
+ xt_entry_foreach(iter0, entry0, total_size) {
+ ret = check_compat_entry_size_and_hooks(iter0, info, &size,
+ entry0, entry0 + total_size, hook_entries, underflows,
+ &j, name);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
goto out_unlock;
@@ -1783,9 +1801,12 @@ translate_compat_table(struct net *net,
entry1 = newinfo->entries[raw_smp_processor_id()];
pos = entry1;
size = total_size;
- ret = COMPAT_IPT_ENTRY_ITERATE(entry0, total_size,
- compat_copy_entry_from_user,
- &pos, &size, name, newinfo, entry1);
+ xt_entry_foreach(iter0, entry0, total_size) {
+ ret = compat_copy_entry_from_user(iter0, &pos,
+ &size, name, newinfo, entry1);
+ if (ret != 0)
+ break;
+ }
xt_compat_flush_offsets(AF_INET);
xt_compat_unlock(AF_INET);
if (ret)
@@ -1796,13 +1817,28 @@ translate_compat_table(struct net *net,
goto free_newinfo;
i = 0;
- ret = IPT_ENTRY_ITERATE(entry1, newinfo->size, compat_check_entry,
- net, name, &i);
+ xt_entry_foreach(iter1, entry1, newinfo->size) {
+ ret = compat_check_entry(iter1, net, name, &i);
+ if (ret != 0)
+ break;
+ }
if (ret) {
+ /*
+ * The first i matches need cleanup_entry (calls ->destroy)
+ * because they had called ->check already. The other j-i
+ * entries need only release.
+ */
+ int skip = i;
j -= i;
- COMPAT_IPT_ENTRY_ITERATE_CONTINUE(entry0, newinfo->size, i,
- compat_release_entry, &j);
- IPT_ENTRY_ITERATE(entry1, newinfo->size, cleanup_entry, net, &i);
+ xt_entry_foreach(iter0, entry0, newinfo->size) {
+ if (skip-- > 0)
+ continue;
+ if (compat_release_entry(iter0, &i) != 0)
+ break;
+ }
+ xt_entry_foreach(iter1, entry1, newinfo->size)
+ if (cleanup_entry(iter1, net, &i) != 0)
+ break;
xt_free_table_info(newinfo);
return ret;
}
@@ -1820,7 +1856,9 @@ translate_compat_table(struct net *net,
free_newinfo:
xt_free_table_info(newinfo);
out:
- COMPAT_IPT_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
+ xt_entry_foreach(iter0, entry0, total_size)
+ if (compat_release_entry(iter0, &j) != 0)
+ break;
return ret;
out_unlock:
xt_compat_flush_offsets(AF_INET);
@@ -1835,6 +1873,7 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
struct compat_ipt_replace tmp;
struct xt_table_info *newinfo;
void *loc_cpu_entry;
+ struct ipt_entry *iter;
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
return -EFAULT;
@@ -1873,7 +1912,9 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
return 0;
free_newinfo_untrans:
- IPT_ENTRY_ITERATE(loc_cpu_entry, newinfo->size, cleanup_entry, net, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1922,6 +1963,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
int ret = 0;
const void *loc_cpu_entry;
unsigned int i = 0;
+ struct ipt_entry *iter;
counters = alloc_counters(table);
if (IS_ERR(counters))
@@ -1934,9 +1976,12 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
loc_cpu_entry = private->entries[raw_smp_processor_id()];
pos = userptr;
size = total_size;
- ret = IPT_ENTRY_ITERATE(loc_cpu_entry, total_size,
- compat_copy_entry_to_user,
- &pos, &size, counters, &i);
+ xt_entry_foreach(iter, loc_cpu_entry, total_size) {
+ ret = compat_copy_entry_to_user(iter, &pos,
+ &size, counters, &i);
+ if (ret != 0)
+ break;
+ }
vfree(counters);
return ret;
@@ -2137,12 +2182,15 @@ void ipt_unregister_table(struct net *net, struct xt_table *table)
struct xt_table_info *private;
void *loc_cpu_entry;
struct module *table_owner = table->me;
+ struct ipt_entry *iter;
private = xt_unregister_table(table);
/* Decrease module usage counts and free resources */
loc_cpu_entry = private->entries[raw_smp_processor_id()];
- IPT_ENTRY_ITERATE(loc_cpu_entry, private->size, cleanup_entry, net, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, private->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
if (private->number > private->initial_entries)
module_put(table_owner);
xt_free_table_info(private);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 4185099..23926e3 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -318,6 +318,7 @@ static void trace_packet(const struct sk_buff *skb,
const void *table_base;
const struct ip6t_entry *root;
const char *hookname, *chainname, *comment;
+ const struct ip6t_entry *iter;
unsigned int rulenum = 0;
table_base = private->entries[smp_processor_id()];
@@ -326,10 +327,10 @@ static void trace_packet(const struct sk_buff *skb,
hookname = chainname = hooknames[hook];
comment = comments[NF_IP6_TRACE_COMMENT_RULE];
- IP6T_ENTRY_ITERATE(root,
- private->size - private->hook_entry[hook],
- get_chainname_rulenum,
- e, hookname, &chainname, &comment, &rulenum);
+ xt_entry_foreach(iter, root, private->size - private->hook_entry[hook])
+ if (get_chainname_rulenum(iter, e, hookname,
+ &chainname, &comment, &rulenum) != 0)
+ break;
nf_log_packet(AF_INET6, hook, skb, in, out, &trace_loginfo,
"TRACE: %s:%s:%s:%u ",
@@ -857,8 +858,9 @@ translate_table(struct net *net,
const unsigned int *hook_entries,
const unsigned int *underflows)
{
+ struct ip6t_entry *iter;
unsigned int i;
- int ret;
+ int ret = 0;
newinfo->size = size;
newinfo->number = number;
@@ -872,12 +874,13 @@ translate_table(struct net *net,
duprintf("translate_table: size %u\n", newinfo->size);
i = 0;
/* Walk through entries, checking offsets. */
- ret = IP6T_ENTRY_ITERATE(entry0, newinfo->size,
- check_entry_size_and_hooks,
- newinfo,
- entry0,
- entry0 + size,
- hook_entries, underflows, valid_hooks, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ ret = check_entry_size_and_hooks(iter, newinfo, entry0,
+ entry0 + size, hook_entries, underflows,
+ valid_hooks, &i);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
return ret;
@@ -909,12 +912,16 @@ translate_table(struct net *net,
/* Finally, each sanity check must pass */
i = 0;
- ret = IP6T_ENTRY_ITERATE(entry0, newinfo->size,
- find_check_entry, net, name, size, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ ret = find_check_entry(iter, net, name, size, &i);
+ if (ret != 0)
+ break;
+ }
if (ret != 0) {
- IP6T_ENTRY_ITERATE(entry0, newinfo->size,
- cleanup_entry, net, &i);
+ xt_entry_foreach(iter, entry0, newinfo->size)
+ if (cleanup_entry(iter, net, &i) != 0)
+ break;
return ret;
}
@@ -954,6 +961,7 @@ static void
get_counters(const struct xt_table_info *t,
struct xt_counters counters[])
{
+ struct ip6t_entry *iter;
unsigned int cpu;
unsigned int i;
unsigned int curcpu;
@@ -969,22 +977,18 @@ get_counters(const struct xt_table_info *t,
curcpu = smp_processor_id();
i = 0;
- IP6T_ENTRY_ITERATE(t->entries[curcpu],
- t->size,
- set_entry_to_counter,
- counters,
- &i);
+ xt_entry_foreach(iter, t->entries[curcpu], t->size)
+ if (set_entry_to_counter(iter, counters, &i) != 0)
+ break;
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
i = 0;
xt_info_wrlock(cpu);
- IP6T_ENTRY_ITERATE(t->entries[cpu],
- t->size,
- add_entry_to_counter,
- counters,
- &i);
+ xt_entry_foreach(iter, t->entries[cpu], t->size)
+ if (add_entry_to_counter(iter, counters, &i) != 0)
+ break;
xt_info_wrunlock(cpu);
}
local_bh_enable();
@@ -1142,7 +1146,9 @@ static int compat_calc_entry(const struct ip6t_entry *e,
static int compat_table_info(const struct xt_table_info *info,
struct xt_table_info *newinfo)
{
+ struct ip6t_entry *iter;
void *loc_cpu_entry;
+ int ret = 0;
if (!newinfo || !info)
return -EINVAL;
@@ -1151,9 +1157,12 @@ static int compat_table_info(const struct xt_table_info *info,
memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
newinfo->initial_entries = 0;
loc_cpu_entry = info->entries[raw_smp_processor_id()];
- return IP6T_ENTRY_ITERATE(loc_cpu_entry, info->size,
- compat_calc_entry, info, loc_cpu_entry,
- newinfo);
+ xt_entry_foreach(iter, loc_cpu_entry, info->size) {
+ ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
+ if (ret != 0)
+ break;
+ }
+ return ret;
}
#endif
@@ -1267,6 +1276,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
struct xt_table_info *oldinfo;
struct xt_counters *counters;
const void *loc_cpu_old_entry;
+ struct ip6t_entry *iter;
ret = 0;
counters = vmalloc_node(num_counters * sizeof(struct xt_counters),
@@ -1310,8 +1320,10 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
/* Decrease module usage counts and free resource */
loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
- IP6T_ENTRY_ITERATE(loc_cpu_old_entry, oldinfo->size, cleanup_entry,
- net, NULL);
+ xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
+
xt_free_table_info(oldinfo);
if (copy_to_user(counters_ptr, counters,
sizeof(struct xt_counters) * num_counters) != 0)
@@ -1336,6 +1348,7 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
struct ip6t_replace tmp;
struct xt_table_info *newinfo;
void *loc_cpu_entry;
+ struct ip6t_entry *iter;
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
return -EFAULT;
@@ -1371,7 +1384,9 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
return 0;
free_newinfo_untrans:
- IP6T_ENTRY_ITERATE(loc_cpu_entry, newinfo->size, cleanup_entry, net, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1405,6 +1420,7 @@ do_add_counters(struct net *net, const void __user *user, unsigned int len,
const struct xt_table_info *private;
int ret = 0;
const void *loc_cpu_entry;
+ struct ip6t_entry *iter;
#ifdef CONFIG_COMPAT
struct compat_xt_counters_info compat_tmp;
@@ -1463,11 +1479,9 @@ do_add_counters(struct net *net, const void __user *user, unsigned int len,
curcpu = smp_processor_id();
xt_info_wrlock(curcpu);
loc_cpu_entry = private->entries[curcpu];
- IP6T_ENTRY_ITERATE(loc_cpu_entry,
- private->size,
- add_counter_to_entry,
- paddc,
- &i);
+ xt_entry_foreach(iter, loc_cpu_entry, private->size)
+ if (add_counter_to_entry(iter, paddc, &i) != 0)
+ break;
xt_info_wrunlock(curcpu);
unlock_up_free:
@@ -1753,8 +1767,10 @@ translate_compat_table(struct net *net,
unsigned int i, j;
struct xt_table_info *newinfo, *info;
void *pos, *entry0, *entry1;
+ struct compat_ip6t_entry *iter0;
+ struct ip6t_entry *iter1;
unsigned int size;
- int ret;
+ int ret = 0;
info = *pinfo;
entry0 = *pentry0;
@@ -1771,11 +1787,13 @@ translate_compat_table(struct net *net,
j = 0;
xt_compat_lock(AF_INET6);
/* Walk through entries, checking offsets. */
- ret = COMPAT_IP6T_ENTRY_ITERATE(entry0, total_size,
- check_compat_entry_size_and_hooks,
- info, &size, entry0,
- entry0 + total_size,
- hook_entries, underflows, &j, name);
+ xt_entry_foreach(iter0, entry0, total_size) {
+ ret = check_compat_entry_size_and_hooks(iter0, info, &size,
+ entry0, entry0 + total_size, hook_entries, underflows,
+ &j, name);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
goto out_unlock;
@@ -1816,9 +1834,12 @@ translate_compat_table(struct net *net,
entry1 = newinfo->entries[raw_smp_processor_id()];
pos = entry1;
size = total_size;
- ret = COMPAT_IP6T_ENTRY_ITERATE(entry0, total_size,
- compat_copy_entry_from_user,
- &pos, &size, name, newinfo, entry1);
+ xt_entry_foreach(iter0, entry0, total_size) {
+ ret = compat_copy_entry_from_user(iter0, &pos,
+ &size, name, newinfo, entry1);
+ if (ret != 0)
+ break;
+ }
xt_compat_flush_offsets(AF_INET6);
xt_compat_unlock(AF_INET6);
if (ret)
@@ -1829,13 +1850,28 @@ translate_compat_table(struct net *net,
goto free_newinfo;
i = 0;
- ret = IP6T_ENTRY_ITERATE(entry1, newinfo->size, compat_check_entry,
- net, name, &i);
+ xt_entry_foreach(iter1, entry1, newinfo->size) {
+ ret = compat_check_entry(iter1, net, name, &i);
+ if (ret != 0)
+ break;
+ }
if (ret) {
+ /*
+ * The first i matches need cleanup_entry (calls ->destroy)
+ * because they had called ->check already. The other j-i
+ * entries need only release.
+ */
+ int skip = i;
j -= i;
- COMPAT_IP6T_ENTRY_ITERATE_CONTINUE(entry0, newinfo->size, i,
- compat_release_entry, &j);
- IP6T_ENTRY_ITERATE(entry1, newinfo->size, cleanup_entry, net, &i);
+ xt_entry_foreach(iter0, entry0, newinfo->size) {
+ if (skip-- > 0)
+ continue;
+ if (compat_release_entry(iter0, &j) != 0)
+ break;
+ }
+ xt_entry_foreach(iter1, entry1, newinfo->size)
+ if (cleanup_entry(iter1, net, &i) != 0)
+ break;
xt_free_table_info(newinfo);
return ret;
}
@@ -1853,7 +1889,9 @@ translate_compat_table(struct net *net,
free_newinfo:
xt_free_table_info(newinfo);
out:
- COMPAT_IP6T_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
+ xt_entry_foreach(iter0, entry0, total_size)
+ if (compat_release_entry(iter0, &j) != 0)
+ break;
return ret;
out_unlock:
xt_compat_flush_offsets(AF_INET6);
@@ -1868,6 +1906,7 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
struct compat_ip6t_replace tmp;
struct xt_table_info *newinfo;
void *loc_cpu_entry;
+ struct ip6t_entry *iter;
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
return -EFAULT;
@@ -1906,7 +1945,9 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
return 0;
free_newinfo_untrans:
- IP6T_ENTRY_ITERATE(loc_cpu_entry, newinfo->size, cleanup_entry, net, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1955,6 +1996,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
int ret = 0;
const void *loc_cpu_entry;
unsigned int i = 0;
+ struct ip6t_entry *iter;
counters = alloc_counters(table);
if (IS_ERR(counters))
@@ -1967,9 +2009,12 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
loc_cpu_entry = private->entries[raw_smp_processor_id()];
pos = userptr;
size = total_size;
- ret = IP6T_ENTRY_ITERATE(loc_cpu_entry, total_size,
- compat_copy_entry_to_user,
- &pos, &size, counters, &i);
+ xt_entry_foreach(iter, loc_cpu_entry, total_size) {
+ ret = compat_copy_entry_to_user(iter, &pos,
+ &size, counters, &i);
+ if (ret != 0)
+ break;
+ }
vfree(counters);
return ret;
@@ -2169,12 +2214,15 @@ void ip6t_unregister_table(struct net *net, struct xt_table *table)
struct xt_table_info *private;
void *loc_cpu_entry;
struct module *table_owner = table->me;
+ struct ip6t_entry *iter;
private = xt_unregister_table(table);
/* Decrease module usage counts and free resources */
loc_cpu_entry = private->entries[raw_smp_processor_id()];
- IP6T_ENTRY_ITERATE(loc_cpu_entry, private->size, cleanup_entry, net, NULL);
+ xt_entry_foreach(iter, loc_cpu_entry, private->size)
+ if (cleanup_entry(iter, net, NULL) != 0)
+ break;
if (private->number > private->initial_entries)
module_put(table_owner);
xt_free_table_info(private);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/6] netfilter: xtables: replace XT_ENTRY_ITERATE macro
2010-02-12 10:20 ` [PATCH 1/6] netfilter: xtables: replace XT_ENTRY_ITERATE macro Jan Engelhardt
@ 2010-02-13 10:29 ` Amos Jeffries
2010-02-13 10:39 ` Jan Engelhardt
0 siblings, 1 reply; 16+ messages in thread
From: Amos Jeffries @ 2010-02-13 10:29 UTC (permalink / raw)
To: netfilter-devel
Jan Engelhardt wrote:
> The macro is replaced by a list.h-like foreach loop. This makes
> the code much more inspectable.
>
<snip>
> if (IS_ERR(counters))
> @@ -1967,9 +2009,12 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
> loc_cpu_entry = private->entries[raw_smp_processor_id()];
> pos = userptr;
> size = total_size;
> - ret = IP6T_ENTRY_ITERATE(loc_cpu_entry, total_size,
> - compat_copy_entry_to_user,
> - &pos, &size, counters, &i);
> + xt_entry_foreach(iter, loc_cpu_entry, total_size) {
> + ret = compat_copy_entry_to_user(iter, &pos,
> + &size, counters, &i);
> + if (ret != 0)
> + break;
> + }
>
<snip>
> loc_cpu_entry = private->entries[raw_smp_processor_id()];
> - IP6T_ENTRY_ITERATE(loc_cpu_entry, private->size, cleanup_entry, net, NULL);
> + xt_entry_foreach(iter, loc_cpu_entry, private->size)
> + if (cleanup_entry(iter, net, NULL) != 0)
> + break;
> if (private->number > private->initial_entries)
> module_put(table_owner);
> xt_free_table_info(private);
Just my 2c, but you seem to be floating between several coding styles on
this one.
AYJ
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/6] netfilter: xtables: replace XT_ENTRY_ITERATE macro
2010-02-13 10:29 ` Amos Jeffries
@ 2010-02-13 10:39 ` Jan Engelhardt
2010-02-13 10:42 ` Amos Jeffries
0 siblings, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-13 10:39 UTC (permalink / raw)
To: Amos Jeffries; +Cc: netfilter-devel
On Saturday 2010-02-13 11:29, Amos Jeffries wrote:
> Jan Engelhardt wrote:
>> The macro is replaced by a list.h-like foreach loop. This makes
>> the code much more inspectable.
>>
> <snip>
>> if (IS_ERR(counters))
>> @@ -1967,9 +2009,12 @@ compat_copy_entries_to_user(unsigned int total_size,
>> struct xt_table *table,
>> loc_cpu_entry = private->entries[raw_smp_processor_id()];
>> pos = userptr;
>> size = total_size;
>> - ret = IP6T_ENTRY_ITERATE(loc_cpu_entry, total_size,
>> - compat_copy_entry_to_user,
>> - &pos, &size, counters, &i);
>> + xt_entry_foreach(iter, loc_cpu_entry, total_size) {
>> + ret = compat_copy_entry_to_user(iter, &pos,
>> + &size, counters, &i);
>> + if (ret != 0)
>> + break;
>> + }
>>
>
> <snip>
>
>> loc_cpu_entry = private->entries[raw_smp_processor_id()];
>> - IP6T_ENTRY_ITERATE(loc_cpu_entry, private->size, cleanup_entry, net,
>> NULL);
>> + xt_entry_foreach(iter, loc_cpu_entry, private->size)
>> + if (cleanup_entry(iter, net, NULL) != 0)
>> + break;
>> if (private->number > private->initial_entries)
>> module_put(table_owner);
>> xt_free_table_info(private);
>
> Just my 2c, but you seem to be floating between several coding styles on this
> one.
What, where? It goes by two simple rules:
- no assignments in if()
- only use {} where needed
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/6] netfilter: xtables: replace XT_ENTRY_ITERATE macro
2010-02-13 10:39 ` Jan Engelhardt
@ 2010-02-13 10:42 ` Amos Jeffries
0 siblings, 0 replies; 16+ messages in thread
From: Amos Jeffries @ 2010-02-13 10:42 UTC (permalink / raw)
To: netfilter-devel
Jan Engelhardt wrote:
> On Saturday 2010-02-13 11:29, Amos Jeffries wrote:
>> Jan Engelhardt wrote:
>>> The macro is replaced by a list.h-like foreach loop. This makes
>>> the code much more inspectable.
>>>
>> <snip>
>>> if (IS_ERR(counters))
>>> @@ -1967,9 +2009,12 @@ compat_copy_entries_to_user(unsigned int total_size,
>>> struct xt_table *table,
>>> loc_cpu_entry = private->entries[raw_smp_processor_id()];
>>> pos = userptr;
>>> size = total_size;
>>> - ret = IP6T_ENTRY_ITERATE(loc_cpu_entry, total_size,
>>> - compat_copy_entry_to_user,
>>> - &pos, &size, counters, &i);
>>> + xt_entry_foreach(iter, loc_cpu_entry, total_size) {
>>> + ret = compat_copy_entry_to_user(iter, &pos,
>>> + &size, counters, &i);
>>> + if (ret != 0)
>>> + break;
>>> + }
>>>
>> <snip>
>>
>>> loc_cpu_entry = private->entries[raw_smp_processor_id()];
>>> - IP6T_ENTRY_ITERATE(loc_cpu_entry, private->size, cleanup_entry, net,
>>> NULL);
>>> + xt_entry_foreach(iter, loc_cpu_entry, private->size)
>>> + if (cleanup_entry(iter, net, NULL) != 0)
>>> + break;
>>> if (private->number > private->initial_entries)
>>> module_put(table_owner);
>>> xt_free_table_info(private);
>> Just my 2c, but you seem to be floating between several coding styles on this
>> one.
>
> What, where? It goes by two simple rules:
> - no assignments in if()
> - only use {} where needed
Oh I see. ret being used later. nevermind.
AYJ
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/6] netfilter: xtables: optimize call flow around xt_entry_foreach
2010-02-12 10:20 ip_tables: foreachs and reentrancy Jan Engelhardt
2010-02-12 10:20 ` [PATCH 1/6] netfilter: xtables: replace XT_ENTRY_ITERATE macro Jan Engelhardt
@ 2010-02-12 10:20 ` Jan Engelhardt
2010-02-12 10:20 ` [PATCH 3/6] netfilter: xtables: replace XT_MATCH_ITERATE macro Jan Engelhardt
` (4 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-12 10:20 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
net/ipv4/netfilter/arp_tables.c | 180 ++++++++++++--------------------------
net/ipv4/netfilter/ip_tables.c | 183 +++++++++++++-------------------------
net/ipv6/netfilter/ip6_tables.c | 179 +++++++++++++-------------------------
3 files changed, 182 insertions(+), 360 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f733886..5fdedeb 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -512,8 +512,7 @@ static inline int check_target(struct arpt_entry *e, const char *name)
}
static inline int
-find_check_entry(struct arpt_entry *e, const char *name, unsigned int size,
- unsigned int *i)
+find_check_entry(struct arpt_entry *e, const char *name, unsigned int size)
{
struct arpt_entry_target *t;
struct xt_target *target;
@@ -538,8 +537,6 @@ find_check_entry(struct arpt_entry *e, const char *name, unsigned int size,
ret = check_target(e, name);
if (ret)
goto err;
-
- (*i)++;
return 0;
err:
module_put(t->u.kernel.target->me);
@@ -568,8 +565,7 @@ static inline int check_entry_size_and_hooks(struct arpt_entry *e,
const unsigned char *limit,
const unsigned int *hook_entries,
const unsigned int *underflows,
- unsigned int valid_hooks,
- unsigned int *i)
+ unsigned int valid_hooks)
{
unsigned int h;
@@ -606,19 +602,14 @@ static inline int check_entry_size_and_hooks(struct arpt_entry *e,
/* Clear counters and comefrom */
e->counters = ((struct xt_counters) { 0, 0 });
e->comefrom = 0;
-
- (*i)++;
return 0;
}
-static inline int cleanup_entry(struct arpt_entry *e, unsigned int *i)
+static inline void cleanup_entry(struct arpt_entry *e)
{
struct xt_tgdtor_param par;
struct arpt_entry_target *t;
- if (i && (*i)-- == 0)
- return 1;
-
t = arpt_get_target(e);
par.target = t->u.kernel.target;
par.targinfo = t->data;
@@ -626,7 +617,6 @@ static inline int cleanup_entry(struct arpt_entry *e, unsigned int *i)
if (par.target->destroy != NULL)
par.target->destroy(&par);
module_put(par.target->me);
- return 0;
}
/* Checks and translates the user-supplied table segment (held in
@@ -660,10 +650,10 @@ static int translate_table(const char *name,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + size, hook_entries, underflows,
- valid_hooks, &i);
+ entry0 + size, hook_entries, underflows, valid_hooks);
if (ret != 0)
break;
+ ++i;
}
duprintf("translate_table: ARPT_ENTRY_ITERATE gives %d\n", ret);
if (ret != 0)
@@ -700,15 +690,18 @@ static int translate_table(const char *name,
/* Finally, each sanity check must pass */
i = 0;
xt_entry_foreach(iter, entry0, newinfo->size) {
- ret = find_check_entry(iter, name, size, &i);
+ ret = find_check_entry(iter, name, size);
if (ret != 0)
break;
+ ++i;
}
if (ret != 0) {
- xt_entry_foreach(iter, entry0, newinfo->size)
- if (cleanup_entry(iter, &i) != 0)
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ if (i-- == 0)
break;
+ cleanup_entry(iter);
+ }
return ret;
}
@@ -721,27 +714,6 @@ static int translate_table(const char *name,
return ret;
}
-/* Gets counters. */
-static inline int add_entry_to_counter(const struct arpt_entry *e,
- struct xt_counters total[],
- unsigned int *i)
-{
- ADD_COUNTER(total[*i], e->counters.bcnt, e->counters.pcnt);
-
- (*i)++;
- return 0;
-}
-
-static inline int set_entry_to_counter(const struct arpt_entry *e,
- struct xt_counters total[],
- unsigned int *i)
-{
- SET_COUNTER(total[*i], e->counters.bcnt, e->counters.pcnt);
-
- (*i)++;
- return 0;
-}
-
static void get_counters(const struct xt_table_info *t,
struct xt_counters counters[])
{
@@ -761,18 +733,22 @@ static void get_counters(const struct xt_table_info *t,
curcpu = smp_processor_id();
i = 0;
- xt_entry_foreach(iter, t->entries[curcpu], t->size)
- if (set_entry_to_counter(iter, counters, &i) != 0)
- break;
+ xt_entry_foreach(iter, t->entries[curcpu], t->size) {
+ SET_COUNTER(counters[i], iter->counters.bcnt,
+ iter->counters.pcnt);
+ ++i;
+ }
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
i = 0;
xt_info_wrlock(cpu);
- xt_entry_foreach(iter, t->entries[cpu], t->size)
- if (add_entry_to_counter(iter, counters, &i) != 0)
- break;
+ xt_entry_foreach(iter, t->entries[cpu], t->size) {
+ ADD_COUNTER(counters[i], iter->counters.bcnt,
+ iter->counters.pcnt);
+ ++i;
+ }
xt_info_wrunlock(cpu);
}
local_bh_enable();
@@ -904,7 +880,7 @@ static int compat_table_info(const struct xt_table_info *info,
{
struct arpt_entry *iter;
void *loc_cpu_entry;
- int ret = 0;
+ int ret;
if (!newinfo || !info)
return -EINVAL;
@@ -916,9 +892,9 @@ static int compat_table_info(const struct xt_table_info *info,
xt_entry_foreach(iter, loc_cpu_entry, info->size) {
ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
if (ret != 0)
- break;
+ return ret;
}
- return ret;
+ return 0;
}
#endif
@@ -1078,8 +1054,7 @@ static int __do_replace(struct net *net, const char *name,
/* Decrease module usage counts and free resource */
loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
- if (cleanup_entry(iter, NULL) != 0)
- break;
+ cleanup_entry(iter);
xt_free_table_info(oldinfo);
if (copy_to_user(counters_ptr, counters,
@@ -1142,26 +1117,12 @@ static int do_replace(struct net *net, const void __user *user,
free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
- if (cleanup_entry(iter, NULL) != 0)
- break;
+ cleanup_entry(iter);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
}
-/* We're lazy, and add to the first CPU; overflow works its fey magic
- * and everything is OK. */
-static int
-add_counter_to_entry(struct arpt_entry *e,
- const struct xt_counters addme[],
- unsigned int *i)
-{
- ADD_COUNTER(e->counters, addme[*i].bcnt, addme[*i].pcnt);
-
- (*i)++;
- return 0;
-}
-
static int do_add_counters(struct net *net, const void __user *user,
unsigned int len, int compat)
{
@@ -1234,9 +1195,10 @@ static int do_add_counters(struct net *net, const void __user *user,
curcpu = smp_processor_id();
loc_cpu_entry = private->entries[curcpu];
xt_info_wrlock(curcpu);
- xt_entry_foreach(iter, loc_cpu_entry, private->size)
- if (add_counter_to_entry(iter, paddc, &i) != 0)
- break;
+ xt_entry_foreach(iter, loc_cpu_entry, private->size) {
+ ADD_COUNTER(iter->counters, paddc[i].bcnt, paddc[i].pcnt);
+ ++i;
+ }
xt_info_wrunlock(curcpu);
unlock_up_free:
local_bh_enable();
@@ -1249,17 +1211,12 @@ static int do_add_counters(struct net *net, const void __user *user,
}
#ifdef CONFIG_COMPAT
-static inline int
-compat_release_entry(struct compat_arpt_entry *e, unsigned int *i)
+static inline void compat_release_entry(struct compat_arpt_entry *e)
{
struct arpt_entry_target *t;
- if (i && (*i)-- == 0)
- return 1;
-
t = compat_arpt_get_target(e);
module_put(t->u.kernel.target->me);
- return 0;
}
static inline int
@@ -1270,7 +1227,6 @@ check_compat_entry_size_and_hooks(struct compat_arpt_entry *e,
const unsigned char *limit,
const unsigned int *hook_entries,
const unsigned int *underflows,
- unsigned int *i,
const char *name)
{
struct arpt_entry_target *t;
@@ -1330,8 +1286,6 @@ check_compat_entry_size_and_hooks(struct compat_arpt_entry *e,
/* Clear counters and comefrom */
memset(&e->counters, 0, sizeof(e->counters));
e->comefrom = 0;
-
- (*i)++;
return 0;
release_target:
@@ -1375,19 +1329,6 @@ compat_copy_entry_from_user(struct compat_arpt_entry *e, void **dstptr,
return ret;
}
-static inline int compat_check_entry(struct arpt_entry *e, const char *name,
- unsigned int *i)
-{
- int ret;
-
- ret = check_target(e, name);
- if (ret)
- return ret;
-
- (*i)++;
- return 0;
-}
-
static int translate_compat_table(const char *name,
unsigned int valid_hooks,
struct xt_table_info **pinfo,
@@ -1423,12 +1364,11 @@ static int translate_compat_table(const char *name,
xt_entry_foreach(iter0, entry0, total_size) {
ret = check_compat_entry_size_and_hooks(iter0, info, &size,
entry0, entry0 + total_size, hook_entries, underflows,
- &j, name);
+ name);
if (ret != 0)
- break;
+ goto out_unlock;
+ ++j;
}
- if (ret != 0)
- goto out_unlock;
ret = -EINVAL;
if (j != number) {
@@ -1484,9 +1424,10 @@ static int translate_compat_table(const char *name,
i = 0;
xt_entry_foreach(iter1, entry1, newinfo->size) {
- ret = compat_check_entry(iter1, name, &i);
+ ret = check_target(iter1, name);
if (ret != 0)
break;
+ ++i;
}
if (ret) {
/*
@@ -1499,12 +1440,15 @@ static int translate_compat_table(const char *name,
xt_entry_foreach(iter0, entry0, newinfo->size) {
if (skip-- > 0)
continue;
- if (compat_release_entry(iter0, &j) != 0)
+ if (j-- == 0)
break;
+ compat_release_entry(iter0);
}
- xt_entry_foreach(iter1, entry1, newinfo->size)
- if (cleanup_entry(iter1, &i) != 0)
+ xt_entry_foreach(iter1, entry1, newinfo->size) {
+ if (i-- == 0)
break;
+ cleanup_entry(iter1);
+ }
xt_free_table_info(newinfo);
return ret;
}
@@ -1522,9 +1466,11 @@ static int translate_compat_table(const char *name,
free_newinfo:
xt_free_table_info(newinfo);
out:
- xt_entry_foreach(iter0, entry0, total_size)
- if (compat_release_entry(iter0, &j) != 0)
+ xt_entry_foreach(iter0, entry0, total_size) {
+ if (j-- == 0)
break;
+ compat_release_entry(iter0);
+ }
return ret;
out_unlock:
xt_compat_flush_offsets(NFPROTO_ARP);
@@ -1590,8 +1536,7 @@ static int compat_do_replace(struct net *net, void __user *user,
free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
- if (cleanup_entry(iter, NULL) != 0)
- break;
+ cleanup_entry(iter);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1625,7 +1570,7 @@ static int compat_do_arpt_set_ctl(struct sock *sk, int cmd, void __user *user,
static int compat_copy_entry_to_user(struct arpt_entry *e, void __user **dstptr,
compat_uint_t *size,
struct xt_counters *counters,
- unsigned int *i)
+ unsigned int i)
{
struct arpt_entry_target *t;
struct compat_arpt_entry __user *ce;
@@ -1633,14 +1578,12 @@ static int compat_copy_entry_to_user(struct arpt_entry *e, void __user **dstptr,
compat_uint_t origsize;
int ret;
- ret = -EFAULT;
origsize = *size;
ce = (struct compat_arpt_entry __user *)*dstptr;
- if (copy_to_user(ce, e, sizeof(struct arpt_entry)))
- goto out;
-
- if (copy_to_user(&ce->counters, &counters[*i], sizeof(counters[*i])))
- goto out;
+ if (copy_to_user(ce, e, sizeof(struct arpt_entry)) != 0 ||
+ copy_to_user(&ce->counters, &counters[i],
+ sizeof(counters[i])) != 0)
+ return -EFAULT;
*dstptr += sizeof(struct compat_arpt_entry);
*size -= sizeof(struct arpt_entry) - sizeof(struct compat_arpt_entry);
@@ -1650,18 +1593,12 @@ static int compat_copy_entry_to_user(struct arpt_entry *e, void __user **dstptr,
t = arpt_get_target(e);
ret = xt_compat_target_to_user(t, dstptr, size);
if (ret)
- goto out;
- ret = -EFAULT;
+ return ret;
next_offset = e->next_offset - (origsize - *size);
- if (put_user(target_offset, &ce->target_offset))
- goto out;
- if (put_user(next_offset, &ce->next_offset))
- goto out;
-
- (*i)++;
+ if (put_user(target_offset, &ce->target_offset) != 0 ||
+ put_user(next_offset, &ce->next_offset) != 0)
+ return -EFAULT;
return 0;
-out:
- return ret;
}
static int compat_copy_entries_to_user(unsigned int total_size,
@@ -1687,7 +1624,7 @@ static int compat_copy_entries_to_user(unsigned int total_size,
size = total_size;
xt_entry_foreach(iter, loc_cpu_entry, total_size) {
ret = compat_copy_entry_to_user(iter, &pos,
- &size, counters, &i);
+ &size, counters, i++);
if (ret != 0)
break;
}
@@ -1893,8 +1830,7 @@ void arpt_unregister_table(struct xt_table *table)
/* Decrease module usage counts and free resources */
loc_cpu_entry = private->entries[raw_smp_processor_id()];
xt_entry_foreach(iter, loc_cpu_entry, private->size)
- if (cleanup_entry(iter, NULL) != 0)
- break;
+ cleanup_entry(iter);
if (private->number > private->initial_entries)
module_put(table_owner);
xt_free_table_info(private);
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index b43280a..9c8aa39 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -679,7 +679,7 @@ static int check_target(struct ipt_entry *e, struct net *net, const char *name)
static int
find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
- unsigned int size, unsigned int *i)
+ unsigned int size)
{
struct ipt_entry_target *t;
struct xt_target *target;
@@ -716,8 +716,6 @@ find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
ret = check_target(e, net, name);
if (ret)
goto err;
-
- (*i)++;
return 0;
err:
module_put(t->u.kernel.target->me);
@@ -748,8 +746,7 @@ check_entry_size_and_hooks(struct ipt_entry *e,
const unsigned char *limit,
const unsigned int *hook_entries,
const unsigned int *underflows,
- unsigned int valid_hooks,
- unsigned int *i)
+ unsigned int valid_hooks)
{
unsigned int h;
@@ -786,20 +783,15 @@ check_entry_size_and_hooks(struct ipt_entry *e,
/* Clear counters and comefrom */
e->counters = ((struct xt_counters) { 0, 0 });
e->comefrom = 0;
-
- (*i)++;
return 0;
}
-static int
-cleanup_entry(struct ipt_entry *e, struct net *net, unsigned int *i)
+static void
+cleanup_entry(struct ipt_entry *e, struct net *net)
{
struct xt_tgdtor_param par;
struct ipt_entry_target *t;
- if (i && (*i)-- == 0)
- return 1;
-
/* Cleanup all matches */
IPT_MATCH_ITERATE(e, cleanup_match, net, NULL);
t = ipt_get_target(e);
@@ -811,7 +803,6 @@ cleanup_entry(struct ipt_entry *e, struct net *net, unsigned int *i)
if (par.target->destroy != NULL)
par.target->destroy(&par);
module_put(par.target->me);
- return 0;
}
/* Checks and translates the user-supplied table segment (held in
@@ -845,13 +836,11 @@ translate_table(struct net *net,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + size, hook_entries, underflows,
- valid_hooks, &i);
+ entry0 + size, hook_entries, underflows, valid_hooks);
if (ret != 0)
- break;
+ return ret;
+ ++i;
}
- if (ret != 0)
- return ret;
if (i != number) {
duprintf("translate_table: %u not %u entries\n",
@@ -882,15 +871,18 @@ translate_table(struct net *net,
/* Finally, each sanity check must pass */
i = 0;
xt_entry_foreach(iter, entry0, newinfo->size) {
- ret = find_check_entry(iter, net, name, size, &i);
+ ret = find_check_entry(iter, net, name, size);
if (ret != 0)
break;
+ ++i;
}
if (ret != 0) {
- xt_entry_foreach(iter, entry0, newinfo->size)
- if (cleanup_entry(iter, net, &i) != 0)
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ if (i-- == 0)
break;
+ cleanup_entry(iter, net);
+ }
return ret;
}
@@ -903,29 +895,6 @@ translate_table(struct net *net,
return ret;
}
-/* Gets counters. */
-static inline int
-add_entry_to_counter(const struct ipt_entry *e,
- struct xt_counters total[],
- unsigned int *i)
-{
- ADD_COUNTER(total[*i], e->counters.bcnt, e->counters.pcnt);
-
- (*i)++;
- return 0;
-}
-
-static inline int
-set_entry_to_counter(const struct ipt_entry *e,
- struct ipt_counters total[],
- unsigned int *i)
-{
- SET_COUNTER(total[*i], e->counters.bcnt, e->counters.pcnt);
-
- (*i)++;
- return 0;
-}
-
static void
get_counters(const struct xt_table_info *t,
struct xt_counters counters[])
@@ -946,18 +915,22 @@ get_counters(const struct xt_table_info *t,
curcpu = smp_processor_id();
i = 0;
- xt_entry_foreach(iter, t->entries[curcpu], t->size)
- if (set_entry_to_counter(iter, counters, &i) != 0)
- break;
+ xt_entry_foreach(iter, t->entries[curcpu], t->size) {
+ SET_COUNTER(counters[i], iter->counters.bcnt,
+ iter->counters.pcnt);
+ ++i;
+ }
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
i = 0;
xt_info_wrlock(cpu);
- xt_entry_foreach(iter, t->entries[cpu], t->size)
- if (add_entry_to_counter(iter, counters, &i) != 0)
- break;
+ xt_entry_foreach(iter, t->entries[cpu], t->size) {
+ ADD_COUNTER(counters[i], iter->counters.bcnt,
+ iter->counters.pcnt);
+ ++i; /* macro does multi eval of i */
+ }
xt_info_wrunlock(cpu);
}
local_bh_enable();
@@ -1117,7 +1090,7 @@ static int compat_table_info(const struct xt_table_info *info,
{
struct ipt_entry *iter;
void *loc_cpu_entry;
- int ret = 0;
+ int ret;
if (!newinfo || !info)
return -EINVAL;
@@ -1129,9 +1102,9 @@ static int compat_table_info(const struct xt_table_info *info,
xt_entry_foreach(iter, loc_cpu_entry, info->size) {
ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
if (ret != 0)
- break;
+ return ret;
}
- return ret;
+ return 0;
}
#endif
@@ -1289,8 +1262,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
/* Decrease module usage counts and free resource */
loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
xt_free_table_info(oldinfo);
if (copy_to_user(counters_ptr, counters,
@@ -1353,26 +1325,12 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
}
-/* We're lazy, and add to the first CPU; overflow works its fey magic
- * and everything is OK. */
-static int
-add_counter_to_entry(struct ipt_entry *e,
- const struct xt_counters addme[],
- unsigned int *i)
-{
- ADD_COUNTER(e->counters, addme[*i].bcnt, addme[*i].pcnt);
-
- (*i)++;
- return 0;
-}
-
static int
do_add_counters(struct net *net, const void __user *user,
unsigned int len, int compat)
@@ -1446,9 +1404,10 @@ do_add_counters(struct net *net, const void __user *user,
curcpu = smp_processor_id();
loc_cpu_entry = private->entries[curcpu];
xt_info_wrlock(curcpu);
- xt_entry_foreach(iter, loc_cpu_entry, private->size)
- if (add_counter_to_entry(iter, paddc, &i) != 0)
- break;
+ xt_entry_foreach(iter, loc_cpu_entry, private->size) {
+ ADD_COUNTER(iter->counters, paddc[i].bcnt, paddc[i].pcnt);
+ ++i;
+ }
xt_info_wrunlock(curcpu);
unlock_up_free:
local_bh_enable();
@@ -1476,7 +1435,7 @@ struct compat_ipt_replace {
static int
compat_copy_entry_to_user(struct ipt_entry *e, void __user **dstptr,
unsigned int *size, struct xt_counters *counters,
- unsigned int *i)
+ unsigned int i)
{
struct ipt_entry_target *t;
struct compat_ipt_entry __user *ce;
@@ -1484,14 +1443,12 @@ compat_copy_entry_to_user(struct ipt_entry *e, void __user **dstptr,
compat_uint_t origsize;
int ret;
- ret = -EFAULT;
origsize = *size;
ce = (struct compat_ipt_entry __user *)*dstptr;
- if (copy_to_user(ce, e, sizeof(struct ipt_entry)))
- goto out;
-
- if (copy_to_user(&ce->counters, &counters[*i], sizeof(counters[*i])))
- goto out;
+ if (copy_to_user(ce, e, sizeof(struct ipt_entry)) != 0 ||
+ copy_to_user(&ce->counters, &counters[i],
+ sizeof(counters[i])) != 0)
+ return -EFAULT;
*dstptr += sizeof(struct compat_ipt_entry);
*size -= sizeof(struct ipt_entry) - sizeof(struct compat_ipt_entry);
@@ -1499,22 +1456,16 @@ compat_copy_entry_to_user(struct ipt_entry *e, void __user **dstptr,
ret = IPT_MATCH_ITERATE(e, xt_compat_match_to_user, dstptr, size);
target_offset = e->target_offset - (origsize - *size);
if (ret)
- goto out;
+ return ret;
t = ipt_get_target(e);
ret = xt_compat_target_to_user(t, dstptr, size);
if (ret)
- goto out;
- ret = -EFAULT;
+ return ret;
next_offset = e->next_offset - (origsize - *size);
- if (put_user(target_offset, &ce->target_offset))
- goto out;
- if (put_user(next_offset, &ce->next_offset))
- goto out;
-
- (*i)++;
+ if (put_user(target_offset, &ce->target_offset) != 0 ||
+ put_user(next_offset, &ce->next_offset) != 0)
+ return -EFAULT;
return 0;
-out:
- return ret;
}
static int
@@ -1551,19 +1502,14 @@ compat_release_match(struct ipt_entry_match *m, unsigned int *i)
return 0;
}
-static int
-compat_release_entry(struct compat_ipt_entry *e, unsigned int *i)
+static void compat_release_entry(struct compat_ipt_entry *e)
{
struct ipt_entry_target *t;
- if (i && (*i)-- == 0)
- return 1;
-
/* Cleanup all matches */
COMPAT_IPT_MATCH_ITERATE(e, compat_release_match, NULL);
t = compat_ipt_get_target(e);
module_put(t->u.kernel.target->me);
- return 0;
}
static int
@@ -1574,7 +1520,6 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
const unsigned char *limit,
const unsigned int *hook_entries,
const unsigned int *underflows,
- unsigned int *i,
const char *name)
{
struct ipt_entry_target *t;
@@ -1640,8 +1585,6 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
/* Clear counters and comefrom */
memset(&e->counters, 0, sizeof(e->counters));
e->comefrom = 0;
-
- (*i)++;
return 0;
out:
@@ -1691,8 +1634,7 @@ compat_copy_entry_from_user(struct compat_ipt_entry *e, void **dstptr,
}
static int
-compat_check_entry(struct ipt_entry *e, struct net *net, const char *name,
- unsigned int *i)
+compat_check_entry(struct ipt_entry *e, struct net *net, const char *name)
{
struct xt_mtchk_param mtpar;
unsigned int j;
@@ -1711,8 +1653,6 @@ compat_check_entry(struct ipt_entry *e, struct net *net, const char *name,
ret = check_target(e, net, name);
if (ret)
goto cleanup_matches;
-
- (*i)++;
return 0;
cleanup_matches:
@@ -1737,7 +1677,7 @@ translate_compat_table(struct net *net,
struct compat_ipt_entry *iter0;
struct ipt_entry *iter1;
unsigned int size;
- int ret = 0;
+ int ret;
info = *pinfo;
entry0 = *pentry0;
@@ -1757,12 +1697,11 @@ translate_compat_table(struct net *net,
xt_entry_foreach(iter0, entry0, total_size) {
ret = check_compat_entry_size_and_hooks(iter0, info, &size,
entry0, entry0 + total_size, hook_entries, underflows,
- &j, name);
+ name);
if (ret != 0)
- break;
+ goto out_unlock;
+ ++j;
}
- if (ret != 0)
- goto out_unlock;
ret = -EINVAL;
if (j != number) {
@@ -1818,9 +1757,10 @@ translate_compat_table(struct net *net,
i = 0;
xt_entry_foreach(iter1, entry1, newinfo->size) {
- ret = compat_check_entry(iter1, net, name, &i);
+ ret = compat_check_entry(iter1, net, name);
if (ret != 0)
break;
+ ++i;
}
if (ret) {
/*
@@ -1833,12 +1773,15 @@ translate_compat_table(struct net *net,
xt_entry_foreach(iter0, entry0, newinfo->size) {
if (skip-- > 0)
continue;
- if (compat_release_entry(iter0, &i) != 0)
+ if (j-- == 0)
break;
+ compat_release_entry(iter0);
}
- xt_entry_foreach(iter1, entry1, newinfo->size)
- if (cleanup_entry(iter1, net, &i) != 0)
+ xt_entry_foreach(iter1, entry1, newinfo->size) {
+ if (i-- == 0)
break;
+ cleanup_entry(iter1, net);
+ }
xt_free_table_info(newinfo);
return ret;
}
@@ -1856,9 +1799,11 @@ translate_compat_table(struct net *net,
free_newinfo:
xt_free_table_info(newinfo);
out:
- xt_entry_foreach(iter0, entry0, total_size)
- if (compat_release_entry(iter0, &j) != 0)
+ xt_entry_foreach(iter0, entry0, total_size) {
+ if (j-- == 0)
break;
+ compat_release_entry(iter0);
+ }
return ret;
out_unlock:
xt_compat_flush_offsets(AF_INET);
@@ -1913,8 +1858,7 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -1978,7 +1922,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
size = total_size;
xt_entry_foreach(iter, loc_cpu_entry, total_size) {
ret = compat_copy_entry_to_user(iter, &pos,
- &size, counters, &i);
+ &size, counters, i++);
if (ret != 0)
break;
}
@@ -2189,8 +2133,7 @@ void ipt_unregister_table(struct net *net, struct xt_table *table)
/* Decrease module usage counts and free resources */
loc_cpu_entry = private->entries[raw_smp_processor_id()];
xt_entry_foreach(iter, loc_cpu_entry, private->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
if (private->number > private->initial_entries)
module_put(table_owner);
xt_free_table_info(private);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 23926e3..b7e27c1 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -710,7 +710,7 @@ static int check_target(struct ip6t_entry *e, struct net *net, const char *name)
static int
find_check_entry(struct ip6t_entry *e, struct net *net, const char *name,
- unsigned int size, unsigned int *i)
+ unsigned int size)
{
struct ip6t_entry_target *t;
struct xt_target *target;
@@ -747,8 +747,6 @@ find_check_entry(struct ip6t_entry *e, struct net *net, const char *name,
ret = check_target(e, net, name);
if (ret)
goto err;
-
- (*i)++;
return 0;
err:
module_put(t->u.kernel.target->me);
@@ -779,8 +777,7 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
const unsigned char *limit,
const unsigned int *hook_entries,
const unsigned int *underflows,
- unsigned int valid_hooks,
- unsigned int *i)
+ unsigned int valid_hooks)
{
unsigned int h;
@@ -817,20 +814,14 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
/* Clear counters and comefrom */
e->counters = ((struct xt_counters) { 0, 0 });
e->comefrom = 0;
-
- (*i)++;
return 0;
}
-static int
-cleanup_entry(struct ip6t_entry *e, struct net *net, unsigned int *i)
+static void cleanup_entry(struct ip6t_entry *e, struct net *net)
{
struct xt_tgdtor_param par;
struct ip6t_entry_target *t;
- if (i && (*i)-- == 0)
- return 1;
-
/* Cleanup all matches */
IP6T_MATCH_ITERATE(e, cleanup_match, net, NULL);
t = ip6t_get_target(e);
@@ -842,7 +833,6 @@ cleanup_entry(struct ip6t_entry *e, struct net *net, unsigned int *i)
if (par.target->destroy != NULL)
par.target->destroy(&par);
module_put(par.target->me);
- return 0;
}
/* Checks and translates the user-supplied table segment (held in
@@ -876,13 +866,11 @@ translate_table(struct net *net,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + size, hook_entries, underflows,
- valid_hooks, &i);
+ entry0 + size, hook_entries, underflows, valid_hooks);
if (ret != 0)
- break;
+ return ret;
+ ++i;
}
- if (ret != 0)
- return ret;
if (i != number) {
duprintf("translate_table: %u not %u entries\n",
@@ -913,15 +901,18 @@ translate_table(struct net *net,
/* Finally, each sanity check must pass */
i = 0;
xt_entry_foreach(iter, entry0, newinfo->size) {
- ret = find_check_entry(iter, net, name, size, &i);
+ ret = find_check_entry(iter, net, name, size);
if (ret != 0)
break;
+ ++i;
}
if (ret != 0) {
- xt_entry_foreach(iter, entry0, newinfo->size)
- if (cleanup_entry(iter, net, &i) != 0)
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ if (i-- == 0)
break;
+ cleanup_entry(iter, net);
+ }
return ret;
}
@@ -934,29 +925,6 @@ translate_table(struct net *net,
return ret;
}
-/* Gets counters. */
-static inline int
-add_entry_to_counter(const struct ip6t_entry *e,
- struct xt_counters total[],
- unsigned int *i)
-{
- ADD_COUNTER(total[*i], e->counters.bcnt, e->counters.pcnt);
-
- (*i)++;
- return 0;
-}
-
-static inline int
-set_entry_to_counter(const struct ip6t_entry *e,
- struct ip6t_counters total[],
- unsigned int *i)
-{
- SET_COUNTER(total[*i], e->counters.bcnt, e->counters.pcnt);
-
- (*i)++;
- return 0;
-}
-
static void
get_counters(const struct xt_table_info *t,
struct xt_counters counters[])
@@ -977,18 +945,22 @@ get_counters(const struct xt_table_info *t,
curcpu = smp_processor_id();
i = 0;
- xt_entry_foreach(iter, t->entries[curcpu], t->size)
- if (set_entry_to_counter(iter, counters, &i) != 0)
- break;
+ xt_entry_foreach(iter, t->entries[curcpu], t->size) {
+ SET_COUNTER(counters[i], iter->counters.bcnt,
+ iter->counters.pcnt);
+ ++i;
+ }
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
i = 0;
xt_info_wrlock(cpu);
- xt_entry_foreach(iter, t->entries[cpu], t->size)
- if (add_entry_to_counter(iter, counters, &i) != 0)
- break;
+ xt_entry_foreach(iter, t->entries[cpu], t->size) {
+ ADD_COUNTER(counters[i], iter->counters.bcnt,
+ iter->counters.pcnt);
+ ++i;
+ }
xt_info_wrunlock(cpu);
}
local_bh_enable();
@@ -1148,7 +1120,7 @@ static int compat_table_info(const struct xt_table_info *info,
{
struct ip6t_entry *iter;
void *loc_cpu_entry;
- int ret = 0;
+ int ret;
if (!newinfo || !info)
return -EINVAL;
@@ -1160,9 +1132,9 @@ static int compat_table_info(const struct xt_table_info *info,
xt_entry_foreach(iter, loc_cpu_entry, info->size) {
ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
if (ret != 0)
- break;
+ return ret;
}
- return ret;
+ return 0;
}
#endif
@@ -1321,8 +1293,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
/* Decrease module usage counts and free resource */
loc_cpu_old_entry = oldinfo->entries[raw_smp_processor_id()];
xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
xt_free_table_info(oldinfo);
if (copy_to_user(counters_ptr, counters,
@@ -1385,26 +1356,12 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
}
-/* We're lazy, and add to the first CPU; overflow works its fey magic
- * and everything is OK. */
-static int
-add_counter_to_entry(struct ip6t_entry *e,
- const struct xt_counters addme[],
- unsigned int *i)
-{
- ADD_COUNTER(e->counters, addme[*i].bcnt, addme[*i].pcnt);
-
- (*i)++;
- return 0;
-}
-
static int
do_add_counters(struct net *net, const void __user *user, unsigned int len,
int compat)
@@ -1479,9 +1436,10 @@ do_add_counters(struct net *net, const void __user *user, unsigned int len,
curcpu = smp_processor_id();
xt_info_wrlock(curcpu);
loc_cpu_entry = private->entries[curcpu];
- xt_entry_foreach(iter, loc_cpu_entry, private->size)
- if (add_counter_to_entry(iter, paddc, &i) != 0)
- break;
+ xt_entry_foreach(iter, loc_cpu_entry, private->size) {
+ ADD_COUNTER(iter->counters, paddc[i].bcnt, paddc[i].pcnt);
+ ++i;
+ }
xt_info_wrunlock(curcpu);
unlock_up_free:
@@ -1510,7 +1468,7 @@ struct compat_ip6t_replace {
static int
compat_copy_entry_to_user(struct ip6t_entry *e, void __user **dstptr,
unsigned int *size, struct xt_counters *counters,
- unsigned int *i)
+ unsigned int i)
{
struct ip6t_entry_target *t;
struct compat_ip6t_entry __user *ce;
@@ -1518,14 +1476,12 @@ compat_copy_entry_to_user(struct ip6t_entry *e, void __user **dstptr,
compat_uint_t origsize;
int ret;
- ret = -EFAULT;
origsize = *size;
ce = (struct compat_ip6t_entry __user *)*dstptr;
- if (copy_to_user(ce, e, sizeof(struct ip6t_entry)))
- goto out;
-
- if (copy_to_user(&ce->counters, &counters[*i], sizeof(counters[*i])))
- goto out;
+ if (copy_to_user(ce, e, sizeof(struct ip6t_entry)) != 0 ||
+ copy_to_user(&ce->counters, &counters[i],
+ sizeof(counters[i])) != 0)
+ return -EFAULT;
*dstptr += sizeof(struct compat_ip6t_entry);
*size -= sizeof(struct ip6t_entry) - sizeof(struct compat_ip6t_entry);
@@ -1533,22 +1489,16 @@ compat_copy_entry_to_user(struct ip6t_entry *e, void __user **dstptr,
ret = IP6T_MATCH_ITERATE(e, xt_compat_match_to_user, dstptr, size);
target_offset = e->target_offset - (origsize - *size);
if (ret)
- goto out;
+ return ret;
t = ip6t_get_target(e);
ret = xt_compat_target_to_user(t, dstptr, size);
if (ret)
- goto out;
- ret = -EFAULT;
+ return ret;
next_offset = e->next_offset - (origsize - *size);
- if (put_user(target_offset, &ce->target_offset))
- goto out;
- if (put_user(next_offset, &ce->next_offset))
- goto out;
-
- (*i)++;
+ if (put_user(target_offset, &ce->target_offset) != 0 ||
+ put_user(next_offset, &ce->next_offset) != 0)
+ return -EFAULT;
return 0;
-out:
- return ret;
}
static int
@@ -1585,19 +1535,14 @@ compat_release_match(struct ip6t_entry_match *m, unsigned int *i)
return 0;
}
-static int
-compat_release_entry(struct compat_ip6t_entry *e, unsigned int *i)
+static void compat_release_entry(struct compat_ip6t_entry *e)
{
struct ip6t_entry_target *t;
- if (i && (*i)-- == 0)
- return 1;
-
/* Cleanup all matches */
COMPAT_IP6T_MATCH_ITERATE(e, compat_release_match, NULL);
t = compat_ip6t_get_target(e);
module_put(t->u.kernel.target->me);
- return 0;
}
static int
@@ -1608,7 +1553,6 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
const unsigned char *limit,
const unsigned int *hook_entries,
const unsigned int *underflows,
- unsigned int *i,
const char *name)
{
struct ip6t_entry_target *t;
@@ -1674,8 +1618,6 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
/* Clear counters and comefrom */
memset(&e->counters, 0, sizeof(e->counters));
e->comefrom = 0;
-
- (*i)++;
return 0;
out:
@@ -1725,7 +1667,7 @@ compat_copy_entry_from_user(struct compat_ip6t_entry *e, void **dstptr,
}
static int compat_check_entry(struct ip6t_entry *e, struct net *net,
- const char *name, unsigned int *i)
+ const char *name)
{
unsigned int j;
int ret;
@@ -1744,8 +1686,6 @@ static int compat_check_entry(struct ip6t_entry *e, struct net *net,
ret = check_target(e, net, name);
if (ret)
goto cleanup_matches;
-
- (*i)++;
return 0;
cleanup_matches:
@@ -1790,12 +1730,11 @@ translate_compat_table(struct net *net,
xt_entry_foreach(iter0, entry0, total_size) {
ret = check_compat_entry_size_and_hooks(iter0, info, &size,
entry0, entry0 + total_size, hook_entries, underflows,
- &j, name);
+ name);
if (ret != 0)
- break;
+ goto out_unlock;
+ ++j;
}
- if (ret != 0)
- goto out_unlock;
ret = -EINVAL;
if (j != number) {
@@ -1851,9 +1790,10 @@ translate_compat_table(struct net *net,
i = 0;
xt_entry_foreach(iter1, entry1, newinfo->size) {
- ret = compat_check_entry(iter1, net, name, &i);
+ ret = compat_check_entry(iter1, net, name);
if (ret != 0)
break;
+ ++i;
}
if (ret) {
/*
@@ -1866,12 +1806,15 @@ translate_compat_table(struct net *net,
xt_entry_foreach(iter0, entry0, newinfo->size) {
if (skip-- > 0)
continue;
- if (compat_release_entry(iter0, &j) != 0)
+ if (j-- == 0)
break;
+ compat_release_entry(iter0);
}
- xt_entry_foreach(iter1, entry1, newinfo->size)
- if (cleanup_entry(iter1, net, &i) != 0)
+ xt_entry_foreach(iter1, entry1, newinfo->size) {
+ if (i-- == 0)
break;
+ cleanup_entry(iter1, net);
+ }
xt_free_table_info(newinfo);
return ret;
}
@@ -1889,9 +1832,11 @@ translate_compat_table(struct net *net,
free_newinfo:
xt_free_table_info(newinfo);
out:
- xt_entry_foreach(iter0, entry0, total_size)
- if (compat_release_entry(iter0, &j) != 0)
+ xt_entry_foreach(iter0, entry0, total_size) {
+ if (j-- == 0)
break;
+ compat_release_entry(iter0);
+ }
return ret;
out_unlock:
xt_compat_flush_offsets(AF_INET6);
@@ -1946,8 +1891,7 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len)
free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
@@ -2011,7 +1955,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
size = total_size;
xt_entry_foreach(iter, loc_cpu_entry, total_size) {
ret = compat_copy_entry_to_user(iter, &pos,
- &size, counters, &i);
+ &size, counters, i++);
if (ret != 0)
break;
}
@@ -2221,8 +2165,7 @@ void ip6t_unregister_table(struct net *net, struct xt_table *table)
/* Decrease module usage counts and free resources */
loc_cpu_entry = private->entries[raw_smp_processor_id()];
xt_entry_foreach(iter, loc_cpu_entry, private->size)
- if (cleanup_entry(iter, net, NULL) != 0)
- break;
+ cleanup_entry(iter, net);
if (private->number > private->initial_entries)
module_put(table_owner);
xt_free_table_info(private);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/6] netfilter: xtables: replace XT_MATCH_ITERATE macro
2010-02-12 10:20 ip_tables: foreachs and reentrancy Jan Engelhardt
2010-02-12 10:20 ` [PATCH 1/6] netfilter: xtables: replace XT_ENTRY_ITERATE macro Jan Engelhardt
2010-02-12 10:20 ` [PATCH 2/6] netfilter: xtables: optimize call flow around xt_entry_foreach Jan Engelhardt
@ 2010-02-12 10:20 ` Jan Engelhardt
2010-02-12 10:20 ` [PATCH 4/6] netfilter: xtables: optimize call flow around xt_ematch_foreach Jan Engelhardt
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-12 10:20 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel
The macro is replaced by a list.h-like foreach loop. This makes
the code more inspectable.
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
include/linux/netfilter/x_tables.h | 10 +++-
include/linux/netfilter_ipv4/ip_tables.h | 6 +--
include/linux/netfilter_ipv6/ip6_tables.h | 6 +--
net/ipv4/netfilter/ip_tables.c | 78 ++++++++++++++++++++++------
net/ipv6/netfilter/ip6_tables.c | 78 ++++++++++++++++++++++------
net/netfilter/xt_TCPMSS.c | 12 +++--
6 files changed, 141 insertions(+), 49 deletions(-)
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 6a25875..26598e6 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -121,6 +121,7 @@ struct xt_counters_info {
#define XT_INV_PROTO 0x40 /* Invert the sense of PROTO. */
+#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define XT_MATCH_ITERATE(type, e, fn, args...) \
({ \
@@ -140,7 +141,6 @@ struct xt_counters_info {
__ret; \
})
-#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define XT_ENTRY_ITERATE_CONTINUE(type, entries, size, n, fn, args...) \
({ \
@@ -173,6 +173,14 @@ struct xt_counters_info {
(pos) < (typeof(pos))((char *)(ehead) + (esize)); \
(pos) = (typeof(pos))((char *)(pos) + (pos)->next_offset))
+/* can only be xt_entry_match, so no use of typeof here */
+#define xt_ematch_foreach(pos, entry) \
+ for ((pos) = (struct xt_entry_match *)entry->elems; \
+ (pos) < (struct xt_entry_match *)((char *)(entry) + \
+ (entry)->target_offset); \
+ (pos) = (struct xt_entry_match *)((char *)(pos) + \
+ (pos)->u.match_size))
+
#ifdef __KERNEL__
#include <linux/netdevice.h>
diff --git a/include/linux/netfilter_ipv4/ip_tables.h b/include/linux/netfilter_ipv4/ip_tables.h
index 5b20ae7..704a7b6 100644
--- a/include/linux/netfilter_ipv4/ip_tables.h
+++ b/include/linux/netfilter_ipv4/ip_tables.h
@@ -223,11 +223,11 @@ ipt_get_target(struct ipt_entry *e)
return (void *)e + e->target_offset;
}
+#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define IPT_MATCH_ITERATE(e, fn, args...) \
XT_MATCH_ITERATE(struct ipt_entry, e, fn, ## args)
-#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define IPT_ENTRY_ITERATE(entries, size, fn, args...) \
XT_ENTRY_ITERATE(struct ipt_entry, entries, size, fn, ## args)
@@ -315,10 +315,6 @@ compat_ipt_get_target(struct compat_ipt_entry *e)
#define COMPAT_IPT_ALIGN(s) COMPAT_XT_ALIGN(s)
-/* fn returns 0 to continue iteration */
-#define COMPAT_IPT_MATCH_ITERATE(e, fn, args...) \
- XT_MATCH_ITERATE(struct compat_ipt_entry, e, fn, ## args)
-
#endif /* CONFIG_COMPAT */
#endif /*__KERNEL__*/
#endif /* _IPTABLES_H */
diff --git a/include/linux/netfilter_ipv6/ip6_tables.h b/include/linux/netfilter_ipv6/ip6_tables.h
index 8bb3f5b..e5ba03d 100644
--- a/include/linux/netfilter_ipv6/ip6_tables.h
+++ b/include/linux/netfilter_ipv6/ip6_tables.h
@@ -280,11 +280,11 @@ ip6t_get_target(struct ip6t_entry *e)
return (void *)e + e->target_offset;
}
+#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define IP6T_MATCH_ITERATE(e, fn, args...) \
XT_MATCH_ITERATE(struct ip6t_entry, e, fn, ## args)
-#ifndef __KERNEL__
/* fn returns 0 to continue iteration */
#define IP6T_ENTRY_ITERATE(entries, size, fn, args...) \
XT_ENTRY_ITERATE(struct ip6t_entry, entries, size, fn, ## args)
@@ -343,10 +343,6 @@ compat_ip6t_get_target(struct compat_ip6t_entry *e)
#define COMPAT_IP6T_ALIGN(s) COMPAT_XT_ALIGN(s)
-/* fn returns 0 to continue iteration */
-#define COMPAT_IP6T_MATCH_ITERATE(e, fn, args...) \
- XT_MATCH_ITERATE(struct compat_ip6t_entry, e, fn, ## args)
-
#endif /* CONFIG_COMPAT */
#endif /*__KERNEL__*/
#endif /* _IP6_TABLES_H */
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 9c8aa39..3a7fc73 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -366,16 +366,21 @@ ipt_do_table(struct sk_buff *skb,
do {
const struct ipt_entry_target *t;
+ const struct xt_entry_match *ematch;
IP_NF_ASSERT(e);
IP_NF_ASSERT(back);
if (!ip_packet_match(ip, indev, outdev,
- &e->ip, mtpar.fragoff) ||
- IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0) {
+ &e->ip, mtpar.fragoff)) {
+ no_match:
e = ipt_next_entry(e);
continue;
}
+ xt_ematch_foreach(ematch, e)
+ if (do_match(ematch, skb, &mtpar) != 0)
+ goto no_match;
+
ADD_COUNTER(e->counters, ntohs(ip->tot_len), 1);
t = ipt_get_target(e);
@@ -686,6 +691,7 @@ find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
int ret;
unsigned int j;
struct xt_mtchk_param mtpar;
+ struct xt_entry_match *ematch;
ret = check_entry(e, name);
if (ret)
@@ -697,7 +703,11 @@ find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
mtpar.entryinfo = &e->ip;
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV4;
- ret = IPT_MATCH_ITERATE(e, find_check_match, &mtpar, &j);
+ xt_ematch_foreach(ematch, e) {
+ ret = find_check_match(ematch, &mtpar, &j);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
goto cleanup_matches;
@@ -720,7 +730,9 @@ find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
err:
module_put(t->u.kernel.target->me);
cleanup_matches:
- IPT_MATCH_ITERATE(e, cleanup_match, net, &j);
+ xt_ematch_foreach(ematch, e)
+ if (cleanup_match(ematch, net, &j) != 0)
+ break;
return ret;
}
@@ -791,9 +803,12 @@ cleanup_entry(struct ipt_entry *e, struct net *net)
{
struct xt_tgdtor_param par;
struct ipt_entry_target *t;
+ struct xt_entry_match *ematch;
/* Cleanup all matches */
- IPT_MATCH_ITERATE(e, cleanup_match, net, NULL);
+ xt_ematch_foreach(ematch, e)
+ if (cleanup_match(ematch, net, NULL) != 0)
+ break;
t = ipt_get_target(e);
par.net = net;
@@ -1060,13 +1075,16 @@ static int compat_calc_entry(const struct ipt_entry *e,
const struct xt_table_info *info,
const void *base, struct xt_table_info *newinfo)
{
+ const struct xt_entry_match *ematch;
const struct ipt_entry_target *t;
unsigned int entry_offset;
int off, i, ret;
off = sizeof(struct ipt_entry) - sizeof(struct compat_ipt_entry);
entry_offset = (void *)e - base;
- IPT_MATCH_ITERATE(e, compat_calc_match, &off);
+ xt_ematch_foreach(ematch, e)
+ if (compat_calc_match(ematch, &off) != 0)
+ break;
t = ipt_get_target_c(e);
off += xt_compat_target_offset(t->u.kernel.target);
newinfo->size -= off;
@@ -1441,7 +1459,8 @@ compat_copy_entry_to_user(struct ipt_entry *e, void __user **dstptr,
struct compat_ipt_entry __user *ce;
u_int16_t target_offset, next_offset;
compat_uint_t origsize;
- int ret;
+ const struct xt_entry_match *ematch;
+ int ret = 0;
origsize = *size;
ce = (struct compat_ipt_entry __user *)*dstptr;
@@ -1453,7 +1472,11 @@ compat_copy_entry_to_user(struct ipt_entry *e, void __user **dstptr,
*dstptr += sizeof(struct compat_ipt_entry);
*size -= sizeof(struct ipt_entry) - sizeof(struct compat_ipt_entry);
- ret = IPT_MATCH_ITERATE(e, xt_compat_match_to_user, dstptr, size);
+ xt_ematch_foreach(ematch, e) {
+ ret = xt_compat_match_to_user(ematch, dstptr, size);
+ if (ret != 0)
+ break;
+ }
target_offset = e->target_offset - (origsize - *size);
if (ret)
return ret;
@@ -1505,9 +1528,12 @@ compat_release_match(struct ipt_entry_match *m, unsigned int *i)
static void compat_release_entry(struct compat_ipt_entry *e)
{
struct ipt_entry_target *t;
+ struct xt_entry_match *ematch;
/* Cleanup all matches */
- COMPAT_IPT_MATCH_ITERATE(e, compat_release_match, NULL);
+ xt_ematch_foreach(ematch, e)
+ if (compat_release_match(ematch, NULL) != 0)
+ break;
t = compat_ipt_get_target(e);
module_put(t->u.kernel.target->me);
}
@@ -1522,6 +1548,7 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
const unsigned int *underflows,
const char *name)
{
+ struct xt_entry_match *ematch;
struct ipt_entry_target *t;
struct xt_target *target;
unsigned int entry_offset;
@@ -1550,8 +1577,12 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
off = sizeof(struct ipt_entry) - sizeof(struct compat_ipt_entry);
entry_offset = (void *)e - (void *)base;
j = 0;
- ret = COMPAT_IPT_MATCH_ITERATE(e, compat_find_calc_match, name,
- &e->ip, e->comefrom, &off, &j);
+ xt_ematch_foreach(ematch, e) {
+ ret = compat_find_calc_match(ematch, name,
+ &e->ip, e->comefrom, &off, &j);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
goto release_matches;
@@ -1590,7 +1621,9 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
out:
module_put(t->u.kernel.target->me);
release_matches:
- IPT_MATCH_ITERATE(e, compat_release_match, &j);
+ xt_ematch_foreach(ematch, e)
+ if (compat_release_match(ematch, &j) != 0)
+ break;
return ret;
}
@@ -1604,6 +1637,7 @@ compat_copy_entry_from_user(struct compat_ipt_entry *e, void **dstptr,
struct ipt_entry *de;
unsigned int origsize;
int ret, h;
+ struct xt_entry_match *ematch;
ret = 0;
origsize = *size;
@@ -1614,8 +1648,11 @@ compat_copy_entry_from_user(struct compat_ipt_entry *e, void **dstptr,
*dstptr += sizeof(struct ipt_entry);
*size += sizeof(struct ipt_entry) - sizeof(struct compat_ipt_entry);
- ret = COMPAT_IPT_MATCH_ITERATE(e, xt_compat_match_from_user,
- dstptr, size);
+ xt_ematch_foreach(ematch, e) {
+ ret = xt_compat_match_from_user(ematch, dstptr, size);
+ if (ret != 0)
+ break;
+ }
if (ret)
return ret;
de->target_offset = e->target_offset - (origsize - *size);
@@ -1636,9 +1673,10 @@ compat_copy_entry_from_user(struct compat_ipt_entry *e, void **dstptr,
static int
compat_check_entry(struct ipt_entry *e, struct net *net, const char *name)
{
+ struct xt_entry_match *ematch;
struct xt_mtchk_param mtpar;
unsigned int j;
- int ret;
+ int ret = 0;
j = 0;
mtpar.net = net;
@@ -1646,7 +1684,11 @@ compat_check_entry(struct ipt_entry *e, struct net *net, const char *name)
mtpar.entryinfo = &e->ip;
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV4;
- ret = IPT_MATCH_ITERATE(e, check_match, &mtpar, &j);
+ xt_ematch_foreach(ematch, e) {
+ ret = check_match(ematch, &mtpar, &j);
+ if (ret != 0)
+ break;
+ }
if (ret)
goto cleanup_matches;
@@ -1656,7 +1698,9 @@ compat_check_entry(struct ipt_entry *e, struct net *net, const char *name)
return 0;
cleanup_matches:
- IPT_MATCH_ITERATE(e, cleanup_match, net, &j);
+ xt_ematch_foreach(ematch, e)
+ if (cleanup_match(ematch, net, &j) != 0)
+ break;
return ret;
}
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index b7e27c1..1537e6b 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -393,16 +393,21 @@ ip6t_do_table(struct sk_buff *skb,
do {
const struct ip6t_entry_target *t;
+ const struct xt_entry_match *ematch;
IP_NF_ASSERT(e);
IP_NF_ASSERT(back);
if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
- &mtpar.thoff, &mtpar.fragoff, &hotdrop) ||
- IP6T_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0) {
+ &mtpar.thoff, &mtpar.fragoff, &hotdrop)) {
+ no_match:
e = ip6t_next_entry(e);
continue;
}
+ xt_ematch_foreach(ematch, e)
+ if (do_match(ematch, skb, &mtpar) != 0)
+ goto no_match;
+
ADD_COUNTER(e->counters,
ntohs(ipv6_hdr(skb)->payload_len) +
sizeof(struct ipv6hdr), 1);
@@ -717,6 +722,7 @@ find_check_entry(struct ip6t_entry *e, struct net *net, const char *name,
int ret;
unsigned int j;
struct xt_mtchk_param mtpar;
+ struct xt_entry_match *ematch;
ret = check_entry(e, name);
if (ret)
@@ -728,7 +734,11 @@ find_check_entry(struct ip6t_entry *e, struct net *net, const char *name,
mtpar.entryinfo = &e->ipv6;
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV6;
- ret = IP6T_MATCH_ITERATE(e, find_check_match, &mtpar, &j);
+ xt_ematch_foreach(ematch, e) {
+ ret = find_check_match(ematch, &mtpar, &j);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
goto cleanup_matches;
@@ -751,7 +761,9 @@ find_check_entry(struct ip6t_entry *e, struct net *net, const char *name,
err:
module_put(t->u.kernel.target->me);
cleanup_matches:
- IP6T_MATCH_ITERATE(e, cleanup_match, net, &j);
+ xt_ematch_foreach(ematch, e)
+ if (cleanup_match(ematch, net, &j) != 0)
+ break;
return ret;
}
@@ -821,9 +833,12 @@ static void cleanup_entry(struct ip6t_entry *e, struct net *net)
{
struct xt_tgdtor_param par;
struct ip6t_entry_target *t;
+ struct xt_entry_match *ematch;
/* Cleanup all matches */
- IP6T_MATCH_ITERATE(e, cleanup_match, net, NULL);
+ xt_ematch_foreach(ematch, e)
+ if (cleanup_match(ematch, net, NULL) != 0)
+ break;
t = ip6t_get_target(e);
par.net = net;
@@ -1090,13 +1105,16 @@ static int compat_calc_entry(const struct ip6t_entry *e,
const struct xt_table_info *info,
const void *base, struct xt_table_info *newinfo)
{
+ const struct xt_entry_match *ematch;
const struct ip6t_entry_target *t;
unsigned int entry_offset;
int off, i, ret;
off = sizeof(struct ip6t_entry) - sizeof(struct compat_ip6t_entry);
entry_offset = (void *)e - base;
- IP6T_MATCH_ITERATE(e, compat_calc_match, &off);
+ xt_ematch_foreach(ematch, e)
+ if (compat_calc_match(ematch, &off) != 0)
+ break;
t = ip6t_get_target_c(e);
off += xt_compat_target_offset(t->u.kernel.target);
newinfo->size -= off;
@@ -1474,7 +1492,8 @@ compat_copy_entry_to_user(struct ip6t_entry *e, void __user **dstptr,
struct compat_ip6t_entry __user *ce;
u_int16_t target_offset, next_offset;
compat_uint_t origsize;
- int ret;
+ const struct xt_entry_match *ematch;
+ int ret = 0;
origsize = *size;
ce = (struct compat_ip6t_entry __user *)*dstptr;
@@ -1486,7 +1505,11 @@ compat_copy_entry_to_user(struct ip6t_entry *e, void __user **dstptr,
*dstptr += sizeof(struct compat_ip6t_entry);
*size -= sizeof(struct ip6t_entry) - sizeof(struct compat_ip6t_entry);
- ret = IP6T_MATCH_ITERATE(e, xt_compat_match_to_user, dstptr, size);
+ xt_ematch_foreach(ematch, e) {
+ ret = xt_compat_match_to_user(ematch, dstptr, size);
+ if (ret != 0)
+ break;
+ }
target_offset = e->target_offset - (origsize - *size);
if (ret)
return ret;
@@ -1538,9 +1561,12 @@ compat_release_match(struct ip6t_entry_match *m, unsigned int *i)
static void compat_release_entry(struct compat_ip6t_entry *e)
{
struct ip6t_entry_target *t;
+ struct xt_entry_match *ematch;
/* Cleanup all matches */
- COMPAT_IP6T_MATCH_ITERATE(e, compat_release_match, NULL);
+ xt_ematch_foreach(ematch, e)
+ if (compat_release_match(ematch, NULL) != 0)
+ break;
t = compat_ip6t_get_target(e);
module_put(t->u.kernel.target->me);
}
@@ -1555,6 +1581,7 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
const unsigned int *underflows,
const char *name)
{
+ struct xt_entry_match *ematch;
struct ip6t_entry_target *t;
struct xt_target *target;
unsigned int entry_offset;
@@ -1583,8 +1610,12 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
off = sizeof(struct ip6t_entry) - sizeof(struct compat_ip6t_entry);
entry_offset = (void *)e - (void *)base;
j = 0;
- ret = COMPAT_IP6T_MATCH_ITERATE(e, compat_find_calc_match, name,
- &e->ipv6, e->comefrom, &off, &j);
+ xt_ematch_foreach(ematch, e) {
+ ret = compat_find_calc_match(ematch, name,
+ &e->ipv6, e->comefrom, &off, &j);
+ if (ret != 0)
+ break;
+ }
if (ret != 0)
goto release_matches;
@@ -1623,7 +1654,9 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
out:
module_put(t->u.kernel.target->me);
release_matches:
- IP6T_MATCH_ITERATE(e, compat_release_match, &j);
+ xt_ematch_foreach(ematch, e)
+ if (compat_release_match(ematch, &j) != 0)
+ break;
return ret;
}
@@ -1637,6 +1670,7 @@ compat_copy_entry_from_user(struct compat_ip6t_entry *e, void **dstptr,
struct ip6t_entry *de;
unsigned int origsize;
int ret, h;
+ struct xt_entry_match *ematch;
ret = 0;
origsize = *size;
@@ -1647,8 +1681,11 @@ compat_copy_entry_from_user(struct compat_ip6t_entry *e, void **dstptr,
*dstptr += sizeof(struct ip6t_entry);
*size += sizeof(struct ip6t_entry) - sizeof(struct compat_ip6t_entry);
- ret = COMPAT_IP6T_MATCH_ITERATE(e, xt_compat_match_from_user,
- dstptr, size);
+ xt_ematch_foreach(ematch, e) {
+ ret = xt_compat_match_from_user(ematch, dstptr, size);
+ if (ret != 0)
+ break;
+ }
if (ret)
return ret;
de->target_offset = e->target_offset - (origsize - *size);
@@ -1670,8 +1707,9 @@ static int compat_check_entry(struct ip6t_entry *e, struct net *net,
const char *name)
{
unsigned int j;
- int ret;
+ int ret = 0;
struct xt_mtchk_param mtpar;
+ struct xt_entry_match *ematch;
j = 0;
mtpar.net = net;
@@ -1679,7 +1717,11 @@ static int compat_check_entry(struct ip6t_entry *e, struct net *net,
mtpar.entryinfo = &e->ipv6;
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV6;
- ret = IP6T_MATCH_ITERATE(e, check_match, &mtpar, &j);
+ xt_ematch_foreach(ematch, e) {
+ ret = check_match(ematch, &mtpar, &j);
+ if (ret != 0)
+ break;
+ }
if (ret)
goto cleanup_matches;
@@ -1689,7 +1731,9 @@ static int compat_check_entry(struct ip6t_entry *e, struct net *net,
return 0;
cleanup_matches:
- IP6T_MATCH_ITERATE(e, cleanup_match, net, &j);
+ xt_ematch_foreach(ematch, e)
+ if (cleanup_match(ematch, net, &j) != 0)
+ break;
return ret;
}
diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 6f21b43..0e357ac 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -239,6 +239,7 @@ static bool tcpmss_tg4_check(const struct xt_tgchk_param *par)
{
const struct xt_tcpmss_info *info = par->targinfo;
const struct ipt_entry *e = par->entryinfo;
+ const struct xt_entry_match *ematch;
if (info->mss == XT_TCPMSS_CLAMP_PMTU &&
(par->hook_mask & ~((1 << NF_INET_FORWARD) |
@@ -248,8 +249,9 @@ static bool tcpmss_tg4_check(const struct xt_tgchk_param *par)
"FORWARD, OUTPUT and POSTROUTING hooks\n");
return false;
}
- if (IPT_MATCH_ITERATE(e, find_syn_match))
- return true;
+ xt_ematch_foreach(ematch, e)
+ if (find_syn_match(ematch))
+ return true;
printk("xt_TCPMSS: Only works on TCP SYN packets\n");
return false;
}
@@ -259,6 +261,7 @@ static bool tcpmss_tg6_check(const struct xt_tgchk_param *par)
{
const struct xt_tcpmss_info *info = par->targinfo;
const struct ip6t_entry *e = par->entryinfo;
+ const struct xt_entry_match *ematch;
if (info->mss == XT_TCPMSS_CLAMP_PMTU &&
(par->hook_mask & ~((1 << NF_INET_FORWARD) |
@@ -268,8 +271,9 @@ static bool tcpmss_tg6_check(const struct xt_tgchk_param *par)
"FORWARD, OUTPUT and POSTROUTING hooks\n");
return false;
}
- if (IP6T_MATCH_ITERATE(e, find_syn_match))
- return true;
+ xt_ematch_foreach(ematch, e)
+ if (find_syn_match(ematch))
+ return true;
printk("xt_TCPMSS: Only works on TCP SYN packets\n");
return false;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/6] netfilter: xtables: optimize call flow around xt_ematch_foreach
2010-02-12 10:20 ip_tables: foreachs and reentrancy Jan Engelhardt
` (2 preceding siblings ...)
2010-02-12 10:20 ` [PATCH 3/6] netfilter: xtables: replace XT_MATCH_ITERATE macro Jan Engelhardt
@ 2010-02-12 10:20 ` Jan Engelhardt
2010-02-12 10:20 ` [PATCH 5/6] netfilter: xtables: reduce arguments to translate_table Jan Engelhardt
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-12 10:20 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
net/ipv4/netfilter/ip_tables.c | 93 +++++++++++++--------------------------
net/ipv6/netfilter/ip6_tables.c | 93 +++++++++++++--------------------------
2 files changed, 62 insertions(+), 124 deletions(-)
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 3a7fc73..36edc7d 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -572,14 +572,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
return 1;
}
-static int
-cleanup_match(struct ipt_entry_match *m, struct net *net, unsigned int *i)
+static void cleanup_match(struct ipt_entry_match *m, struct net *net)
{
struct xt_mtdtor_param par;
- if (i && (*i)-- == 0)
- return 1;
-
par.net = net;
par.match = m->u.kernel.match;
par.matchinfo = m->data;
@@ -587,7 +583,6 @@ cleanup_match(struct ipt_entry_match *m, struct net *net, unsigned int *i)
if (par.match->destroy != NULL)
par.match->destroy(&par);
module_put(par.match->me);
- return 0;
}
static int
@@ -612,8 +607,7 @@ check_entry(const struct ipt_entry *e, const char *name)
}
static int
-check_match(struct ipt_entry_match *m, struct xt_mtchk_param *par,
- unsigned int *i)
+check_match(struct ipt_entry_match *m, struct xt_mtchk_param *par)
{
const struct ipt_ip *ip = par->entryinfo;
int ret;
@@ -628,13 +622,11 @@ check_match(struct ipt_entry_match *m, struct xt_mtchk_param *par,
par.match->name);
return ret;
}
- ++*i;
return 0;
}
static int
-find_check_match(struct ipt_entry_match *m, struct xt_mtchk_param *par,
- unsigned int *i)
+find_check_match(struct ipt_entry_match *m, struct xt_mtchk_param *par)
{
struct xt_match *match;
int ret;
@@ -648,7 +640,7 @@ find_check_match(struct ipt_entry_match *m, struct xt_mtchk_param *par,
}
m->u.kernel.match = match;
- ret = check_match(m, par, i);
+ ret = check_match(m, par);
if (ret)
goto err;
@@ -704,12 +696,11 @@ find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV4;
xt_ematch_foreach(ematch, e) {
- ret = find_check_match(ematch, &mtpar, &j);
+ ret = find_check_match(ematch, &mtpar);
if (ret != 0)
- break;
+ goto cleanup_matches;
+ ++j;
}
- if (ret != 0)
- goto cleanup_matches;
t = ipt_get_target(e);
target = try_then_request_module(xt_find_target(AF_INET,
@@ -730,9 +721,11 @@ find_check_entry(struct ipt_entry *e, struct net *net, const char *name,
err:
module_put(t->u.kernel.target->me);
cleanup_matches:
- xt_ematch_foreach(ematch, e)
- if (cleanup_match(ematch, net, &j) != 0)
+ xt_ematch_foreach(ematch, e) {
+ if (j-- == 0)
break;
+ cleanup_match(ematch, net);
+ }
return ret;
}
@@ -807,8 +800,7 @@ cleanup_entry(struct ipt_entry *e, struct net *net)
/* Cleanup all matches */
xt_ematch_foreach(ematch, e)
- if (cleanup_match(ematch, net, NULL) != 0)
- break;
+ cleanup_match(ematch, net);
t = ipt_get_target(e);
par.net = net;
@@ -1064,13 +1056,6 @@ static int compat_standard_to_user(void __user *dst, const void *src)
return copy_to_user(dst, &cv, sizeof(cv)) ? -EFAULT : 0;
}
-static inline int
-compat_calc_match(const struct ipt_entry_match *m, int *size)
-{
- *size += xt_compat_match_offset(m->u.kernel.match);
- return 0;
-}
-
static int compat_calc_entry(const struct ipt_entry *e,
const struct xt_table_info *info,
const void *base, struct xt_table_info *newinfo)
@@ -1083,8 +1068,7 @@ static int compat_calc_entry(const struct ipt_entry *e,
off = sizeof(struct ipt_entry) - sizeof(struct compat_ipt_entry);
entry_offset = (void *)e - base;
xt_ematch_foreach(ematch, e)
- if (compat_calc_match(ematch, &off) != 0)
- break;
+ off += xt_compat_match_offset(ematch->u.kernel.match);
t = ipt_get_target_c(e);
off += xt_compat_target_offset(t->u.kernel.target);
newinfo->size -= off;
@@ -1475,11 +1459,9 @@ compat_copy_entry_to_user(struct ipt_entry *e, void __user **dstptr,
xt_ematch_foreach(ematch, e) {
ret = xt_compat_match_to_user(ematch, dstptr, size);
if (ret != 0)
- break;
+ return ret;
}
target_offset = e->target_offset - (origsize - *size);
- if (ret)
- return ret;
t = ipt_get_target(e);
ret = xt_compat_target_to_user(t, dstptr, size);
if (ret)
@@ -1496,7 +1478,7 @@ compat_find_calc_match(struct ipt_entry_match *m,
const char *name,
const struct ipt_ip *ip,
unsigned int hookmask,
- int *size, unsigned int *i)
+ int *size)
{
struct xt_match *match;
@@ -1510,18 +1492,6 @@ compat_find_calc_match(struct ipt_entry_match *m,
}
m->u.kernel.match = match;
*size += xt_compat_match_offset(match);
-
- (*i)++;
- return 0;
-}
-
-static int
-compat_release_match(struct ipt_entry_match *m, unsigned int *i)
-{
- if (i && (*i)-- == 0)
- return 1;
-
- module_put(m->u.kernel.match->me);
return 0;
}
@@ -1532,8 +1502,7 @@ static void compat_release_entry(struct compat_ipt_entry *e)
/* Cleanup all matches */
xt_ematch_foreach(ematch, e)
- if (compat_release_match(ematch, NULL) != 0)
- break;
+ module_put(ematch->u.kernel.match->me);
t = compat_ipt_get_target(e);
module_put(t->u.kernel.target->me);
}
@@ -1579,12 +1548,11 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
j = 0;
xt_ematch_foreach(ematch, e) {
ret = compat_find_calc_match(ematch, name,
- &e->ip, e->comefrom, &off, &j);
+ &e->ip, e->comefrom, &off);
if (ret != 0)
- break;
+ goto release_matches;
+ ++j;
}
- if (ret != 0)
- goto release_matches;
t = compat_ipt_get_target(e);
target = try_then_request_module(xt_find_target(AF_INET,
@@ -1621,9 +1589,11 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
out:
module_put(t->u.kernel.target->me);
release_matches:
- xt_ematch_foreach(ematch, e)
- if (compat_release_match(ematch, &j) != 0)
+ xt_ematch_foreach(ematch, e) {
+ if (j-- == 0)
break;
+ module_put(ematch->u.kernel.match->me);
+ }
return ret;
}
@@ -1651,10 +1621,8 @@ compat_copy_entry_from_user(struct compat_ipt_entry *e, void **dstptr,
xt_ematch_foreach(ematch, e) {
ret = xt_compat_match_from_user(ematch, dstptr, size);
if (ret != 0)
- break;
+ return ret;
}
- if (ret)
- return ret;
de->target_offset = e->target_offset - (origsize - *size);
t = compat_ipt_get_target(e);
target = t->u.kernel.target;
@@ -1685,12 +1653,11 @@ compat_check_entry(struct ipt_entry *e, struct net *net, const char *name)
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV4;
xt_ematch_foreach(ematch, e) {
- ret = check_match(ematch, &mtpar, &j);
+ ret = check_match(ematch, &mtpar);
if (ret != 0)
- break;
+ goto cleanup_matches;
+ ++j;
}
- if (ret)
- goto cleanup_matches;
ret = check_target(e, net, name);
if (ret)
@@ -1698,9 +1665,11 @@ compat_check_entry(struct ipt_entry *e, struct net *net, const char *name)
return 0;
cleanup_matches:
- xt_ematch_foreach(ematch, e)
- if (cleanup_match(ematch, net, &j) != 0)
+ xt_ematch_foreach(ematch, e) {
+ if (j-- == 0)
break;
+ cleanup_match(ematch, net);
+ }
return ret;
}
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 1537e6b..c5a963e 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -603,14 +603,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
return 1;
}
-static int
-cleanup_match(struct ip6t_entry_match *m, struct net *net, unsigned int *i)
+static void cleanup_match(struct ip6t_entry_match *m, struct net *net)
{
struct xt_mtdtor_param par;
- if (i && (*i)-- == 0)
- return 1;
-
par.net = net;
par.match = m->u.kernel.match;
par.matchinfo = m->data;
@@ -618,7 +614,6 @@ cleanup_match(struct ip6t_entry_match *m, struct net *net, unsigned int *i)
if (par.match->destroy != NULL)
par.match->destroy(&par);
module_put(par.match->me);
- return 0;
}
static int
@@ -642,8 +637,7 @@ check_entry(const struct ip6t_entry *e, const char *name)
return 0;
}
-static int check_match(struct ip6t_entry_match *m, struct xt_mtchk_param *par,
- unsigned int *i)
+static int check_match(struct ip6t_entry_match *m, struct xt_mtchk_param *par)
{
const struct ip6t_ip6 *ipv6 = par->entryinfo;
int ret;
@@ -658,13 +652,11 @@ static int check_match(struct ip6t_entry_match *m, struct xt_mtchk_param *par,
par.match->name);
return ret;
}
- ++*i;
return 0;
}
static int
-find_check_match(struct ip6t_entry_match *m, struct xt_mtchk_param *par,
- unsigned int *i)
+find_check_match(struct ip6t_entry_match *m, struct xt_mtchk_param *par)
{
struct xt_match *match;
int ret;
@@ -678,7 +670,7 @@ find_check_match(struct ip6t_entry_match *m, struct xt_mtchk_param *par,
}
m->u.kernel.match = match;
- ret = check_match(m, par, i);
+ ret = check_match(m, par);
if (ret)
goto err;
@@ -735,12 +727,11 @@ find_check_entry(struct ip6t_entry *e, struct net *net, const char *name,
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV6;
xt_ematch_foreach(ematch, e) {
- ret = find_check_match(ematch, &mtpar, &j);
+ ret = find_check_match(ematch, &mtpar);
if (ret != 0)
- break;
+ goto cleanup_matches;
+ ++j;
}
- if (ret != 0)
- goto cleanup_matches;
t = ip6t_get_target(e);
target = try_then_request_module(xt_find_target(AF_INET6,
@@ -761,9 +752,11 @@ find_check_entry(struct ip6t_entry *e, struct net *net, const char *name,
err:
module_put(t->u.kernel.target->me);
cleanup_matches:
- xt_ematch_foreach(ematch, e)
- if (cleanup_match(ematch, net, &j) != 0)
+ xt_ematch_foreach(ematch, e) {
+ if (j-- == 0)
break;
+ cleanup_match(ematch, net);
+ }
return ret;
}
@@ -837,8 +830,7 @@ static void cleanup_entry(struct ip6t_entry *e, struct net *net)
/* Cleanup all matches */
xt_ematch_foreach(ematch, e)
- if (cleanup_match(ematch, net, NULL) != 0)
- break;
+ cleanup_match(ematch, net);
t = ip6t_get_target(e);
par.net = net;
@@ -1094,13 +1086,6 @@ static int compat_standard_to_user(void __user *dst, const void *src)
return copy_to_user(dst, &cv, sizeof(cv)) ? -EFAULT : 0;
}
-static inline int
-compat_calc_match(const struct ip6t_entry_match *m, int *size)
-{
- *size += xt_compat_match_offset(m->u.kernel.match);
- return 0;
-}
-
static int compat_calc_entry(const struct ip6t_entry *e,
const struct xt_table_info *info,
const void *base, struct xt_table_info *newinfo)
@@ -1113,8 +1098,7 @@ static int compat_calc_entry(const struct ip6t_entry *e,
off = sizeof(struct ip6t_entry) - sizeof(struct compat_ip6t_entry);
entry_offset = (void *)e - base;
xt_ematch_foreach(ematch, e)
- if (compat_calc_match(ematch, &off) != 0)
- break;
+ off += xt_compat_match_offset(ematch->u.kernel.match);
t = ip6t_get_target_c(e);
off += xt_compat_target_offset(t->u.kernel.target);
newinfo->size -= off;
@@ -1508,11 +1492,9 @@ compat_copy_entry_to_user(struct ip6t_entry *e, void __user **dstptr,
xt_ematch_foreach(ematch, e) {
ret = xt_compat_match_to_user(ematch, dstptr, size);
if (ret != 0)
- break;
+ return ret;
}
target_offset = e->target_offset - (origsize - *size);
- if (ret)
- return ret;
t = ip6t_get_target(e);
ret = xt_compat_target_to_user(t, dstptr, size);
if (ret)
@@ -1529,7 +1511,7 @@ compat_find_calc_match(struct ip6t_entry_match *m,
const char *name,
const struct ip6t_ip6 *ipv6,
unsigned int hookmask,
- int *size, unsigned int *i)
+ int *size)
{
struct xt_match *match;
@@ -1543,18 +1525,6 @@ compat_find_calc_match(struct ip6t_entry_match *m,
}
m->u.kernel.match = match;
*size += xt_compat_match_offset(match);
-
- (*i)++;
- return 0;
-}
-
-static int
-compat_release_match(struct ip6t_entry_match *m, unsigned int *i)
-{
- if (i && (*i)-- == 0)
- return 1;
-
- module_put(m->u.kernel.match->me);
return 0;
}
@@ -1565,8 +1535,7 @@ static void compat_release_entry(struct compat_ip6t_entry *e)
/* Cleanup all matches */
xt_ematch_foreach(ematch, e)
- if (compat_release_match(ematch, NULL) != 0)
- break;
+ module_put(ematch->u.kernel.match->me);
t = compat_ip6t_get_target(e);
module_put(t->u.kernel.target->me);
}
@@ -1612,12 +1581,11 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
j = 0;
xt_ematch_foreach(ematch, e) {
ret = compat_find_calc_match(ematch, name,
- &e->ipv6, e->comefrom, &off, &j);
+ &e->ipv6, e->comefrom, &off);
if (ret != 0)
- break;
+ goto release_matches;
+ ++j;
}
- if (ret != 0)
- goto release_matches;
t = compat_ip6t_get_target(e);
target = try_then_request_module(xt_find_target(AF_INET6,
@@ -1654,9 +1622,11 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
out:
module_put(t->u.kernel.target->me);
release_matches:
- xt_ematch_foreach(ematch, e)
- if (compat_release_match(ematch, &j) != 0)
+ xt_ematch_foreach(ematch, e) {
+ if (j-- == 0)
break;
+ module_put(ematch->u.kernel.match->me);
+ }
return ret;
}
@@ -1684,10 +1654,8 @@ compat_copy_entry_from_user(struct compat_ip6t_entry *e, void **dstptr,
xt_ematch_foreach(ematch, e) {
ret = xt_compat_match_from_user(ematch, dstptr, size);
if (ret != 0)
- break;
+ return ret;
}
- if (ret)
- return ret;
de->target_offset = e->target_offset - (origsize - *size);
t = compat_ip6t_get_target(e);
target = t->u.kernel.target;
@@ -1718,12 +1686,11 @@ static int compat_check_entry(struct ip6t_entry *e, struct net *net,
mtpar.hook_mask = e->comefrom;
mtpar.family = NFPROTO_IPV6;
xt_ematch_foreach(ematch, e) {
- ret = check_match(ematch, &mtpar, &j);
+ ret = check_match(ematch, &mtpar);
if (ret != 0)
- break;
+ goto cleanup_matches;
+ ++j;
}
- if (ret)
- goto cleanup_matches;
ret = check_target(e, net, name);
if (ret)
@@ -1731,9 +1698,11 @@ static int compat_check_entry(struct ip6t_entry *e, struct net *net,
return 0;
cleanup_matches:
- xt_ematch_foreach(ematch, e)
- if (cleanup_match(ematch, net, &j) != 0)
+ xt_ematch_foreach(ematch, e) {
+ if (j-- == 0)
break;
+ cleanup_match(ematch, net);
+ }
return ret;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/6] netfilter: xtables: reduce arguments to translate_table
2010-02-12 10:20 ip_tables: foreachs and reentrancy Jan Engelhardt
` (3 preceding siblings ...)
2010-02-12 10:20 ` [PATCH 4/6] netfilter: xtables: optimize call flow around xt_ematch_foreach Jan Engelhardt
@ 2010-02-12 10:20 ` Jan Engelhardt
2010-02-12 10:20 ` [PATCH 6/6] netfilter: xtables2: make ip_tables reentrant Jan Engelhardt
2010-02-24 17:40 ` ip_tables: foreachs and reentrancy Patrick McHardy
6 siblings, 0 replies; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-12 10:20 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel
Just pass in the entire repl struct. In case of a new table (e.g.
ip6t_register_table), the repldata has been previously filled with
table->name and table->size already (in ip6t_alloc_initial_table).
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
net/ipv4/netfilter/arp_tables.c | 42 +++++++++++++-------------------------
net/ipv4/netfilter/ip_tables.c | 42 +++++++++++++-------------------------
net/ipv6/netfilter/ip6_tables.c | 42 +++++++++++++-------------------------
3 files changed, 45 insertions(+), 81 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 5fdedeb..57098dc 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -622,21 +622,15 @@ static inline void cleanup_entry(struct arpt_entry *e)
/* Checks and translates the user-supplied table segment (held in
* newinfo).
*/
-static int translate_table(const char *name,
- unsigned int valid_hooks,
- struct xt_table_info *newinfo,
- void *entry0,
- unsigned int size,
- unsigned int number,
- const unsigned int *hook_entries,
- const unsigned int *underflows)
+static int translate_table(struct xt_table_info *newinfo, void *entry0,
+ const struct arpt_replace *repl)
{
struct arpt_entry *iter;
unsigned int i;
int ret = 0;
- newinfo->size = size;
- newinfo->number = number;
+ newinfo->size = repl->size;
+ newinfo->number = repl->num_entries;
/* Init all hooks to impossible value. */
for (i = 0; i < NF_ARP_NUMHOOKS; i++) {
@@ -650,7 +644,8 @@ static int translate_table(const char *name,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + size, hook_entries, underflows, valid_hooks);
+ entry0 + repl->size, repl->hook_entry, repl->underflow,
+ repl->valid_hooks);
if (ret != 0)
break;
++i;
@@ -659,30 +654,30 @@ static int translate_table(const char *name,
if (ret != 0)
return ret;
- if (i != number) {
+ if (i != repl->num_entries) {
duprintf("translate_table: %u not %u entries\n",
- i, number);
+ i, repl->num_entries);
return -EINVAL;
}
/* Check hooks all assigned */
for (i = 0; i < NF_ARP_NUMHOOKS; i++) {
/* Only hooks which are valid */
- if (!(valid_hooks & (1 << i)))
+ if (!(repl->valid_hooks & (1 << i)))
continue;
if (newinfo->hook_entry[i] == 0xFFFFFFFF) {
duprintf("Invalid hook entry %u %u\n",
- i, hook_entries[i]);
+ i, repl->hook_entry[i]);
return -EINVAL;
}
if (newinfo->underflow[i] == 0xFFFFFFFF) {
duprintf("Invalid underflow %u %u\n",
- i, underflows[i]);
+ i, repl->underflow[i]);
return -EINVAL;
}
}
- if (!mark_source_chains(newinfo, valid_hooks, entry0)) {
+ if (!mark_source_chains(newinfo, repl->valid_hooks, entry0)) {
duprintf("Looping hook\n");
return -ELOOP;
}
@@ -690,7 +685,7 @@ static int translate_table(const char *name,
/* Finally, each sanity check must pass */
i = 0;
xt_entry_foreach(iter, entry0, newinfo->size) {
- ret = find_check_entry(iter, name, size);
+ ret = find_check_entry(iter, repl->name, repl->size);
if (ret != 0)
break;
++i;
@@ -1101,9 +1096,7 @@ static int do_replace(struct net *net, const void __user *user,
goto free_newinfo;
}
- ret = translate_table(tmp.name, tmp.valid_hooks,
- newinfo, loc_cpu_entry, tmp.size, tmp.num_entries,
- tmp.hook_entry, tmp.underflow);
+ ret = translate_table(newinfo, loc_cpu_entry, &tmp);
if (ret != 0)
goto free_newinfo;
@@ -1795,12 +1788,7 @@ struct xt_table *arpt_register_table(struct net *net,
loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
memcpy(loc_cpu_entry, repl->entries, repl->size);
- ret = translate_table(table->name, table->valid_hooks,
- newinfo, loc_cpu_entry, repl->size,
- repl->num_entries,
- repl->hook_entry,
- repl->underflow);
-
+ ret = translate_table(newinfo, loc_cpu_entry, repl);
duprintf("arpt_register_table: translate table gives %d\n", ret);
if (ret != 0)
goto out_free;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 36edc7d..c92f4e5 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -815,22 +815,15 @@ cleanup_entry(struct ipt_entry *e, struct net *net)
/* Checks and translates the user-supplied table segment (held in
newinfo) */
static int
-translate_table(struct net *net,
- const char *name,
- unsigned int valid_hooks,
- struct xt_table_info *newinfo,
- void *entry0,
- unsigned int size,
- unsigned int number,
- const unsigned int *hook_entries,
- const unsigned int *underflows)
+translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
+ const struct ipt_replace *repl)
{
struct ipt_entry *iter;
unsigned int i;
int ret = 0;
- newinfo->size = size;
- newinfo->number = number;
+ newinfo->size = repl->size;
+ newinfo->number = repl->num_entries;
/* Init all hooks to impossible value. */
for (i = 0; i < NF_INET_NUMHOOKS; i++) {
@@ -843,42 +836,43 @@ translate_table(struct net *net,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + size, hook_entries, underflows, valid_hooks);
+ entry0 + repl->size, repl->hook_entry, repl->underflow,
+ repl->valid_hooks);
if (ret != 0)
return ret;
++i;
}
- if (i != number) {
+ if (i != repl->num_entries) {
duprintf("translate_table: %u not %u entries\n",
- i, number);
+ i, repl->num_entries);
return -EINVAL;
}
/* Check hooks all assigned */
for (i = 0; i < NF_INET_NUMHOOKS; i++) {
/* Only hooks which are valid */
- if (!(valid_hooks & (1 << i)))
+ if (!(repl->valid_hooks & (1 << i)))
continue;
if (newinfo->hook_entry[i] == 0xFFFFFFFF) {
duprintf("Invalid hook entry %u %u\n",
- i, hook_entries[i]);
+ i, repl->hook_entry[i]);
return -EINVAL;
}
if (newinfo->underflow[i] == 0xFFFFFFFF) {
duprintf("Invalid underflow %u %u\n",
- i, underflows[i]);
+ i, repl->underflow[i]);
return -EINVAL;
}
}
- if (!mark_source_chains(newinfo, valid_hooks, entry0))
+ if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
return -ELOOP;
/* Finally, each sanity check must pass */
i = 0;
xt_entry_foreach(iter, entry0, newinfo->size) {
- ret = find_check_entry(iter, net, name, size);
+ ret = find_check_entry(iter, net, repl->name, repl->size);
if (ret != 0)
break;
++i;
@@ -1311,9 +1305,7 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
goto free_newinfo;
}
- ret = translate_table(net, tmp.name, tmp.valid_hooks,
- newinfo, loc_cpu_entry, tmp.size, tmp.num_entries,
- tmp.hook_entry, tmp.underflow);
+ ret = translate_table(net, newinfo, loc_cpu_entry, &tmp);
if (ret != 0)
goto free_newinfo;
@@ -2112,11 +2104,7 @@ struct xt_table *ipt_register_table(struct net *net,
loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
memcpy(loc_cpu_entry, repl->entries, repl->size);
- ret = translate_table(net, table->name, table->valid_hooks,
- newinfo, loc_cpu_entry, repl->size,
- repl->num_entries,
- repl->hook_entry,
- repl->underflow);
+ ret = translate_table(net, newinfo, loc_cpu_entry, repl);
if (ret != 0)
goto out_free;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index c5a963e..f704286 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -845,22 +845,15 @@ static void cleanup_entry(struct ip6t_entry *e, struct net *net)
/* Checks and translates the user-supplied table segment (held in
newinfo) */
static int
-translate_table(struct net *net,
- const char *name,
- unsigned int valid_hooks,
- struct xt_table_info *newinfo,
- void *entry0,
- unsigned int size,
- unsigned int number,
- const unsigned int *hook_entries,
- const unsigned int *underflows)
+translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
+ const struct ip6t_replace *repl)
{
struct ip6t_entry *iter;
unsigned int i;
int ret = 0;
- newinfo->size = size;
- newinfo->number = number;
+ newinfo->size = repl->size;
+ newinfo->number = repl->num_entries;
/* Init all hooks to impossible value. */
for (i = 0; i < NF_INET_NUMHOOKS; i++) {
@@ -873,42 +866,43 @@ translate_table(struct net *net,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + size, hook_entries, underflows, valid_hooks);
+ entry0 + repl->size, repl->hook_entry, repl->underflow,
+ repl->valid_hooks);
if (ret != 0)
return ret;
++i;
}
- if (i != number) {
+ if (i != repl->num_entries) {
duprintf("translate_table: %u not %u entries\n",
- i, number);
+ i, repl->num_entries);
return -EINVAL;
}
/* Check hooks all assigned */
for (i = 0; i < NF_INET_NUMHOOKS; i++) {
/* Only hooks which are valid */
- if (!(valid_hooks & (1 << i)))
+ if (!(repl->valid_hooks & (1 << i)))
continue;
if (newinfo->hook_entry[i] == 0xFFFFFFFF) {
duprintf("Invalid hook entry %u %u\n",
- i, hook_entries[i]);
+ i, repl->hook_entry[i]);
return -EINVAL;
}
if (newinfo->underflow[i] == 0xFFFFFFFF) {
duprintf("Invalid underflow %u %u\n",
- i, underflows[i]);
+ i, repl->underflow[i]);
return -EINVAL;
}
}
- if (!mark_source_chains(newinfo, valid_hooks, entry0))
+ if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
return -ELOOP;
/* Finally, each sanity check must pass */
i = 0;
xt_entry_foreach(iter, entry0, newinfo->size) {
- ret = find_check_entry(iter, net, name, size);
+ ret = find_check_entry(iter, net, repl->name, repl->size);
if (ret != 0)
break;
++i;
@@ -1342,9 +1336,7 @@ do_replace(struct net *net, const void __user *user, unsigned int len)
goto free_newinfo;
}
- ret = translate_table(net, tmp.name, tmp.valid_hooks,
- newinfo, loc_cpu_entry, tmp.size, tmp.num_entries,
- tmp.hook_entry, tmp.underflow);
+ ret = translate_table(net, newinfo, loc_cpu_entry, &tmp);
if (ret != 0)
goto free_newinfo;
@@ -2145,11 +2137,7 @@ struct xt_table *ip6t_register_table(struct net *net,
loc_cpu_entry = newinfo->entries[raw_smp_processor_id()];
memcpy(loc_cpu_entry, repl->entries, repl->size);
- ret = translate_table(net, table->name, table->valid_hooks,
- newinfo, loc_cpu_entry, repl->size,
- repl->num_entries,
- repl->hook_entry,
- repl->underflow);
+ ret = translate_table(net, newinfo, loc_cpu_entry, repl);
if (ret != 0)
goto out_free;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/6] netfilter: xtables2: make ip_tables reentrant
2010-02-12 10:20 ip_tables: foreachs and reentrancy Jan Engelhardt
` (4 preceding siblings ...)
2010-02-12 10:20 ` [PATCH 5/6] netfilter: xtables: reduce arguments to translate_table Jan Engelhardt
@ 2010-02-12 10:20 ` Jan Engelhardt
2010-02-24 17:40 ` ip_tables: foreachs and reentrancy Patrick McHardy
6 siblings, 0 replies; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-12 10:20 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel
Currently, the table traverser stores return addresses in the ruleset
itself (struct ip6t_entry->comefrom). This has a well-known drawback:
the jumpstack is overwritten on reentry, making it necessary for
targets to return absolute verdicts. Also, the ruleset (which might
be heavy memory-wise) needs to be replicated for each CPU that can
possibly invoke ip6t_do_table.
This patch decouples the jumpstack from struct ip6t_entry and instead
puts it into xt_table_info. Not being restricted by 'comefrom'
anymore, we can set up a stack as needed. By default, there is room
allocated for two entries into the traverser. The setting is
configurable at runtime through sysfs and will take effect when a
table is replaced by a new one.
arp_tables is not touched though, because there is just one/two
modules and further patches seek to collapse the table traverser
anyhow.
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
include/linux/netfilter/x_tables.h | 7 +++
net/ipv4/netfilter/arp_tables.c | 6 ++-
net/ipv4/netfilter/ip_tables.c | 65 ++++++++++++++++--------------
net/ipv6/netfilter/ip6_tables.c | 56 ++++++++++---------------
net/netfilter/x_tables.c | 79 ++++++++++++++++++++++++++++++++++++
5 files changed, 147 insertions(+), 66 deletions(-)
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 26598e6..a1be066 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -399,6 +399,13 @@ struct xt_table_info {
unsigned int hook_entry[NF_INET_NUMHOOKS];
unsigned int underflow[NF_INET_NUMHOOKS];
+ /*
+ * Number of user chains. Since tables cannot have loops, at most
+ * @stacksize jumps (number of user chains) can possibly be made.
+ */
+ unsigned int stacksize;
+ unsigned int *stackptr;
+ void ***jumpstack;
/* ipt_entry tables: one per CPU */
/* Note : this field MUST be the last one, see XT_TABLE_INFO_SZ */
void *entries[1];
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 57098dc..23ae7f0 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -649,6 +649,9 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
if (ret != 0)
break;
++i;
+ if (strcmp(arpt_get_target(iter)->u.user.name,
+ XT_ERROR_TARGET) == 0)
+ ++newinfo->stacksize;
}
duprintf("translate_table: ARPT_ENTRY_ITERATE gives %d\n", ret);
if (ret != 0)
@@ -1773,8 +1776,7 @@ struct xt_table *arpt_register_table(struct net *net,
{
int ret;
struct xt_table_info *newinfo;
- struct xt_table_info bootstrap
- = { 0, 0, 0, { 0 }, { 0 }, { } };
+ struct xt_table_info bootstrap = {0};
void *loc_cpu_entry;
struct xt_table *new_table;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index c92f4e5..f8bd549 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -322,8 +322,6 @@ ipt_do_table(struct sk_buff *skb,
const struct net_device *out,
struct xt_table *table)
{
-#define tb_comefrom ((struct ipt_entry *)table_base)->comefrom
-
static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
const struct iphdr *ip;
bool hotdrop = false;
@@ -331,7 +329,8 @@ ipt_do_table(struct sk_buff *skb,
unsigned int verdict = NF_DROP;
const char *indev, *outdev;
const void *table_base;
- struct ipt_entry *e, *back;
+ struct ipt_entry *e, **jumpstack;
+ unsigned int *stackptr, origptr, cpu;
const struct xt_table_info *private;
struct xt_match_param mtpar;
struct xt_target_param tgpar;
@@ -357,19 +356,23 @@ ipt_do_table(struct sk_buff *skb,
IP_NF_ASSERT(table->valid_hooks & (1 << hook));
xt_info_rdlock_bh();
private = table->private;
- table_base = private->entries[smp_processor_id()];
+ cpu = smp_processor_id();
+ table_base = private->entries[cpu];
+ jumpstack = (struct ipt_entry **)private->jumpstack[cpu];
+ stackptr = &private->stackptr[cpu];
+ origptr = *stackptr;
e = get_entry(table_base, private->hook_entry[hook]);
- /* For return from builtin chain */
- back = get_entry(table_base, private->underflow[hook]);
+ pr_devel("Entering %s(hook %u); sp at %u (UF %p)\n",
+ table->name, hook, origptr,
+ get_entry(table_base, private->underflow[hook]));
do {
const struct ipt_entry_target *t;
const struct xt_entry_match *ematch;
IP_NF_ASSERT(e);
- IP_NF_ASSERT(back);
if (!ip_packet_match(ip, indev, outdev,
&e->ip, mtpar.fragoff)) {
no_match:
@@ -404,17 +407,28 @@ ipt_do_table(struct sk_buff *skb,
verdict = (unsigned)(-v) - 1;
break;
}
- e = back;
- back = get_entry(table_base, back->comefrom);
+ if (*stackptr == 0) {
+ e = get_entry(table_base,
+ private->underflow[hook]);
+ pr_devel("Underflow (this is normal) "
+ "to %p\n", e);
+ } else {
+ e = jumpstack[--*stackptr];
+ pr_devel("Pulled %p out from pos %u\n",
+ e, *stackptr);
+ e = ipt_next_entry(e);
+ }
continue;
}
if (table_base + v != ipt_next_entry(e) &&
!(e->ip.flags & IPT_F_GOTO)) {
- /* Save old back ptr in next entry */
- struct ipt_entry *next = ipt_next_entry(e);
- next->comefrom = (void *)back - table_base;
- /* set back pointer to next entry */
- back = next;
+ if (*stackptr >= private->stacksize) {
+ verdict = NF_DROP;
+ break;
+ }
+ jumpstack[(*stackptr)++] = e;
+ pr_devel("Pushed %p into pos %u\n",
+ e, *stackptr - 1);
}
e = get_entry(table_base, v);
@@ -427,18 +441,7 @@ ipt_do_table(struct sk_buff *skb,
tgpar.targinfo = t->data;
-#ifdef CONFIG_NETFILTER_DEBUG
- tb_comefrom = 0xeeeeeeec;
-#endif
verdict = t->u.kernel.target->target(skb, &tgpar);
-#ifdef CONFIG_NETFILTER_DEBUG
- if (tb_comefrom != 0xeeeeeeec && verdict == IPT_CONTINUE) {
- printk("Target %s reentered!\n",
- t->u.kernel.target->name);
- verdict = NF_DROP;
- }
- tb_comefrom = 0x57acc001;
-#endif
/* Target might have changed stuff. */
ip = ip_hdr(skb);
if (verdict == IPT_CONTINUE)
@@ -448,7 +451,9 @@ ipt_do_table(struct sk_buff *skb,
break;
} while (!hotdrop);
xt_info_rdunlock_bh();
-
+ pr_devel("Exiting %s; resetting sp from %u to %u\n",
+ __func__, *stackptr, origptr);
+ *stackptr = origptr;
#ifdef DEBUG_ALLOW_ALL
return NF_ACCEPT;
#else
@@ -456,8 +461,6 @@ ipt_do_table(struct sk_buff *skb,
return NF_DROP;
else return verdict;
#endif
-
-#undef tb_comefrom
}
/* Figures out from what hook each rule can be called: returns 0 if
@@ -841,6 +844,9 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
if (ret != 0)
return ret;
++i;
+ if (strcmp(ipt_get_target(iter)->u.user.name,
+ XT_ERROR_TARGET) == 0)
+ ++newinfo->stacksize;
}
if (i != repl->num_entries) {
@@ -2089,8 +2095,7 @@ struct xt_table *ipt_register_table(struct net *net,
{
int ret;
struct xt_table_info *newinfo;
- struct xt_table_info bootstrap
- = { 0, 0, 0, { 0 }, { 0 }, { } };
+ struct xt_table_info bootstrap = {0};
void *loc_cpu_entry;
struct xt_table *new_table;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index f704286..1636004 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -352,15 +352,14 @@ ip6t_do_table(struct sk_buff *skb,
const struct net_device *out,
struct xt_table *table)
{
-#define tb_comefrom ((struct ip6t_entry *)table_base)->comefrom
-
static const char nulldevname[IFNAMSIZ] __attribute__((aligned(sizeof(long))));
bool hotdrop = false;
/* Initializing verdict to NF_DROP keeps gcc happy. */
unsigned int verdict = NF_DROP;
const char *indev, *outdev;
const void *table_base;
- struct ip6t_entry *e, *back;
+ struct ip6t_entry *e, **jumpstack;
+ unsigned int *stackptr, origptr, cpu;
const struct xt_table_info *private;
struct xt_match_param mtpar;
struct xt_target_param tgpar;
@@ -384,19 +383,19 @@ ip6t_do_table(struct sk_buff *skb,
xt_info_rdlock_bh();
private = table->private;
- table_base = private->entries[smp_processor_id()];
+ cpu = smp_processor_id();
+ table_base = private->entries[cpu];
+ jumpstack = (struct ip6t_entry **)private->jumpstack[cpu];
+ stackptr = &private->stackptr[cpu];
+ origptr = *stackptr;
e = get_entry(table_base, private->hook_entry[hook]);
- /* For return from builtin chain */
- back = get_entry(table_base, private->underflow[hook]);
-
do {
const struct ip6t_entry_target *t;
const struct xt_entry_match *ematch;
IP_NF_ASSERT(e);
- IP_NF_ASSERT(back);
if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
&mtpar.thoff, &mtpar.fragoff, &hotdrop)) {
no_match:
@@ -433,17 +432,20 @@ ip6t_do_table(struct sk_buff *skb,
verdict = (unsigned)(-v) - 1;
break;
}
- e = back;
- back = get_entry(table_base, back->comefrom);
+ if (*stackptr == 0)
+ e = get_entry(table_base,
+ private->underflow[hook]);
+ else
+ e = ip6t_next_entry(jumpstack[--*stackptr]);
continue;
}
if (table_base + v != ip6t_next_entry(e) &&
!(e->ipv6.flags & IP6T_F_GOTO)) {
- /* Save old back ptr in next entry */
- struct ip6t_entry *next = ip6t_next_entry(e);
- next->comefrom = (void *)back - table_base;
- /* set back pointer to next entry */
- back = next;
+ if (*stackptr >= private->stacksize) {
+ verdict = NF_DROP;
+ break;
+ }
+ jumpstack[(*stackptr)++] = e;
}
e = get_entry(table_base, v);
@@ -455,19 +457,7 @@ ip6t_do_table(struct sk_buff *skb,
tgpar.target = t->u.kernel.target;
tgpar.targinfo = t->data;
-#ifdef CONFIG_NETFILTER_DEBUG
- tb_comefrom = 0xeeeeeeec;
-#endif
verdict = t->u.kernel.target->target(skb, &tgpar);
-
-#ifdef CONFIG_NETFILTER_DEBUG
- if (tb_comefrom != 0xeeeeeeec && verdict == IP6T_CONTINUE) {
- printk("Target %s reentered!\n",
- t->u.kernel.target->name);
- verdict = NF_DROP;
- }
- tb_comefrom = 0x57acc001;
-#endif
if (verdict == IP6T_CONTINUE)
e = ip6t_next_entry(e);
else
@@ -475,10 +465,8 @@ ip6t_do_table(struct sk_buff *skb,
break;
} while (!hotdrop);
-#ifdef CONFIG_NETFILTER_DEBUG
- tb_comefrom = NETFILTER_LINK_POISON;
-#endif
xt_info_rdunlock_bh();
+ *stackptr = origptr;
#ifdef DEBUG_ALLOW_ALL
return NF_ACCEPT;
@@ -487,8 +475,6 @@ ip6t_do_table(struct sk_buff *skb,
return NF_DROP;
else return verdict;
#endif
-
-#undef tb_comefrom
}
/* Figures out from what hook each rule can be called: returns 0 if
@@ -871,6 +857,9 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
if (ret != 0)
return ret;
++i;
+ if (strcmp(ip6t_get_target(iter)->u.user.name,
+ XT_ERROR_TARGET) == 0)
+ ++newinfo->stacksize;
}
if (i != repl->num_entries) {
@@ -2122,8 +2111,7 @@ struct xt_table *ip6t_register_table(struct net *net,
{
int ret;
struct xt_table_info *newinfo;
- struct xt_table_info bootstrap
- = { 0, 0, 0, { 0 }, { 0 }, { } };
+ struct xt_table_info bootstrap = {0};
void *loc_cpu_entry;
struct xt_table *new_table;
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 2120ab7..5e7a571 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -68,6 +68,11 @@ static const char *const xt_prefix[NFPROTO_NUMPROTO] = {
[NFPROTO_IPV6] = "ip6",
};
+/* Allow this many total (re)entries. */
+static unsigned int xt_jumpstack_multiplier = 2;
+module_param_named(jumpstack_multiplier, xt_jumpstack_multiplier,
+ uint, S_IRUGO | S_IWUSR);
+
/* Registration hooks for targets. */
int
xt_register_target(struct xt_target *target)
@@ -661,6 +666,26 @@ void xt_free_table_info(struct xt_table_info *info)
else
vfree(info->entries[cpu]);
}
+
+ if (info->jumpstack != NULL) {
+ if (sizeof(void *) * info->stacksize > PAGE_SIZE) {
+ for_each_possible_cpu(cpu)
+ vfree(info->jumpstack[cpu]);
+ } else {
+ for_each_possible_cpu(cpu)
+ kfree(info->jumpstack[cpu]);
+ }
+ }
+
+ if (sizeof(void **) * nr_cpu_ids > PAGE_SIZE)
+ vfree(info->jumpstack);
+ else
+ kfree(info->jumpstack);
+ if (sizeof(unsigned int) * nr_cpu_ids > PAGE_SIZE)
+ vfree(info->stackptr);
+ else
+ kfree(info->stackptr);
+
kfree(info);
}
EXPORT_SYMBOL(xt_free_table_info);
@@ -705,6 +730,49 @@ EXPORT_SYMBOL_GPL(xt_compat_unlock);
DEFINE_PER_CPU(struct xt_info_lock, xt_info_locks);
EXPORT_PER_CPU_SYMBOL_GPL(xt_info_locks);
+static int xt_jumpstack_alloc(struct xt_table_info *i)
+{
+ unsigned int size;
+ int cpu;
+
+ size = sizeof(unsigned int) * nr_cpu_ids;
+ if (size > PAGE_SIZE)
+ i->stackptr = vmalloc(size);
+ else
+ i->stackptr = kmalloc(size, GFP_KERNEL);
+ if (i->stackptr == NULL)
+ return -ENOMEM;
+ memset(i->stackptr, 0, size);
+
+ size = sizeof(void **) * nr_cpu_ids;
+ if (size > PAGE_SIZE)
+ i->jumpstack = vmalloc(size);
+ else
+ i->jumpstack = kmalloc(size, GFP_KERNEL);
+ if (i->jumpstack == NULL)
+ return -ENOMEM;
+ memset(i->jumpstack, 0, size);
+
+ i->stacksize *= xt_jumpstack_multiplier;
+ size = sizeof(void *) * i->stacksize;
+ for_each_possible_cpu(cpu) {
+ if (size > PAGE_SIZE)
+ i->jumpstack[cpu] = vmalloc_node(size,
+ cpu_to_node(cpu));
+ else
+ i->jumpstack[cpu] = kmalloc_node(size,
+ GFP_KERNEL, cpu_to_node(cpu));
+ if (i->jumpstack[cpu] == NULL)
+ /*
+ * Freeing will be done later on by the callers. The
+ * chain is: xt_replace_table -> __do_replace ->
+ * do_replace -> xt_free_table_info.
+ */
+ return -ENOMEM;
+ }
+
+ return 0;
+}
struct xt_table_info *
xt_replace_table(struct xt_table *table,
@@ -713,6 +781,7 @@ xt_replace_table(struct xt_table *table,
int *error)
{
struct xt_table_info *private;
+ int ret;
/* Do the substitution. */
local_bh_disable();
@@ -727,6 +796,12 @@ xt_replace_table(struct xt_table *table,
return NULL;
}
+ ret = xt_jumpstack_alloc(newinfo);
+ if (ret < 0) {
+ *error = ret;
+ return NULL;
+ }
+
table->private = newinfo;
newinfo->initial_entries = private->initial_entries;
@@ -751,6 +826,10 @@ struct xt_table *xt_register_table(struct net *net,
struct xt_table_info *private;
struct xt_table *t, *table;
+ ret = xt_jumpstack_alloc(newinfo);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
/* Don't add one object to multiple lists. */
table = kmemdup(input_table, sizeof(struct xt_table), GFP_KERNEL);
if (!table) {
--
1.6.6.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: ip_tables: foreachs and reentrancy
2010-02-12 10:20 ip_tables: foreachs and reentrancy Jan Engelhardt
` (5 preceding siblings ...)
2010-02-12 10:20 ` [PATCH 6/6] netfilter: xtables2: make ip_tables reentrant Jan Engelhardt
@ 2010-02-24 17:40 ` Patrick McHardy
2010-02-24 19:33 ` Jan Engelhardt
6 siblings, 1 reply; 16+ messages in thread
From: Patrick McHardy @ 2010-02-24 17:40 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netfilter-devel
Jan Engelhardt wrote:
> the next patch group following the previous cleanups is this one.
>
> The macros were really ugly (you could not easily tell where the
> argument list that was passed to the real function started), so
> I took the freedom to remodel these based upon the excellent ideas
> from linux/list.h. It seems to have turned out well, there is much
> less argument passing.
>
> The reentrancy patch title should speak for itself.
>
>
> The following changes since commit 05c7a108fdbd2ebbc357a78597646c111f8ebc62:
> Jan Engelhardt (1):
> netfilter: xtables: add const qualifiers
>
> are available in the git repository at:
>
> git://dev.medozas.de/linux master-d30b8f5
>
> Jan Engelhardt (6):
> netfilter: xtables: replace XT_ENTRY_ITERATE macro
> netfilter: xtables: optimize call flow around xt_entry_foreach
> netfilter: xtables: replace XT_MATCH_ITERATE macro
> netfilter: xtables: optimize call flow around xt_ematch_foreach
> netfilter: xtables: reduce arguments to translate_table
> netfilter: xtables2: make ip_tables reentrant
I've applied patch 1-5 for now. Patch 6 doesn't add any value
so far, so it should go in a series that actually makes use of
this (please note: I don't want any larger changes anymore in
this release however).
BTW, I noticed some minor cosmetic problems that I didn't fix
up to avoid clashes, please send me a patch on top to fix up
indentation in spots like this:
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
entry0 + repl->size, repl->hook_entry, repl->underflow,
repl->valid_hooks);
Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: ip_tables: foreachs and reentrancy
2010-02-24 17:40 ` ip_tables: foreachs and reentrancy Patrick McHardy
@ 2010-02-24 19:33 ` Jan Engelhardt
2010-02-25 10:24 ` Patrick McHardy
2010-02-26 16:54 ` Patrick McHardy
0 siblings, 2 replies; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-24 19:33 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Wednesday 2010-02-24 18:40, Patrick McHardy wrote:
>> Jan Engelhardt (6):
>> netfilter: xtables: replace XT_ENTRY_ITERATE macro
>> netfilter: xtables: optimize call flow around xt_entry_foreach
>> netfilter: xtables: replace XT_MATCH_ITERATE macro
>> netfilter: xtables: optimize call flow around xt_ematch_foreach
>> netfilter: xtables: reduce arguments to translate_table
>> netfilter: xtables2: make ip_tables reentrant
>
>I've applied patch 1-5 for now. Patch 6 doesn't add any value
>so far, so it should go in a series that actually makes use of
>this
Rusty left a note "must return absolute verdict" in ipt_REJECT
and ip6t_REJECT, so maybe he thought of something.
Irrespective of that however, there is xt_TEE in Xtables-addons which
would greatly benefit in the 6th patch (which is standalone, nothing
else is needed in the kernel to get it done), as it could then track
both the original copy and the tee'd copy through the tables, instead
of having to forget one.
Given the nftables snapshot also has some sort of reentrancy safety,
I don't see why Xtables should not have that. And since the Xtables
patch also makes it a runtime-tunable, we've got the best of all
worlds.
>(please note: I don't want any larger changes anymore in
>this release however).
Uff. That was a pretty small patchstream NF produced this time
towards upstream.
My intention was to have the entire set merged by 2.6.32, only
delayed by my own tinkering around with the get_cycle measurement of
the data collapse. But it was ready on-time for the 2.6.34 window!
What went wrong? Do we need to begin the merge-into-kabernext window
much earlier?
>BTW, I noticed some minor cosmetic problems that I didn't fix
>up to avoid clashes, please send me a patch on top to fix up
>indentation in spots like this:
>
> /* Walk through entries, checking offsets. */
> xt_entry_foreach(iter, entry0, newinfo->size) {
> ret = check_entry_size_and_hooks(iter, newinfo, entry0,
> entry0 + repl->size, repl->hook_entry, repl->underflow,
> repl->valid_hooks);
>
>Thanks!
Heh, this drives it ad absurdum.
"Now, some people will claim that having 8-character
indentations makes the code move too far to the right,[...]
if you need more than 3 levels of indentation, you're screwed"
Linus, I bet you did not envision a 3-level indent (which is what
we have here in ip_tables.c) and still getting screwed in the end :-)
But oh well what don't we all do just to get code merged at all.
Here goes:
parent 0f234214d15fa914436d304ecf5c3e43449e79f9 (v2.6.33-rc8-1216-g0f23421)
commit 289654709ca94f8de4fa615b23f8e9e9f1527fae
Author: Jan Engelhardt <jengelh@medozas.de>
Date: Wed Feb 24 20:29:05 2010 +0100
iptables: restore maintainer's idea of indent
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
net/ipv4/netfilter/arp_tables.c | 23 ++++++++++++++---------
net/ipv4/netfilter/ip_tables.c | 25 +++++++++++++++----------
net/ipv6/netfilter/ip6_tables.c | 25 +++++++++++++++----------
3 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 57098dc..f07d77f 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -644,8 +644,10 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + repl->size, repl->hook_entry, repl->underflow,
- repl->valid_hooks);
+ entry0 + repl->size,
+ repl->hook_entry,
+ repl->underflow,
+ repl->valid_hooks);
if (ret != 0)
break;
++i;
@@ -730,7 +732,7 @@ static void get_counters(const struct xt_table_info *t,
i = 0;
xt_entry_foreach(iter, t->entries[curcpu], t->size) {
SET_COUNTER(counters[i], iter->counters.bcnt,
- iter->counters.pcnt);
+ iter->counters.pcnt);
++i;
}
@@ -741,7 +743,7 @@ static void get_counters(const struct xt_table_info *t,
xt_info_wrlock(cpu);
xt_entry_foreach(iter, t->entries[cpu], t->size) {
ADD_COUNTER(counters[i], iter->counters.bcnt,
- iter->counters.pcnt);
+ iter->counters.pcnt);
++i;
}
xt_info_wrunlock(cpu);
@@ -1356,8 +1358,11 @@ static int translate_compat_table(const char *name,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter0, entry0, total_size) {
ret = check_compat_entry_size_and_hooks(iter0, info, &size,
- entry0, entry0 + total_size, hook_entries, underflows,
- name);
+ entry0,
+ entry0 + total_size,
+ hook_entries,
+ underflows,
+ name);
if (ret != 0)
goto out_unlock;
++j;
@@ -1401,8 +1406,8 @@ static int translate_compat_table(const char *name,
pos = entry1;
size = total_size;
xt_entry_foreach(iter0, entry0, total_size) {
- ret = compat_copy_entry_from_user(iter0, &pos,
- &size, name, newinfo, entry1);
+ ret = compat_copy_entry_from_user(iter0, &pos, &size,
+ name, newinfo, entry1);
if (ret != 0)
break;
}
@@ -1617,7 +1622,7 @@ static int compat_copy_entries_to_user(unsigned int total_size,
size = total_size;
xt_entry_foreach(iter, loc_cpu_entry, total_size) {
ret = compat_copy_entry_to_user(iter, &pos,
- &size, counters, i++);
+ &size, counters, i++);
if (ret != 0)
break;
}
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index c92f4e5..b29c66d 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -836,8 +836,10 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + repl->size, repl->hook_entry, repl->underflow,
- repl->valid_hooks);
+ entry0 + repl->size,
+ repl->hook_entry,
+ repl->underflow,
+ repl->valid_hooks);
if (ret != 0)
return ret;
++i;
@@ -918,7 +920,7 @@ get_counters(const struct xt_table_info *t,
i = 0;
xt_entry_foreach(iter, t->entries[curcpu], t->size) {
SET_COUNTER(counters[i], iter->counters.bcnt,
- iter->counters.pcnt);
+ iter->counters.pcnt);
++i;
}
@@ -929,7 +931,7 @@ get_counters(const struct xt_table_info *t,
xt_info_wrlock(cpu);
xt_entry_foreach(iter, t->entries[cpu], t->size) {
ADD_COUNTER(counters[i], iter->counters.bcnt,
- iter->counters.pcnt);
+ iter->counters.pcnt);
++i; /* macro does multi eval of i */
}
xt_info_wrunlock(cpu);
@@ -1540,7 +1542,7 @@ check_compat_entry_size_and_hooks(struct compat_ipt_entry *e,
j = 0;
xt_ematch_foreach(ematch, e) {
ret = compat_find_calc_match(ematch, name,
- &e->ip, e->comefrom, &off);
+ &e->ip, e->comefrom, &off);
if (ret != 0)
goto release_matches;
++j;
@@ -1701,8 +1703,11 @@ translate_compat_table(struct net *net,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter0, entry0, total_size) {
ret = check_compat_entry_size_and_hooks(iter0, info, &size,
- entry0, entry0 + total_size, hook_entries, underflows,
- name);
+ entry0,
+ entry0 + total_size,
+ hook_entries,
+ underflows,
+ name);
if (ret != 0)
goto out_unlock;
++j;
@@ -1746,8 +1751,8 @@ translate_compat_table(struct net *net,
pos = entry1;
size = total_size;
xt_entry_foreach(iter0, entry0, total_size) {
- ret = compat_copy_entry_from_user(iter0, &pos,
- &size, name, newinfo, entry1);
+ ret = compat_copy_entry_from_user(iter0, &pos, &size,
+ name, newinfo, entry1);
if (ret != 0)
break;
}
@@ -1927,7 +1932,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
size = total_size;
xt_entry_foreach(iter, loc_cpu_entry, total_size) {
ret = compat_copy_entry_to_user(iter, &pos,
- &size, counters, i++);
+ &size, counters, i++);
if (ret != 0)
break;
}
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index f704286..9210e31 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -866,8 +866,10 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter, entry0, newinfo->size) {
ret = check_entry_size_and_hooks(iter, newinfo, entry0,
- entry0 + repl->size, repl->hook_entry, repl->underflow,
- repl->valid_hooks);
+ entry0 + repl->size,
+ repl->hook_entry,
+ repl->underflow,
+ repl->valid_hooks);
if (ret != 0)
return ret;
++i;
@@ -948,7 +950,7 @@ get_counters(const struct xt_table_info *t,
i = 0;
xt_entry_foreach(iter, t->entries[curcpu], t->size) {
SET_COUNTER(counters[i], iter->counters.bcnt,
- iter->counters.pcnt);
+ iter->counters.pcnt);
++i;
}
@@ -959,7 +961,7 @@ get_counters(const struct xt_table_info *t,
xt_info_wrlock(cpu);
xt_entry_foreach(iter, t->entries[cpu], t->size) {
ADD_COUNTER(counters[i], iter->counters.bcnt,
- iter->counters.pcnt);
+ iter->counters.pcnt);
++i;
}
xt_info_wrunlock(cpu);
@@ -1573,7 +1575,7 @@ check_compat_entry_size_and_hooks(struct compat_ip6t_entry *e,
j = 0;
xt_ematch_foreach(ematch, e) {
ret = compat_find_calc_match(ematch, name,
- &e->ipv6, e->comefrom, &off);
+ &e->ipv6, e->comefrom, &off);
if (ret != 0)
goto release_matches;
++j;
@@ -1734,8 +1736,11 @@ translate_compat_table(struct net *net,
/* Walk through entries, checking offsets. */
xt_entry_foreach(iter0, entry0, total_size) {
ret = check_compat_entry_size_and_hooks(iter0, info, &size,
- entry0, entry0 + total_size, hook_entries, underflows,
- name);
+ entry0,
+ entry0 + total_size,
+ hook_entries,
+ underflows,
+ name);
if (ret != 0)
goto out_unlock;
++j;
@@ -1779,8 +1784,8 @@ translate_compat_table(struct net *net,
pos = entry1;
size = total_size;
xt_entry_foreach(iter0, entry0, total_size) {
- ret = compat_copy_entry_from_user(iter0, &pos,
- &size, name, newinfo, entry1);
+ ret = compat_copy_entry_from_user(iter0, &pos, &size,
+ name, newinfo, entry1);
if (ret != 0)
break;
}
@@ -1960,7 +1965,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table,
size = total_size;
xt_entry_foreach(iter, loc_cpu_entry, total_size) {
ret = compat_copy_entry_to_user(iter, &pos,
- &size, counters, i++);
+ &size, counters, i++);
if (ret != 0)
break;
}
--
# Created with git-export-patch
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: ip_tables: foreachs and reentrancy
2010-02-24 19:33 ` Jan Engelhardt
@ 2010-02-25 10:24 ` Patrick McHardy
2010-02-25 11:07 ` Jan Engelhardt
2010-02-26 16:54 ` Patrick McHardy
1 sibling, 1 reply; 16+ messages in thread
From: Patrick McHardy @ 2010-02-25 10:24 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netfilter-devel
Jan Engelhardt wrote:
> On Wednesday 2010-02-24 18:40, Patrick McHardy wrote:
>>> Jan Engelhardt (6):
>>> netfilter: xtables: replace XT_ENTRY_ITERATE macro
>>> netfilter: xtables: optimize call flow around xt_entry_foreach
>>> netfilter: xtables: replace XT_MATCH_ITERATE macro
>>> netfilter: xtables: optimize call flow around xt_ematch_foreach
>>> netfilter: xtables: reduce arguments to translate_table
>>> netfilter: xtables2: make ip_tables reentrant
>> I've applied patch 1-5 for now. Patch 6 doesn't add any value
>> so far, so it should go in a series that actually makes use of
>> this
>
> Rusty left a note "must return absolute verdict" in ipt_REJECT
> and ip6t_REJECT, so maybe he thought of something.
>
> Irrespective of that however, there is xt_TEE in Xtables-addons which
> would greatly benefit in the 6th patch (which is standalone, nothing
> else is needed in the kernel to get it done), as it could then track
> both the original copy and the tee'd copy through the tables, instead
> of having to forget one.
There's no benefit to the kernel at this point, just the risk of
breakage. Anyways, its too late now.
> My intention was to have the entire set merged by 2.6.32, only
> delayed by my own tinkering around with the get_cycle measurement of
> the data collapse. But it was ready on-time for the 2.6.34 window!
> What went wrong? Do we need to begin the merge-into-kabernext window
> much earlier?
Yes, you should begin submitting patches when net-next opens,
usually at -rc1.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: ip_tables: foreachs and reentrancy
2010-02-25 10:24 ` Patrick McHardy
@ 2010-02-25 11:07 ` Jan Engelhardt
2010-02-25 11:09 ` Patrick McHardy
0 siblings, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2010-02-25 11:07 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Thursday 2010-02-25 11:24, Patrick McHardy wrote:
>
>> My intention was to have the entire set merged by 2.6.32, only
>> delayed by my own tinkering around with the get_cycle measurement of
>> the data collapse. But it was ready on-time for the 2.6.34 window!
>> What went wrong? Do we need to begin the merge-into-kabernext window
>> much earlier?
>
>Yes, you should begin submitting patches when net-next opens,
>usually at -rc1.
Thanks for clearing this point of doubt up - I was under the
impression that submissions were only accepted when kaber-next
opens.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: ip_tables: foreachs and reentrancy
2010-02-25 11:07 ` Jan Engelhardt
@ 2010-02-25 11:09 ` Patrick McHardy
0 siblings, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2010-02-25 11:09 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netfilter-devel
Jan Engelhardt wrote:
> On Thursday 2010-02-25 11:24, Patrick McHardy wrote:
>>> My intention was to have the entire set merged by 2.6.32, only
>>> delayed by my own tinkering around with the get_cycle measurement of
>>> the data collapse. But it was ready on-time for the 2.6.34 window!
>>> What went wrong? Do we need to begin the merge-into-kabernext window
>>> much earlier?
>> Yes, you should begin submitting patches when net-next opens,
>> usually at -rc1.
>
> Thanks for clearing this point of doubt up - I was under the
> impression that submissions were only accepted when kaber-next
> opens.
It opens when submissions show up :)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: ip_tables: foreachs and reentrancy
2010-02-24 19:33 ` Jan Engelhardt
2010-02-25 10:24 ` Patrick McHardy
@ 2010-02-26 16:54 ` Patrick McHardy
1 sibling, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2010-02-26 16:54 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netfilter-devel
Jan Engelhardt wrote:
> parent 0f234214d15fa914436d304ecf5c3e43449e79f9 (v2.6.33-rc8-1216-g0f23421)
> commit 289654709ca94f8de4fa615b23f8e9e9f1527fae
> Author: Jan Engelhardt <jengelh@medozas.de>
> Date: Wed Feb 24 20:29:05 2010 +0100
>
> iptables: restore maintainer's idea of indent
Applied, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread