kernel/iptables patches

kernel/iptables patches

[Jan Engelhardt]

Hi,


please apply the following tidbits for the kernel-side iptables code.
Alternatively, the set is provided at
	git://dev.medozas.de/linux master
with base dave/master:v2.6.30-rc1-1016-g928f308.


Jan Engelhardt (10):
      netfilter: xtables: use NFPROTO_ for xt_proto_init callsites
      netfilter: queue: use NFPROTO_ for queue callsites
      netfilter: xtables: use NFPROTO_ in standard targets
      netfilter: xtables: remove redundant casts
      netfilter: xtables: fix const inconsistency
      netfilter: xtables: consolidate open-coded logic
      netfilter: xtables: reduce indent level by one
      netfilter: xtables: remove some goto
      netfilter: xtables: remove another level of indent
      netfilter: xtables: consolidate comefrom debug cast access

These transforms are part of an ongoing investigation on how to
consolidate the *tables, and making it a bit more readable seemed the
first step. The patches seem useful independently nevertheless.

[PATCH 01/11] netfilter: xtables: use NFPROTO_ for xt_proto_init callsites

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/ip_tables.c  |    4 ++--
 net/ipv6/netfilter/ip6_tables.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 2ec8d72..3051dda 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -2201,12 +2201,12 @@ static struct xt_match icmp_matchstruct __read_mostly = {
 
 static int __net_init ip_tables_net_init(struct net *net)
 {
-	return xt_proto_init(net, AF_INET);
+	return xt_proto_init(net, NFPROTO_IPV4);
 }
 
 static void __net_exit ip_tables_net_exit(struct net *net)
 {
-	xt_proto_fini(net, AF_INET);
+	xt_proto_fini(net, NFPROTO_IPV4);
 }
 
 static struct pernet_operations ip_tables_net_ops = {
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 219e165..41fbc1e 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -2234,12 +2234,12 @@ static struct xt_match icmp6_matchstruct __read_mostly = {
 
 static int __net_init ip6_tables_net_init(struct net *net)
 {
-	return xt_proto_init(net, AF_INET6);
+	return xt_proto_init(net, NFPROTO_IPV6);
 }
 
 static void __net_exit ip6_tables_net_exit(struct net *net)
 {
-	xt_proto_fini(net, AF_INET6);
+	xt_proto_fini(net, NFPROTO_IPV6);
 }
 
 static struct pernet_operations ip6_tables_net_ops = {
-- 
1.6.2.2

[PATCH 02/11] netfilter: queue: use NFPROTO_ for queue callsites

[Jan Engelhardt]

af is an nfproto.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/ip_queue.c  |    2 +-
 net/ipv6/netfilter/ip6_queue.c |    2 +-
 net/netfilter/nf_queue.c       |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
index 5f22c91..c156db2 100644
--- a/net/ipv4/netfilter/ip_queue.c
+++ b/net/ipv4/netfilter/ip_queue.c
@@ -596,7 +596,7 @@ static int __init ip_queue_init(void)
 #ifdef CONFIG_SYSCTL
 	ipq_sysctl_header = register_sysctl_paths(net_ipv4_ctl_path, ipq_table);
 #endif
-	status = nf_register_queue_handler(PF_INET, &nfqh);
+	status = nf_register_queue_handler(NFPROTO_IPV4, &nfqh);
 	if (status < 0) {
 		printk(KERN_ERR "ip_queue: failed to register queue handler\n");
 		goto cleanup_sysctl;
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
index b693f84..1cf3f0c 100644
--- a/net/ipv6/netfilter/ip6_queue.c
+++ b/net/ipv6/netfilter/ip6_queue.c
@@ -598,7 +598,7 @@ static int __init ip6_queue_init(void)
 #ifdef CONFIG_SYSCTL
 	ipq_sysctl_header = register_sysctl_paths(net_ipv6_ctl_path, ipq_table);
 #endif
-	status = nf_register_queue_handler(PF_INET6, &nfqh);
+	status = nf_register_queue_handler(NFPROTO_IPV6, &nfqh);
 	if (status < 0) {
 		printk(KERN_ERR "ip6_queue: failed to register queue handler\n");
 		goto cleanup_sysctl;
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 4f2310c..3a6fd77 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -204,10 +204,10 @@ int nf_queue(struct sk_buff *skb,
 				  queuenum);
 
 	switch (pf) {
-	case AF_INET:
+	case NFPROTO_IPV4:
 		skb->protocol = htons(ETH_P_IP);
 		break;
-	case AF_INET6:
+	case NFPROTO_IPV6:
 		skb->protocol = htons(ETH_P_IPV6);
 		break;
 	}
-- 
1.6.2.2

[PATCH 03/11] netfilter: xtables: use NFPROTO_ in standard targets

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/ip_tables.c  |    6 +++---
 net/ipv6/netfilter/ip6_tables.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 3051dda..5abc276 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -2158,7 +2158,7 @@ static bool icmp_checkentry(const struct xt_mtchk_param *par)
 static struct xt_target ipt_standard_target __read_mostly = {
 	.name		= IPT_STANDARD_TARGET,
 	.targetsize	= sizeof(int),
-	.family		= AF_INET,
+	.family		= NFPROTO_IPV4,
 #ifdef CONFIG_COMPAT
 	.compatsize	= sizeof(compat_int_t),
 	.compat_from_user = compat_standard_from_user,
@@ -2170,7 +2170,7 @@ static struct xt_target ipt_error_target __read_mostly = {
 	.name		= IPT_ERROR_TARGET,
 	.target		= ipt_error,
 	.targetsize	= IPT_FUNCTION_MAXNAMELEN,
-	.family		= AF_INET,
+	.family		= NFPROTO_IPV4,
 };
 
 static struct nf_sockopt_ops ipt_sockopts = {
@@ -2196,7 +2196,7 @@ static struct xt_match icmp_matchstruct __read_mostly = {
 	.matchsize	= sizeof(struct ipt_icmp),
 	.checkentry	= icmp_checkentry,
 	.proto		= IPPROTO_ICMP,
-	.family		= AF_INET,
+	.family		= NFPROTO_IPV4,
 };
 
 static int __net_init ip_tables_net_init(struct net *net)
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 41fbc1e..37a850e 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -2191,7 +2191,7 @@ static bool icmp6_checkentry(const struct xt_mtchk_param *par)
 static struct xt_target ip6t_standard_target __read_mostly = {
 	.name		= IP6T_STANDARD_TARGET,
 	.targetsize	= sizeof(int),
-	.family		= AF_INET6,
+	.family		= NFPROTO_IPV6,
 #ifdef CONFIG_COMPAT
 	.compatsize	= sizeof(compat_int_t),
 	.compat_from_user = compat_standard_from_user,
@@ -2203,7 +2203,7 @@ static struct xt_target ip6t_error_target __read_mostly = {
 	.name		= IP6T_ERROR_TARGET,
 	.target		= ip6t_error,
 	.targetsize	= IP6T_FUNCTION_MAXNAMELEN,
-	.family		= AF_INET6,
+	.family		= NFPROTO_IPV6,
 };
 
 static struct nf_sockopt_ops ip6t_sockopts = {
@@ -2229,7 +2229,7 @@ static struct xt_match icmp6_matchstruct __read_mostly = {
 	.matchsize	= sizeof(struct ip6t_icmp),
 	.checkentry	= icmp6_checkentry,
 	.proto		= IPPROTO_ICMPV6,
-	.family		= AF_INET6,
+	.family		= NFPROTO_IPV6,
 };
 
 static int __net_init ip6_tables_net_init(struct net *net)
-- 
1.6.2.2

[PATCH 04/11] netfilter: xtables: remove redundant casts

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/ip_tables.c  |    2 +-
 net/ipv6/netfilter/ip6_tables.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 5abc276..7fcbc16 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -280,7 +280,7 @@ static void trace_packet(struct sk_buff *skb,
 	char *hookname, *chainname, *comment;
 	unsigned int rulenum = 0;
 
-	table_base = (void *)private->entries[smp_processor_id()];
+	table_base = private->entries[smp_processor_id()];
 	root = get_entry(table_base, private->hook_entry[hook]);
 
 	hookname = chainname = (char *)hooknames[hook];
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 37a850e..2b52fac 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -312,7 +312,7 @@ static void trace_packet(struct sk_buff *skb,
 	char *hookname, *chainname, *comment;
 	unsigned int rulenum = 0;
 
-	table_base = (void *)private->entries[smp_processor_id()];
+	table_base = private->entries[smp_processor_id()];
 	root = get_entry(table_base, private->hook_entry[hook]);
 
 	hookname = chainname = (char *)hooknames[hook];
-- 
1.6.2.2

[PATCH 05/11] netfilter: xtables: fix const inconsistency

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/ip_tables.c  |   14 +++++++-------
 net/ipv6/netfilter/ip6_tables.c |   14 +++++++-------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 7fcbc16..16b7c09 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -238,8 +238,8 @@ static struct nf_loginfo trace_loginfo = {
 /* Mildly perf critical (only if packet tracing is on) */
 static inline int
 get_chainname_rulenum(struct ipt_entry *s, struct ipt_entry *e,
-		      char *hookname, char **chainname,
-		      char **comment, unsigned int *rulenum)
+		      const char *hookname, const char **chainname,
+		      const char **comment, unsigned int *rulenum)
 {
 	struct ipt_standard_target *t = (void *)ipt_get_target(s);
 
@@ -257,8 +257,8 @@ get_chainname_rulenum(struct ipt_entry *s, struct ipt_entry *e,
 		   && unconditional(&s->ip)) {
 			/* Tail of chains: STANDARD target (return/policy) */
 			*comment = *chainname == hookname
-				? (char *)comments[NF_IP_TRACE_COMMENT_POLICY]
-				: (char *)comments[NF_IP_TRACE_COMMENT_RETURN];
+				? comments[NF_IP_TRACE_COMMENT_POLICY]
+				: comments[NF_IP_TRACE_COMMENT_RETURN];
 		}
 		return 1;
 	} else
@@ -277,14 +277,14 @@ static void trace_packet(struct sk_buff *skb,
 {
 	void *table_base;
 	const struct ipt_entry *root;
-	char *hookname, *chainname, *comment;
+	const char *hookname, *chainname, *comment;
 	unsigned int rulenum = 0;
 
 	table_base = private->entries[smp_processor_id()];
 	root = get_entry(table_base, private->hook_entry[hook]);
 
-	hookname = chainname = (char *)hooknames[hook];
-	comment = (char *)comments[NF_IP_TRACE_COMMENT_RULE];
+	hookname = chainname = hooknames[hook];
+	comment = comments[NF_IP_TRACE_COMMENT_RULE];
 
 	IPT_ENTRY_ITERATE(root,
 			  private->size - private->hook_entry[hook],
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 2b52fac..4853a3d 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -270,8 +270,8 @@ static struct nf_loginfo trace_loginfo = {
 /* Mildly perf critical (only if packet tracing is on) */
 static inline int
 get_chainname_rulenum(struct ip6t_entry *s, struct ip6t_entry *e,
-		      char *hookname, char **chainname,
-		      char **comment, unsigned int *rulenum)
+		      const char *hookname, const char **chainname,
+		      const char **comment, unsigned int *rulenum)
 {
 	struct ip6t_standard_target *t = (void *)ip6t_get_target(s);
 
@@ -289,8 +289,8 @@ get_chainname_rulenum(struct ip6t_entry *s, struct ip6t_entry *e,
 		   && unconditional(&s->ipv6)) {
 			/* Tail of chains: STANDARD target (return/policy) */
 			*comment = *chainname == hookname
-				? (char *)comments[NF_IP6_TRACE_COMMENT_POLICY]
-				: (char *)comments[NF_IP6_TRACE_COMMENT_RETURN];
+				? comments[NF_IP6_TRACE_COMMENT_POLICY]
+				: comments[NF_IP6_TRACE_COMMENT_RETURN];
 		}
 		return 1;
 	} else
@@ -309,14 +309,14 @@ static void trace_packet(struct sk_buff *skb,
 {
 	void *table_base;
 	const struct ip6t_entry *root;
-	char *hookname, *chainname, *comment;
+	const char *hookname, *chainname, *comment;
 	unsigned int rulenum = 0;
 
 	table_base = private->entries[smp_processor_id()];
 	root = get_entry(table_base, private->hook_entry[hook]);
 
-	hookname = chainname = (char *)hooknames[hook];
-	comment = (char *)comments[NF_IP6_TRACE_COMMENT_RULE];
+	hookname = chainname = hooknames[hook];
+	comment = comments[NF_IP6_TRACE_COMMENT_RULE];
 
 	IP6T_ENTRY_ITERATE(root,
 			   private->size - private->hook_entry[hook],
-- 
1.6.2.2

[PATCH 06/11] netfilter: xtables: consolidate open-coded logic

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/bridge/netfilter/ebtables.c |   16 ++++++++++------
 net/ipv4/netfilter/arp_tables.c |   14 ++++++++++----
 net/ipv4/netfilter/ip_tables.c  |   14 ++++++++++----
 net/ipv6/netfilter/ip6_tables.c |   14 ++++++++++----
 4 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 820252a..2455583 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -142,6 +142,12 @@ static inline int ebt_basic_match(struct ebt_entry *e, struct ethhdr *h,
 	return 0;
 }
 
+static inline __pure
+struct ebt_entry *ebt_next_entry(const struct ebt_entry *entry)
+{
+	return (void *)entry + entry->next_offset;
+}
+
 /* Do some firewalling */
 unsigned int ebt_do_table (unsigned int hook, struct sk_buff *skb,
    const struct net_device *in, const struct net_device *out,
@@ -249,8 +255,7 @@ letsreturn:
 		/* jump to a udc */
 		cs[sp].n = i + 1;
 		cs[sp].chaininfo = chaininfo;
-		cs[sp].e = (struct ebt_entry *)
-		   (((char *)point) + point->next_offset);
+		cs[sp].e = ebt_next_entry(point);
 		i = 0;
 		chaininfo = (struct ebt_entries *) (base + verdict);
 #ifdef CONFIG_NETFILTER_DEBUG
@@ -266,8 +271,7 @@ letsreturn:
 		sp++;
 		continue;
 letscontinue:
-		point = (struct ebt_entry *)
-		   (((char *)point) + point->next_offset);
+		point = ebt_next_entry(point);
 		i++;
 	}
 
@@ -787,7 +791,7 @@ static int check_chainloops(struct ebt_entries *chain, struct ebt_cl_stack *cl_s
 			/* this can't be 0, so the loop test is correct */
 			cl_s[i].cs.n = pos + 1;
 			pos = 0;
-			cl_s[i].cs.e = ((void *)e + e->next_offset);
+			cl_s[i].cs.e = ebt_next_entry(e);
 			e = (struct ebt_entry *)(hlp2->data);
 			nentries = hlp2->nentries;
 			cl_s[i].from = chain_nr;
@@ -797,7 +801,7 @@ static int check_chainloops(struct ebt_entries *chain, struct ebt_cl_stack *cl_s
 			continue;
 		}
 letscontinue:
-		e = (void *)e + e->next_offset;
+		e = ebt_next_entry(e);
 		pos++;
 	}
 	return 0;
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 831fe18..940e54b 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -231,6 +231,12 @@ static inline struct arpt_entry *get_entry(void *base, unsigned int offset)
 	return (struct arpt_entry *)(base + offset);
 }
 
+static inline __pure
+struct arpt_entry *arpt_next_entry(const struct arpt_entry *entry)
+{
+	return (void *)entry + entry->next_offset;
+}
+
 unsigned int arpt_do_table(struct sk_buff *skb,
 			   unsigned int hook,
 			   const struct net_device *in,
@@ -295,10 +301,10 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 					continue;
 				}
 				if (table_base + v
-				    != (void *)e + e->next_offset) {
+				    != arpt_next_entry(e)) {
 					/* Save old back ptr in next entry */
 					struct arpt_entry *next
-						= (void *)e + e->next_offset;
+						= arpt_next_entry(e);
 					next->comefrom =
 						(void *)back - table_base;
 
@@ -320,13 +326,13 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 				arp = arp_hdr(skb);
 
 				if (verdict == ARPT_CONTINUE)
-					e = (void *)e + e->next_offset;
+					e = arpt_next_entry(e);
 				else
 					/* Verdict */
 					break;
 			}
 		} else {
-			e = (void *)e + e->next_offset;
+			e = arpt_next_entry(e);
 		}
 	} while (!hotdrop);
 	xt_info_rdunlock_bh();
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 16b7c09..7ec4e40 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -297,6 +297,12 @@ static void trace_packet(struct sk_buff *skb,
 }
 #endif
 
+static inline __pure
+struct ipt_entry *ipt_next_entry(const struct ipt_entry *entry)
+{
+	return (void *)entry + entry->next_offset;
+}
+
 /* Returns one of the generic firewall policies, like NF_ACCEPT. */
 unsigned int
 ipt_do_table(struct sk_buff *skb,
@@ -385,11 +391,11 @@ ipt_do_table(struct sk_buff *skb,
 							 back->comefrom);
 					continue;
 				}
-				if (table_base + v != (void *)e + e->next_offset
+				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
-						= (void *)e + e->next_offset;
+						= ipt_next_entry(e);
 					next->comefrom
 						= (void *)back - table_base;
 					/* set back pointer to next entry */
@@ -424,7 +430,7 @@ ipt_do_table(struct sk_buff *skb,
 				datalen = skb->len - ip->ihl * 4;
 
 				if (verdict == IPT_CONTINUE)
-					e = (void *)e + e->next_offset;
+					e = ipt_next_entry(e);
 				else
 					/* Verdict */
 					break;
@@ -432,7 +438,7 @@ ipt_do_table(struct sk_buff *skb,
 		} else {
 
 		no_match:
-			e = (void *)e + e->next_offset;
+			e = ipt_next_entry(e);
 		}
 	} while (!hotdrop);
 	xt_info_rdunlock_bh();
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 4853a3d..9176e98 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -329,6 +329,12 @@ static void trace_packet(struct sk_buff *skb,
 }
 #endif
 
+static inline __pure struct ip6t_entry *
+ip6t_next_entry(const struct ip6t_entry *entry)
+{
+	return (void *)entry + entry->next_offset;
+}
+
 /* Returns one of the generic firewall policies, like NF_ACCEPT. */
 unsigned int
 ip6t_do_table(struct sk_buff *skb,
@@ -414,11 +420,11 @@ ip6t_do_table(struct sk_buff *skb,
 							 back->comefrom);
 					continue;
 				}
-				if (table_base + v != (void *)e + e->next_offset
+				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
-						= (void *)e + e->next_offset;
+						= ip6t_next_entry(e);
 					next->comefrom
 						= (void *)back - table_base;
 					/* set back pointer to next entry */
@@ -451,7 +457,7 @@ ip6t_do_table(struct sk_buff *skb,
 					= 0x57acc001;
 #endif
 				if (verdict == IP6T_CONTINUE)
-					e = (void *)e + e->next_offset;
+					e = ip6t_next_entry(e);
 				else
 					/* Verdict */
 					break;
@@ -459,7 +465,7 @@ ip6t_do_table(struct sk_buff *skb,
 		} else {
 
 		no_match:
-			e = (void *)e + e->next_offset;
+			e = ip6t_next_entry(e);
 		}
 	} while (!hotdrop);
 
-- 
1.6.2.2

[PATCH 07/11] netfilter: xtables: reduce indent level by one

[Jan Engelhardt]

Cosmetic only. Transformation applied:

	-if (foo) { long block; } else { short block; }
	+if (!foo) { short block; continue; } long block;

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/arp_tables.c |  108 +++++++++++++++----------------
 net/ipv4/netfilter/ip_tables.c  |  134 +++++++++++++++++++--------------------
 net/ipv6/netfilter/ip6_tables.c |  132 +++++++++++++++++++--------------------
 3 files changed, 181 insertions(+), 193 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 940e54b..ecba246 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -273,66 +273,62 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 
 	arp = arp_hdr(skb);
 	do {
-		if (arp_packet_match(arp, skb->dev, indev, outdev, &e->arp)) {
-			struct arpt_entry_target *t;
-			int hdr_len;
+		struct arpt_entry_target *t;
+		int hdr_len;
 
-			hdr_len = sizeof(*arp) + (2 * sizeof(struct in_addr)) +
-				(2 * skb->dev->addr_len);
+		if (!arp_packet_match(arp, skb->dev, indev, outdev, &e->arp)) {
+			e = arpt_next_entry(e);
+			continue;
+		}
 
-			ADD_COUNTER(e->counters, hdr_len, 1);
+		hdr_len = sizeof(*arp) + (2 * sizeof(struct in_addr)) +
+			(2 * skb->dev->addr_len);
+		ADD_COUNTER(e->counters, hdr_len, 1);
 
-			t = arpt_get_target(e);
+		t = arpt_get_target(e);
 
-			/* Standard target? */
-			if (!t->u.kernel.target->target) {
-				int v;
+		/* Standard target? */
+		if (!t->u.kernel.target->target) {
+			int v;
 
-				v = ((struct arpt_standard_target *)t)->verdict;
-				if (v < 0) {
-					/* Pop from stack? */
-					if (v != ARPT_RETURN) {
-						verdict = (unsigned)(-v) - 1;
-						break;
-					}
-					e = back;
-					back = get_entry(table_base,
-							 back->comefrom);
-					continue;
-				}
-				if (table_base + v
-				    != arpt_next_entry(e)) {
-					/* Save old back ptr in next entry */
-					struct arpt_entry *next
-						= arpt_next_entry(e);
-					next->comefrom =
-						(void *)back - table_base;
-
-					/* set back pointer to next entry */
-					back = next;
-				}
-
-				e = get_entry(table_base, v);
-			} else {
-				/* Targets which reenter must return
-				 * abs. verdicts
-				 */
-				tgpar.target   = t->u.kernel.target;
-				tgpar.targinfo = t->data;
-				verdict = t->u.kernel.target->target(skb,
-								     &tgpar);
-
-				/* Target might have changed stuff. */
-				arp = arp_hdr(skb);
-
-				if (verdict == ARPT_CONTINUE)
-					e = arpt_next_entry(e);
-				else
-					/* Verdict */
+			v = ((struct arpt_standard_target *)t)->verdict;
+			if (v < 0) {
+				/* Pop from stack? */
+				if (v != ARPT_RETURN) {
+					verdict = (unsigned)(-v) - 1;
 					break;
+				}
+				e = back;
+				back = get_entry(table_base, back->comefrom);
+				continue;
 			}
+			if (table_base + v
+			    != arpt_next_entry(e)) {
+				/* Save old back ptr in next entry */
+				struct arpt_entry *next = arpt_next_entry(e);
+				next->comefrom = (void *)back - table_base;
+
+				/* set back pointer to next entry */
+				back = next;
+			}
+
+			e = get_entry(table_base, v);
 		} else {
-			e = arpt_next_entry(e);
+			/* Targets which reenter must return
+			 * abs. verdicts
+			 */
+			tgpar.target   = t->u.kernel.target;
+			tgpar.targinfo = t->data;
+			verdict = t->u.kernel.target->target(skb, &tgpar);
+
+			/* Target might have changed stuff. */
+			arp = arp_hdr(skb);
+
+			if (verdict == ARPT_CONTINUE)
+				e = arpt_next_entry(e);
+			else
+				/* Verdict */
+				break;
 		}
 	} while (!hotdrop);
 	xt_info_rdunlock_bh();
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 7ec4e40..82888bc 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -354,91 +354,87 @@ ipt_do_table(struct sk_buff *skb,
 	back = get_entry(table_base, private->underflow[hook]);
 
 	do {
+		struct ipt_entry_target *t;
+
 		IP_NF_ASSERT(e);
 		IP_NF_ASSERT(back);
-		if (ip_packet_match(ip, indev, outdev,
+		if (!ip_packet_match(ip, indev, outdev,
 		    &e->ip, mtpar.fragoff)) {
-			struct ipt_entry_target *t;
+ no_match:
+			e = ipt_next_entry(e);
+			continue;
+		}
 
-			if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
-				goto no_match;
+		if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
+			goto no_match;
 
-			ADD_COUNTER(e->counters, ntohs(ip->tot_len), 1);
+		ADD_COUNTER(e->counters, ntohs(ip->tot_len), 1);
 
-			t = ipt_get_target(e);
-			IP_NF_ASSERT(t->u.kernel.target);
+		t = ipt_get_target(e);
+		IP_NF_ASSERT(t->u.kernel.target);
 
 #if defined(CONFIG_NETFILTER_XT_TARGET_TRACE) || \
     defined(CONFIG_NETFILTER_XT_TARGET_TRACE_MODULE)
-			/* The packet is traced: log it */
-			if (unlikely(skb->nf_trace))
-				trace_packet(skb, hook, in, out,
-					     table->name, private, e);
+		/* The packet is traced: log it */
+		if (unlikely(skb->nf_trace))
+			trace_packet(skb, hook, in, out,
+				     table->name, private, e);
 #endif
-			/* Standard target? */
-			if (!t->u.kernel.target->target) {
-				int v;
-
-				v = ((struct ipt_standard_target *)t)->verdict;
-				if (v < 0) {
-					/* Pop from stack? */
-					if (v != IPT_RETURN) {
-						verdict = (unsigned)(-v) - 1;
-						break;
-					}
-					e = back;
-					back = get_entry(table_base,
-							 back->comefrom);
-					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;
+		/* Standard target? */
+		if (!t->u.kernel.target->target) {
+			int v;
+
+			v = ((struct ipt_standard_target *)t)->verdict;
+			if (v < 0) {
+				/* Pop from stack? */
+				if (v != IPT_RETURN) {
+					verdict = (unsigned)(-v) - 1;
+					break;
 				}
+				e = back;
+				back = get_entry(table_base, back->comefrom);
+				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;
+			}
 
-				e = get_entry(table_base, v);
-			} else {
-				/* Targets which reenter must return
-				   abs. verdicts */
-				tgpar.target   = t->u.kernel.target;
-				tgpar.targinfo = t->data;
+			e = get_entry(table_base, v);
+		} else {
+			/* Targets which reenter must return
+			   abs. verdicts */
+			tgpar.target   = t->u.kernel.target;
+			tgpar.targinfo = t->data;
 #ifdef CONFIG_NETFILTER_DEBUG
-				((struct ipt_entry *)table_base)->comefrom
-					= 0xeeeeeeec;
+			((struct ipt_entry *)table_base)->comefrom
+				= 0xeeeeeeec;
 #endif
-				verdict = t->u.kernel.target->target(skb,
-								     &tgpar);
+			verdict = t->u.kernel.target->target(skb, &tgpar);
 #ifdef CONFIG_NETFILTER_DEBUG
-				if (((struct ipt_entry *)table_base)->comefrom
-				    != 0xeeeeeeec
-				    && verdict == IPT_CONTINUE) {
-					printk("Target %s reentered!\n",
-					       t->u.kernel.target->name);
-					verdict = NF_DROP;
-				}
-				((struct ipt_entry *)table_base)->comefrom
-					= 0x57acc001;
-#endif
-				/* Target might have changed stuff. */
-				ip = ip_hdr(skb);
-				datalen = skb->len - ip->ihl * 4;
-
-				if (verdict == IPT_CONTINUE)
-					e = ipt_next_entry(e);
-				else
-					/* Verdict */
-					break;
+			if (((struct ipt_entry *)table_base)->comefrom
+			    != 0xeeeeeeec
+			    && verdict == IPT_CONTINUE) {
+				printk("Target %s reentered!\n",
+				       t->u.kernel.target->name);
+				verdict = NF_DROP;
 			}
-		} else {
-
-		no_match:
-			e = ipt_next_entry(e);
+			((struct ipt_entry *)table_base)->comefrom
+				= 0x57acc001;
+#endif
+			/* Target might have changed stuff. */
+			ip = ip_hdr(skb);
+			datalen = skb->len - ip->ihl * 4;
+
+			if (verdict == IPT_CONTINUE)
+				e = ipt_next_entry(e);
+			else
+				/* Verdict */
+				break;
 		}
 	} while (!hotdrop);
 	xt_info_rdunlock_bh();
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 9176e98..a6c2213 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -381,91 +381,87 @@ ip6t_do_table(struct sk_buff *skb,
 	back = get_entry(table_base, private->underflow[hook]);
 
 	do {
+		struct ip6t_entry_target *t;
+
 		IP_NF_ASSERT(e);
 		IP_NF_ASSERT(back);
-		if (ip6_packet_match(skb, indev, outdev, &e->ipv6,
-			&mtpar.thoff, &mtpar.fragoff, &hotdrop)) {
-			struct ip6t_entry_target *t;
+		if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
+		    &mtpar.thoff, &mtpar.fragoff, &hotdrop)) {
+ no_match:
+			e = ip6t_next_entry(e);
+			continue;
+		}
 
-			if (IP6T_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
-				goto no_match;
+		if (IP6T_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
+			goto no_match;
 
-			ADD_COUNTER(e->counters,
-				    ntohs(ipv6_hdr(skb)->payload_len) +
-				    sizeof(struct ipv6hdr), 1);
+		ADD_COUNTER(e->counters,
+			    ntohs(ipv6_hdr(skb)->payload_len) +
+			    sizeof(struct ipv6hdr), 1);
 
-			t = ip6t_get_target(e);
-			IP_NF_ASSERT(t->u.kernel.target);
+		t = ip6t_get_target(e);
+		IP_NF_ASSERT(t->u.kernel.target);
 
 #if defined(CONFIG_NETFILTER_XT_TARGET_TRACE) || \
     defined(CONFIG_NETFILTER_XT_TARGET_TRACE_MODULE)
-			/* The packet is traced: log it */
-			if (unlikely(skb->nf_trace))
-				trace_packet(skb, hook, in, out,
-					     table->name, private, e);
+		/* The packet is traced: log it */
+		if (unlikely(skb->nf_trace))
+			trace_packet(skb, hook, in, out,
+				     table->name, private, e);
 #endif
-			/* Standard target? */
-			if (!t->u.kernel.target->target) {
-				int v;
-
-				v = ((struct ip6t_standard_target *)t)->verdict;
-				if (v < 0) {
-					/* Pop from stack? */
-					if (v != IP6T_RETURN) {
-						verdict = (unsigned)(-v) - 1;
-						break;
-					}
-					e = back;
-					back = get_entry(table_base,
-							 back->comefrom);
-					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;
+		/* Standard target? */
+		if (!t->u.kernel.target->target) {
+			int v;
+
+			v = ((struct ip6t_standard_target *)t)->verdict;
+			if (v < 0) {
+				/* Pop from stack? */
+				if (v != IP6T_RETURN) {
+					verdict = (unsigned)(-v) - 1;
+					break;
 				}
+				e = back;
+				back = get_entry(table_base, back->comefrom);
+				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;
+			}
 
-				e = get_entry(table_base, v);
-			} else {
-				/* Targets which reenter must return
-				   abs. verdicts */
-				tgpar.target   = t->u.kernel.target;
-				tgpar.targinfo = t->data;
+			e = get_entry(table_base, v);
+		} else {
+			/* Targets which reenter must return
+			   abs. verdicts */
+			tgpar.target   = t->u.kernel.target;
+			tgpar.targinfo = t->data;
 
 #ifdef CONFIG_NETFILTER_DEBUG
-				((struct ip6t_entry *)table_base)->comefrom
-					= 0xeeeeeeec;
+			((struct ip6t_entry *)table_base)->comefrom
+				= 0xeeeeeeec;
 #endif
-				verdict = t->u.kernel.target->target(skb,
-								     &tgpar);
+			verdict = t->u.kernel.target->target(skb, &tgpar);
 
 #ifdef CONFIG_NETFILTER_DEBUG
-				if (((struct ip6t_entry *)table_base)->comefrom
-				    != 0xeeeeeeec
-				    && verdict == IP6T_CONTINUE) {
-					printk("Target %s reentered!\n",
-					       t->u.kernel.target->name);
-					verdict = NF_DROP;
-				}
-				((struct ip6t_entry *)table_base)->comefrom
-					= 0x57acc001;
-#endif
-				if (verdict == IP6T_CONTINUE)
-					e = ip6t_next_entry(e);
-				else
-					/* Verdict */
-					break;
+			if (((struct ip6t_entry *)table_base)->comefrom
+			    != 0xeeeeeeec
+			    && verdict == IP6T_CONTINUE) {
+				printk("Target %s reentered!\n",
+				       t->u.kernel.target->name);
+				verdict = NF_DROP;
 			}
-		} else {
-
-		no_match:
-			e = ip6t_next_entry(e);
+			((struct ip6t_entry *)table_base)->comefrom
+				= 0x57acc001;
+#endif
+			if (verdict == IP6T_CONTINUE)
+				e = ip6t_next_entry(e);
+			else
+				/* Verdict */
+				break;
 		}
 	} while (!hotdrop);
 
-- 
1.6.2.2

Re: [PATCH 07/11] netfilter: xtables: reduce indent level by one

[Amos Jeffries]

Jan Engelhardt wrote:
> Cosmetic only. Transformation applied:
> 
> 	-if (foo) { long block; } else { short block; }
> 	+if (!foo) { short block; continue; } long block;
> 
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  net/ipv4/netfilter/arp_tables.c |  108 +++++++++++++++----------------
>  net/ipv4/netfilter/ip_tables.c  |  134 +++++++++++++++++++--------------------
>  net/ipv6/netfilter/ip6_tables.c |  132 +++++++++++++++++++--------------------
>  3 files changed, 181 insertions(+), 193 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
> index 940e54b..ecba246 100644
> --- a/net/ipv4/netfilter/arp_tables.c
> +++ b/net/ipv4/netfilter/arp_tables.c
> @@ -273,66 +273,62 @@ unsigned int arpt_do_table(struct sk_buff *skb,
>  
>  	arp = arp_hdr(skb);
>  	do {
> -		if (arp_packet_match(arp, skb->dev, indev, outdev, &e->arp)) {
> -			struct arpt_entry_target *t;
> -			int hdr_len;
> +		struct arpt_entry_target *t;
> +		int hdr_len;
>  
> -			hdr_len = sizeof(*arp) + (2 * sizeof(struct in_addr)) +
> -				(2 * skb->dev->addr_len);
> +		if (!arp_packet_match(arp, skb->dev, indev, outdev, &e->arp)) {
> +			e = arpt_next_entry(e);
> +			continue;
> +		}
>  
> -			ADD_COUNTER(e->counters, hdr_len, 1);
> +		hdr_len = sizeof(*arp) + (2 * sizeof(struct in_addr)) +
> +			(2 * skb->dev->addr_len);
> +		ADD_COUNTER(e->counters, hdr_len, 1);
>  
> -			t = arpt_get_target(e);
> +		t = arpt_get_target(e);
>  
> -			/* Standard target? */
> -			if (!t->u.kernel.target->target) {
> -				int v;
> +		/* Standard target? */
> +		if (!t->u.kernel.target->target) {
> +			int v;
>  
> -				v = ((struct arpt_standard_target *)t)->verdict;
> -				if (v < 0) {
> -					/* Pop from stack? */
> -					if (v != ARPT_RETURN) {
> -						verdict = (unsigned)(-v) - 1;
> -						break;
> -					}
> -					e = back;
> -					back = get_entry(table_base,
> -							 back->comefrom);
> -					continue;
> -				}
> -				if (table_base + v
> -				    != arpt_next_entry(e)) {
> -					/* Save old back ptr in next entry */
> -					struct arpt_entry *next
> -						= arpt_next_entry(e);
> -					next->comefrom =
> -						(void *)back - table_base;
> -
> -					/* set back pointer to next entry */
> -					back = next;
> -				}
> -
> -				e = get_entry(table_base, v);
> -			} else {
> -				/* Targets which reenter must return
> -				 * abs. verdicts
> -				 */
> -				tgpar.target   = t->u.kernel.target;
> -				tgpar.targinfo = t->data;
> -				verdict = t->u.kernel.target->target(skb,
> -								     &tgpar);
> -
> -				/* Target might have changed stuff. */
> -				arp = arp_hdr(skb);
> -
> -				if (verdict == ARPT_CONTINUE)
> -					e = arpt_next_entry(e);
> -				else
> -					/* Verdict */
> +			v = ((struct arpt_standard_target *)t)->verdict;
> +			if (v < 0) {
> +				/* Pop from stack? */
> +				if (v != ARPT_RETURN) {
> +					verdict = (unsigned)(-v) - 1;
>  					break;
> +				}
> +				e = back;
> +				back = get_entry(table_base, back->comefrom);
> +				continue;
>  			}
> +			if (table_base + v
> +			    != arpt_next_entry(e)) {
> +				/* Save old back ptr in next entry */
> +				struct arpt_entry *next = arpt_next_entry(e);
> +				next->comefrom = (void *)back - table_base;
> +
> +				/* set back pointer to next entry */
> +				back = next;
> +			}
> +
> +			e = get_entry(table_base, v);
>  		} else {
> -			e = arpt_next_entry(e);
> +			/* Targets which reenter must return
> +			 * abs. verdicts
> +			 */
> +			tgpar.target   = t->u.kernel.target;
> +			tgpar.targinfo = t->data;
> +			verdict = t->u.kernel.target->target(skb, &tgpar);
> +
> +			/* Target might have changed stuff. */
> +			arp = arp_hdr(skb);
> +
> +			if (verdict == ARPT_CONTINUE)
> +				e = arpt_next_entry(e);
> +			else
> +				/* Verdict */
> +				break;
>  		}
>  	} while (!hotdrop);
>  	xt_info_rdunlock_bh();
> diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
> index 7ec4e40..82888bc 100644
> --- a/net/ipv4/netfilter/ip_tables.c
> +++ b/net/ipv4/netfilter/ip_tables.c
> @@ -354,91 +354,87 @@ ipt_do_table(struct sk_buff *skb,
>  	back = get_entry(table_base, private->underflow[hook]);
>  
>  	do {
> +		struct ipt_entry_target *t;
> +
>  		IP_NF_ASSERT(e);
>  		IP_NF_ASSERT(back);
> -		if (ip_packet_match(ip, indev, outdev,
> +		if (!ip_packet_match(ip, indev, outdev,
>  		    &e->ip, mtpar.fragoff)) {
> -			struct ipt_entry_target *t;
> + no_match:
> +			e = ipt_next_entry(e);
> +			continue;
> +		}
>  
> -			if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
> -				goto no_match;
> +		if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
> +			goto no_match;
>  

Didn't your patch just earlier remove these no_match jumps from xtables?
Seems a bit odd to be adding them in here again and in the v6 tables as 
well.


Amos

Re: [PATCH 07/11] netfilter: xtables: reduce indent level by one

[Jan Engelhardt]

On Saturday 2009-05-09 04:08, Amos Jeffries wrote:
> Jan Engelhardt wrote:
>> Cosmetic only. Transformation applied:
>>
>> 	-if (foo) { long block; } else { short block; }
>> 	+if (!foo) { short block; continue; } long block;
>>
>> -			if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
>> -				goto no_match;
>> +		if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
>> +			goto no_match;
>
> Didn't your patch just earlier remove these no_match jumps from xtables?

You are looking at patch #07/11, the gotos are removed in patch #08/11.

Re: [PATCH 07/11] netfilter: xtables: reduce indent level by one

[Amos Jeffries]

Jan Engelhardt wrote:
> On Saturday 2009-05-09 04:08, Amos Jeffries wrote:
>> Jan Engelhardt wrote:
>>> Cosmetic only. Transformation applied:
>>>
>>> 	-if (foo) { long block; } else { short block; }
>>> 	+if (!foo) { short block; continue; } long block;
>>>
>>> -			if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
>>> -				goto no_match;
>>> +		if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
>>> +			goto no_match;
>> Didn't your patch just earlier remove these no_match jumps from xtables?
> 
> You are looking at patch #07/11, the gotos are removed in patch #08/11.

Ah, the weirdness introduced by splitting patches in to small tasks.
Gotcha.

Amos

[PATCH 08/11] netfilter: xtables: remove some goto

[Jan Engelhardt]

Combining two ifs, and goto is easily gone.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/ip_tables.c  |    7 ++-----
 net/ipv6/netfilter/ip6_tables.c |    7 ++-----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 82888bc..cfcb7af 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -359,15 +359,12 @@ ipt_do_table(struct sk_buff *skb,
 		IP_NF_ASSERT(e);
 		IP_NF_ASSERT(back);
 		if (!ip_packet_match(ip, indev, outdev,
-		    &e->ip, mtpar.fragoff)) {
- no_match:
+		    &e->ip, mtpar.fragoff) ||
+		    IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0) {
 			e = ipt_next_entry(e);
 			continue;
 		}
 
-		if (IPT_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
-			goto no_match;
-
 		ADD_COUNTER(e->counters, ntohs(ip->tot_len), 1);
 
 		t = ipt_get_target(e);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index a6c2213..d01b8a3 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -386,15 +386,12 @@ ip6t_do_table(struct sk_buff *skb,
 		IP_NF_ASSERT(e);
 		IP_NF_ASSERT(back);
 		if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
-		    &mtpar.thoff, &mtpar.fragoff, &hotdrop)) {
- no_match:
+		    &mtpar.thoff, &mtpar.fragoff, &hotdrop) ||
+		    IP6T_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0) {
 			e = ip6t_next_entry(e);
 			continue;
 		}
 
-		if (IP6T_MATCH_ITERATE(e, do_match, skb, &mtpar) != 0)
-			goto no_match;
-
 		ADD_COUNTER(e->counters,
 			    ntohs(ipv6_hdr(skb)->payload_len) +
 			    sizeof(struct ipv6hdr), 1);
-- 
1.6.2.2

[PATCH 09/11] netfilter: xtables: remove another level of indent

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/arp_tables.c |   33 ++++++++++++------------
 net/ipv4/netfilter/ip_tables.c  |   52 ++++++++++++++++++--------------------
 net/ipv6/netfilter/ip6_tables.c |   44 +++++++++++++++-----------------
 3 files changed, 63 insertions(+), 66 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index ecba246..7505dff 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -313,23 +313,24 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 			}
 
 			e = get_entry(table_base, v);
-		} else {
-			/* Targets which reenter must return
-			 * abs. verdicts
-			 */
-			tgpar.target   = t->u.kernel.target;
-			tgpar.targinfo = t->data;
-			verdict = t->u.kernel.target->target(skb, &tgpar);
-
-			/* Target might have changed stuff. */
-			arp = arp_hdr(skb);
-
-			if (verdict == ARPT_CONTINUE)
-				e = arpt_next_entry(e);
-			else
-				/* Verdict */
-				break;
+			continue;
 		}
+
+		/* Targets which reenter must return
+		 * abs. verdicts
+		 */
+		tgpar.target   = t->u.kernel.target;
+		tgpar.targinfo = t->data;
+		verdict = t->u.kernel.target->target(skb, &tgpar);
+
+		/* Target might have changed stuff. */
+		arp = arp_hdr(skb);
+
+		if (verdict == ARPT_CONTINUE)
+			e = arpt_next_entry(e);
+		else
+			/* Verdict */
+			break;
 	} while (!hotdrop);
 	xt_info_rdunlock_bh();
 
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index cfcb7af..d91ecd4 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -402,37 +402,35 @@ ipt_do_table(struct sk_buff *skb,
 			}
 
 			e = get_entry(table_base, v);
-		} else {
-			/* Targets which reenter must return
-			   abs. verdicts */
-			tgpar.target   = t->u.kernel.target;
-			tgpar.targinfo = t->data;
+			continue;
+		}
+
+		/* Targets which reenter must return
+		   abs. verdicts */
+		tgpar.target   = t->u.kernel.target;
+		tgpar.targinfo = t->data;
 #ifdef CONFIG_NETFILTER_DEBUG
-			((struct ipt_entry *)table_base)->comefrom
-				= 0xeeeeeeec;
+		((struct ipt_entry *)table_base)->comefrom = 0xeeeeeeec;
 #endif
-			verdict = t->u.kernel.target->target(skb, &tgpar);
+		verdict = t->u.kernel.target->target(skb, &tgpar);
 #ifdef CONFIG_NETFILTER_DEBUG
-			if (((struct ipt_entry *)table_base)->comefrom
-			    != 0xeeeeeeec
-			    && verdict == IPT_CONTINUE) {
-				printk("Target %s reentered!\n",
-				       t->u.kernel.target->name);
-				verdict = NF_DROP;
-			}
-			((struct ipt_entry *)table_base)->comefrom
-				= 0x57acc001;
-#endif
-			/* Target might have changed stuff. */
-			ip = ip_hdr(skb);
-			datalen = skb->len - ip->ihl * 4;
-
-			if (verdict == IPT_CONTINUE)
-				e = ipt_next_entry(e);
-			else
-				/* Verdict */
-				break;
+		if (((struct ipt_entry *)table_base)->comefrom != 0xeeeeeeec &&
+		    verdict == IPT_CONTINUE) {
+			printk("Target %s reentered!\n",
+			       t->u.kernel.target->name);
+			verdict = NF_DROP;
 		}
+		((struct ipt_entry *)table_base)->comefrom = 0x57acc001;
+#endif
+		/* Target might have changed stuff. */
+		ip = ip_hdr(skb);
+		datalen = skb->len - ip->ihl * 4;
+
+		if (verdict == IPT_CONTINUE)
+			e = ipt_next_entry(e);
+		else
+			/* Verdict */
+			break;
 	} while (!hotdrop);
 	xt_info_rdunlock_bh();
 
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index d01b8a3..5a178be 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -431,35 +431,33 @@ ip6t_do_table(struct sk_buff *skb,
 			}
 
 			e = get_entry(table_base, v);
-		} else {
-			/* Targets which reenter must return
-			   abs. verdicts */
-			tgpar.target   = t->u.kernel.target;
-			tgpar.targinfo = t->data;
+			continue;
+		}
+
+		/* Targets which reenter must return
+		   abs. verdicts */
+		tgpar.target   = t->u.kernel.target;
+		tgpar.targinfo = t->data;
 
 #ifdef CONFIG_NETFILTER_DEBUG
-			((struct ip6t_entry *)table_base)->comefrom
-				= 0xeeeeeeec;
+		((struct ip6t_entry *)table_base)->comefrom = 0xeeeeeeec;
 #endif
-			verdict = t->u.kernel.target->target(skb, &tgpar);
+		verdict = t->u.kernel.target->target(skb, &tgpar);
 
 #ifdef CONFIG_NETFILTER_DEBUG
-			if (((struct ip6t_entry *)table_base)->comefrom
-			    != 0xeeeeeeec
-			    && verdict == IP6T_CONTINUE) {
-				printk("Target %s reentered!\n",
-				       t->u.kernel.target->name);
-				verdict = NF_DROP;
-			}
-			((struct ip6t_entry *)table_base)->comefrom
-				= 0x57acc001;
-#endif
-			if (verdict == IP6T_CONTINUE)
-				e = ip6t_next_entry(e);
-			else
-				/* Verdict */
-				break;
+		if (((struct ip6t_entry *)table_base)->comefrom != 0xeeeeeeec &&
+		    verdict == IP6T_CONTINUE) {
+			printk("Target %s reentered!\n",
+			       t->u.kernel.target->name);
+			verdict = NF_DROP;
 		}
+		((struct ip6t_entry *)table_base)->comefrom = 0x57acc001;
+#endif
+		if (verdict == IP6T_CONTINUE)
+			e = ip6t_next_entry(e);
+		else
+			/* Verdict */
+			break;
 	} while (!hotdrop);
 
 #ifdef CONFIG_NETFILTER_DEBUG
-- 
1.6.2.2

[PATCH 10/11] netfilter: xtables: consolidate comefrom debug cast access

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/ipv4/netfilter/ip_tables.c  |   13 +++++++++----
 net/ipv6/netfilter/ip6_tables.c |   13 ++++++++-----
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index d91ecd4..7b35c0b 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -311,6 +311,8 @@ 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;
 	u_int16_t datalen;
@@ -409,18 +411,19 @@ ipt_do_table(struct sk_buff *skb,
 		   abs. verdicts */
 		tgpar.target   = t->u.kernel.target;
 		tgpar.targinfo = t->data;
+
+
 #ifdef CONFIG_NETFILTER_DEBUG
-		((struct ipt_entry *)table_base)->comefrom = 0xeeeeeeec;
+		tb_comefrom = 0xeeeeeeec;
 #endif
 		verdict = t->u.kernel.target->target(skb, &tgpar);
 #ifdef CONFIG_NETFILTER_DEBUG
-		if (((struct ipt_entry *)table_base)->comefrom != 0xeeeeeeec &&
-		    verdict == IPT_CONTINUE) {
+		if (comefrom != 0xeeeeeeec && verdict == IPT_CONTINUE) {
 			printk("Target %s reentered!\n",
 			       t->u.kernel.target->name);
 			verdict = NF_DROP;
 		}
-		((struct ipt_entry *)table_base)->comefrom = 0x57acc001;
+		tb_comefrom = 0x57acc001;
 #endif
 		/* Target might have changed stuff. */
 		ip = ip_hdr(skb);
@@ -441,6 +444,8 @@ 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
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 5a178be..5164e0b 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -343,6 +343,8 @@ 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. */
@@ -440,18 +442,17 @@ ip6t_do_table(struct sk_buff *skb,
 		tgpar.targinfo = t->data;
 
 #ifdef CONFIG_NETFILTER_DEBUG
-		((struct ip6t_entry *)table_base)->comefrom = 0xeeeeeeec;
+		tb_comefrom = 0xeeeeeeec;
 #endif
 		verdict = t->u.kernel.target->target(skb, &tgpar);
 
 #ifdef CONFIG_NETFILTER_DEBUG
-		if (((struct ip6t_entry *)table_base)->comefrom != 0xeeeeeeec &&
-		    verdict == IP6T_CONTINUE) {
+		if (tb_comefrom != 0xeeeeeeec && verdict == IP6T_CONTINUE) {
 			printk("Target %s reentered!\n",
 			       t->u.kernel.target->name);
 			verdict = NF_DROP;
 		}
-		((struct ip6t_entry *)table_base)->comefrom = 0x57acc001;
+		tb_comefrom = 0x57acc001;
 #endif
 		if (verdict == IP6T_CONTINUE)
 			e = ip6t_next_entry(e);
@@ -461,7 +462,7 @@ ip6t_do_table(struct sk_buff *skb,
 	} while (!hotdrop);
 
 #ifdef CONFIG_NETFILTER_DEBUG
-	((struct ip6t_entry *)table_base)->comefrom = NETFILTER_LINK_POISON;
+	tb_comefrom = NETFILTER_LINK_POISON;
 #endif
 	xt_info_rdunlock_bh();
 
@@ -472,6 +473,8 @@ 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
-- 
1.6.2.2

[PATCH 11/11] netfilter: xtables: print hook name instead of mask

[Jan Engelhardt]

Users cannot make anything of these numbers. Let's just tell them
directly.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/netfilter/x_tables.c |   42 ++++++++++++++++++++++++++++++++++++++----
 1 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 150e5cf..46dba5f 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -329,6 +329,32 @@ int xt_find_revision(u8 af, const char *name, u8 revision, int target,
 }
 EXPORT_SYMBOL_GPL(xt_find_revision);
 
+static char *textify_hooks(char *buf, size_t size, unsigned int mask)
+{
+	static const char *const names[] = {
+		"PREROUTING", "INPUT", "FORWARD",
+		"OUTPUT", "POSTROUTING", "BROUTING",
+	};
+	unsigned int i;
+	char *p = buf;
+	bool np = false;
+	int res;
+
+	*p = '\0';
+	for (i = 0; i < ARRAY_SIZE(names); ++i) {
+		if (!(mask & (1 << i)))
+			continue;
+		res = snprintf(p, size, "%s%s", np ? "/" : "", names[i]);
+		if (res > 0) {
+			size -= res;
+			p += res;
+		}
+		np = true;
+	}
+
+	return buf;
+}
+
 int xt_check_match(struct xt_mtchk_param *par,
 		   unsigned int size, u_int8_t proto, bool inv_proto)
 {
@@ -351,9 +377,13 @@ int xt_check_match(struct xt_mtchk_param *par,
 		return -EINVAL;
 	}
 	if (par->match->hooks && (par->hook_mask & ~par->match->hooks) != 0) {
-		printk("%s_tables: %s match: bad hook_mask %#x/%#x\n",
+		char used[64], allow[64];
+
+		printk("%s_tables: %s match: used from hooks %s, but only "
+		       "valid from %s\n",
 		       xt_prefix[par->family], par->match->name,
-		       par->hook_mask, par->match->hooks);
+		       textify_hooks(used, sizeof(used), par->hook_mask),
+		       textify_hooks(allow, sizeof(allow), par->match->hooks));
 		return -EINVAL;
 	}
 	if (par->match->proto && (par->match->proto != proto || inv_proto)) {
@@ -497,9 +527,13 @@ int xt_check_target(struct xt_tgchk_param *par,
 		return -EINVAL;
 	}
 	if (par->target->hooks && (par->hook_mask & ~par->target->hooks) != 0) {
-		printk("%s_tables: %s target: bad hook_mask %#x/%#x\n",
+		char used[64], allow[64];
+
+		printk("%s_tables: %s target: used from hooks %s, but only "
+		       "usable from %s\n",
 		       xt_prefix[par->family], par->target->name,
-		       par->hook_mask, par->target->hooks);
+		       textify_hooks(used, sizeof(used), par->hook_mask),
+		       textify_hooks(allow, sizeof(allow), par->target->hooks));
 		return -EINVAL;
 	}
 	if (par->target->proto && (par->target->proto != proto || inv_proto)) {
-- 
1.6.2.2

Re: [PATCH 11/11] netfilter: xtables: print hook name instead of mask

[Amos Jeffries]

Jan Engelhardt wrote:
> Users cannot make anything of these numbers. Let's just tell them
> directly.

Yay!
Thank you very, very much.

> 
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
> ---
>  net/netfilter/x_tables.c |   42 ++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index 150e5cf..46dba5f 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -329,6 +329,32 @@ int xt_find_revision(u8 af, const char *name, u8 revision, int target,
>  }
>  EXPORT_SYMBOL_GPL(xt_find_revision);
>  
> +static char *textify_hooks(char *buf, size_t size, unsigned int mask)
> +{
> +	static const char *const names[] = {
> +		"PREROUTING", "INPUT", "FORWARD",
> +		"OUTPUT", "POSTROUTING", "BROUTING",
> +	};
> +	unsigned int i;
> +	char *p = buf;
> +	bool np = false;
> +	int res;
> +
> +	*p = '\0';
> +	for (i = 0; i < ARRAY_SIZE(names); ++i) {
> +		if (!(mask & (1 << i)))
> +			continue;
> +		res = snprintf(p, size, "%s%s", np ? "/" : "", names[i]);
> +		if (res > 0) {
> +			size -= res;
> +			p += res;
> +		}
> +		np = true;
> +	}
> +
> +	return buf;
> +}
> +
>  int xt_check_match(struct xt_mtchk_param *par,
>  		   unsigned int size, u_int8_t proto, bool inv_proto)
>  {
> @@ -351,9 +377,13 @@ int xt_check_match(struct xt_mtchk_param *par,
>  		return -EINVAL;
>  	}
>  	if (par->match->hooks && (par->hook_mask & ~par->match->hooks) != 0) {
> -		printk("%s_tables: %s match: bad hook_mask %#x/%#x\n",
> +		char used[64], allow[64];
> +
> +		printk("%s_tables: %s match: used from hooks %s, but only "
> +		       "valid from %s\n",
>  		       xt_prefix[par->family], par->match->name,
> -		       par->hook_mask, par->match->hooks);
> +		       textify_hooks(used, sizeof(used), par->hook_mask),
> +		       textify_hooks(allow, sizeof(allow), par->match->hooks));
>  		return -EINVAL;
>  	}
>  	if (par->match->proto && (par->match->proto != proto || inv_proto)) {
> @@ -497,9 +527,13 @@ int xt_check_target(struct xt_tgchk_param *par,
>  		return -EINVAL;
>  	}
>  	if (par->target->hooks && (par->hook_mask & ~par->target->hooks) != 0) {
> -		printk("%s_tables: %s target: bad hook_mask %#x/%#x\n",
> +		char used[64], allow[64];
> +
> +		printk("%s_tables: %s target: used from hooks %s, but only "
> +		       "usable from %s\n",
>  		       xt_prefix[par->family], par->target->name,
> -		       par->hook_mask, par->target->hooks);
> +		       textify_hooks(used, sizeof(used), par->hook_mask),
> +		       textify_hooks(allow, sizeof(allow), par->target->hooks));
>  		return -EINVAL;
>  	}
>  	if (par->target->proto && (par->target->proto != proto || inv_proto)) {

Re: kernel/iptables patches

[Jan Engelhardt]

On Friday 2009-05-08 11:26, Jan Engelhardt wrote:
>
>Hi,
>
>
>please apply the following tidbits for the kernel-side iptables code.
>Alternatively, the set is provided at
>	git://dev.medozas.de/linux master
>with base dave/master:v2.6.30-rc1-1016-g928f308.

The -next is already open, so has this been missed?


thanks,
Jan


>Jan Engelhardt (10):
>      netfilter: xtables: use NFPROTO_ for xt_proto_init callsites
>      netfilter: queue: use NFPROTO_ for queue callsites
>      netfilter: xtables: use NFPROTO_ in standard targets
>      netfilter: xtables: remove redundant casts
>      netfilter: xtables: fix const inconsistency
>      netfilter: xtables: consolidate open-coded logic
>      netfilter: xtables: reduce indent level by one
>      netfilter: xtables: remove some goto
>      netfilter: xtables: remove another level of indent
>      netfilter: xtables: consolidate comefrom debug cast access
>
>These transforms are part of an ongoing investigation on how to
>consolidate the *tables, and making it a bit more readable seemed the
>first step. The patches seem useful independently nevertheless.
>--
>To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: kernel/iptables patches

[Patrick McHardy]

Jan Engelhardt wrote:
> On Friday 2009-05-08 11:26, Jan Engelhardt wrote:
>> please apply the following tidbits for the kernel-side iptables code.
>> Alternatively, the set is provided at
>> 	git://dev.medozas.de/linux master
>> with base dave/master:v2.6.30-rc1-1016-g928f308.
> 
> The -next is already open, so has this been missed?

Looks good, nice cleanups. Pulled and pushed back out again.