[PATCH 2/2] Cleanup returnvalues of protocolhandlers

[PATCH 2/2] Cleanup returnvalues of protocolhandlers

[Martin Josefsson]

[-- Attachment #1: Type: text/plain, Size: 14789 bytes --]

Hi Patrick

Here's the second of two patches based on Pablo Neiras patch.

This adds new defines to be returned from conntrack protocol modules in
order to make it clear what they want. Most importantly, they get rid of
the confusing mixing of positive and negative returnvalues.

This patch is incremental to the first one ([PATCH 1/2] Cleanup ctstat)

Please submit for 2.6

Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>

diff -urNp -x '*.orig' linux-2.6.9-rc1-bk16.stats/include/linux/netfilter_ipv4/ip_conntrack.h linux-2.6.9-rc1-bk16/include/linux/netfilter_ipv4/ip_conntrack.h
--- linux-2.6.9-rc1-bk16.stats/include/linux/netfilter_ipv4/ip_conntrack.h	2004-09-10 11:33:31.000000000 +0200
+++ linux-2.6.9-rc1-bk16/include/linux/netfilter_ipv4/ip_conntrack.h	2004-09-11 14:00:13.000000000 +0200
@@ -10,6 +10,14 @@
 #include <linux/compiler.h>
 #include <asm/atomic.h>
 
+/* Protocol specific functions (packet and error) return one of the
+ * return values defined below. Return CONNTRACK_CONT to perform no action
+ * on a packet. */
+#define CONNTRACK_CONT -1
+#define CONNTRACK_INVALID NF_ACCEPT
+#define CONNTRACK_DROP NF_DROP
+#define CONNTRACK_REPEAT NF_REPEAT
+
 enum ip_conntrack_info
 {
 	/* Part of an established connection (either direction). */
diff -urNp -x '*.orig' linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_core.c linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_core.c
--- linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-11 13:28:41.000000000 +0200
+++ linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-11 13:43:12.000000000 +0200
@@ -786,14 +786,14 @@ unsigned int ip_conntrack_in(unsigned in
 
 	proto = ip_ct_find_proto((*pskb)->nh.iph->protocol);
 
-	/* It may be an special packet, error, unclean...
-	 * inverse of the return code tells to the netfilter
-	 * core what to do with the packet. */
-	if (proto->error != NULL 
-	    && (ret = proto->error(*pskb, &ctinfo, hooknum)) <= 0) {
-		CONNTRACK_STAT_INC(error);
-		CONNTRACK_STAT_INC(invalid);
-		return -ret;
+	/* It may be an special packet, error, unclean... */
+	if (proto->error != NULL) {
+		ret = proto->error(*pskb, &ctinfo, hooknum);
+		if (ret != CONNTRACK_CONT) {
+			CONNTRACK_STAT_INC(error);
+			CONNTRACK_STAT_INC(invalid);
+			return ret;
+		}
 	}
 
 	if (!(ct = resolve_normal_ct(*pskb, proto,&set_reply,hooknum,&ctinfo))) {
@@ -811,16 +811,14 @@ unsigned int ip_conntrack_in(unsigned in
 	IP_NF_ASSERT((*pskb)->nfct);
 
 	ret = proto->packet(ct, *pskb, ctinfo);
-	if (ret < 0) {
-		/* Invalid: inverse of the return code tells
-		 * the netfilter core what to do*/
+	if (ret != CONNTRACK_CONT) {
 		nf_conntrack_put((*pskb)->nfct);
 		(*pskb)->nfct = NULL;
 		CONNTRACK_STAT_INC(invalid);
-		return -ret;
+		return ret;
 	}
 
-	if (ret != NF_DROP && ct->helper) {
+	if (ct->helper != NULL) {
 		ret = ct->helper->help(*pskb, ct, ctinfo);
 		if (ret == -1) {
 			/* Invalid */
diff -urNp -x '*.orig' linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_generic.c linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_generic.c
--- linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_generic.c	2004-09-10 11:33:19.000000000 +0200
+++ linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_generic.c	2004-09-11 13:37:13.000000000 +0200
@@ -53,7 +53,7 @@ static int packet(struct ip_conntrack *c
 		  enum ip_conntrack_info ctinfo)
 {
 	ip_ct_refresh_acct(conntrack, ctinfo, skb, ip_ct_generic_timeout);
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */
diff -urNp -x '*.orig' linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_icmp.c linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
--- linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_icmp.c	2004-09-10 11:33:32.000000000 +0200
+++ linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_icmp.c	2004-09-11 13:37:13.000000000 +0200
@@ -104,7 +104,7 @@ static int icmp_packet(struct ip_conntra
 		ip_ct_refresh_acct(ct, ctinfo, skb, ip_ct_icmp_timeout);
 	}
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */
@@ -195,7 +195,7 @@ icmp_error_message(struct sk_buff *skb,
 
 	/* Update skb to refer to this connection */
 	skb->nfct = &h->ctrack->infos[*ctinfo];
-	return -NF_ACCEPT;
+	return CONNTRACK_INVALID;
 }
 
 /* Small and modified version of icmp_rcv */
@@ -210,7 +210,7 @@ icmp_error(struct sk_buff *skb, enum ip_
 		if (LOG_INVALID(IPPROTO_ICMP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL,
 				      "ip_ct_icmp: short packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 
 	/* See ip_conntrack_proto_tcp.c */
@@ -224,13 +224,13 @@ icmp_error(struct sk_buff *skb, enum ip_
 		if (LOG_INVALID(IPPROTO_ICMP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				      "ip_ct_icmp: bad HW ICMP checksum ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	case CHECKSUM_NONE:
 		if ((u16)csum_fold(skb_checksum(skb, 0, skb->len, 0))) {
 			if (LOG_INVALID(IPPROTO_ICMP))
 				nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 					      "ip_ct_icmp: bad ICMP checksum ");
-			return -NF_ACCEPT;
+			return CONNTRACK_INVALID;
 		}
 	default:
 		break;
@@ -247,7 +247,7 @@ checksum_skipped:
 		if (LOG_INVALID(IPPROTO_ICMP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL,
 				      "ip_ct_icmp: invalid ICMP type ");
-		return -NF_ACCEPT;
+		return NF_ACCEPT;
 	}
 
 	/* Need to track icmp error message? */
@@ -256,7 +256,7 @@ checksum_skipped:
 	    && icmph.type != ICMP_TIME_EXCEEDED
 	    && icmph.type != ICMP_PARAMETERPROB
 	    && icmph.type != ICMP_REDIRECT)
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 
 	return icmp_error_message(skb, ctinfo, hooknum);
 }
diff -urNp -x '*.orig' linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_tcp.c linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
--- linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2004-09-10 11:33:32.000000000 +0200
+++ linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2004-09-11 13:37:13.000000000 +0200
@@ -784,7 +784,7 @@ static int tcp_error(struct sk_buff *skb
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				"ip_ct_tcp: short packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
   	}
   
 	/* Not whole TCP header or malformed packet */
@@ -792,7 +792,7 @@ static int tcp_error(struct sk_buff *skb
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				"ip_ct_tcp: truncated/malformed packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
   
 	/* Checksum invalid? Ignore.
@@ -808,7 +808,7 @@ static int tcp_error(struct sk_buff *skb
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: bad TCP checksum ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 
 	/* Check TCP flags. */
@@ -817,10 +817,10 @@ static int tcp_error(struct sk_buff *skb
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: invalid TCP flag combination ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Returns verdict for packet, or -1 for invalid. */
@@ -867,7 +867,7 @@ static int tcp_packet(struct ip_conntrac
 		    	if (del_timer(&conntrack->timeout))
 		    		conntrack->timeout.function((unsigned long)
 		    					    conntrack);
-		    	return -NF_DROP;
+		    	return CONNTRACK_DROP;
 		}
 		conntrack->proto.tcp.last_index = index;
 		conntrack->proto.tcp.last_dir = dir;
@@ -877,7 +877,7 @@ static int tcp_packet(struct ip_conntrac
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: invalid SYN (ignored) ");
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 	case TCP_CONNTRACK_MAX:
 		/* Invalid packet */
 		DEBUGP("ip_ct_tcp: Invalid dir=%i index=%u ostate=%u\n",
@@ -887,7 +887,7 @@ static int tcp_packet(struct ip_conntrac
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: invalid state ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	case TCP_CONNTRACK_SYN_SENT:
 		if (old_state >= TCP_CONNTRACK_TIME_WAIT) {	
 		    	/* Attempt to reopen a closed connection.
@@ -896,7 +896,7 @@ static int tcp_packet(struct ip_conntrac
 		    	if (del_timer(&conntrack->timeout))
 		    		conntrack->timeout.function((unsigned long)
 		    					    conntrack);
-		    	return -NF_REPEAT;
+		    	return CONNTRACK_REPEAT;
 		}
 		break;
 	case TCP_CONNTRACK_CLOSE:
@@ -911,7 +911,7 @@ static int tcp_packet(struct ip_conntrac
 			if (LOG_INVALID(IPPROTO_TCP))
 				nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 					  "ip_ct_tcp: invalid RST (ignored) ");
-			return NF_ACCEPT;
+			return CONNTRACK_CONT;
 		}
 		/* Just fall trough */
 	default:
@@ -922,7 +922,7 @@ static int tcp_packet(struct ip_conntrac
 	if (!tcp_in_window(&conntrack->proto.tcp, dir, &index, 
 			   skb, iph, th)) {
 		WRITE_UNLOCK(&tcp_lock);
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	/* From now on we have got in-window packets */
 	
@@ -953,7 +953,7 @@ static int tcp_packet(struct ip_conntrac
 			if (del_timer(&conntrack->timeout))
 				conntrack->timeout.function((unsigned long)
 							    conntrack);
-			return NF_ACCEPT;
+			return CONNTRACK_CONT;
 		}
 	} else if (!test_bit(IPS_ASSURED_BIT, &conntrack->status)
 		   && (old_state == TCP_CONNTRACK_SYN_RECV
@@ -966,7 +966,7 @@ static int tcp_packet(struct ip_conntrac
 	}
 	ip_ct_refresh_acct(conntrack, ctinfo, skb, timeout);
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
  
   /* Called when a new connection for this protocol found. */
diff -urNp -x '*.orig' linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_udp.c linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_udp.c
--- linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_udp.c	2004-09-10 11:33:32.000000000 +0200
+++ linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_udp.c	2004-09-11 13:37:13.000000000 +0200
@@ -76,7 +76,7 @@ static int udp_packet(struct ip_conntrac
 	} else
 		ip_ct_refresh_acct(conntrack, ctinfo, skb, ip_ct_udp_timeout);
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */
@@ -97,7 +97,7 @@ static int udp_error(struct sk_buff *skb
 		if (LOG_INVALID(IPPROTO_UDP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_udp: short packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	
 	/* Truncated/malformed packets */
@@ -105,12 +105,12 @@ static int udp_error(struct sk_buff *skb
 		if (LOG_INVALID(IPPROTO_UDP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_udp: truncated/malformed packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	
 	/* Packet with no checksum */
 	if (!hdr.check)
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 
 	/* Checksum invalid? Ignore.
 	 * We skip checking packets on the outgoing path
@@ -124,10 +124,10 @@ static int udp_error(struct sk_buff *skb
 		if (LOG_INVALID(IPPROTO_UDP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_udp: bad UDP checksum ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 struct ip_conntrack_protocol ip_conntrack_protocol_udp =
--- linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-09-10 11:33:32.000000000 +0200
+++ linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-09-11 13:49:31.000000000 +0200
@@ -321,10 +321,10 @@ static int sctp_packet(struct ip_conntra
 	DEBUGP("\n");
 
 	if (skb_copy_bits(skb, skb->nh.iph->ihl * 4, &sctph, sizeof(sctph)) != 0)
-		return -1;
+		return CONNTRACK_INVALID;
 
 	if (do_basic_checks(conntrack, skb, map) != 0)
-		return -1;
+		return CONNTRACK_INVALID;
 
 	/* Check the verification tag (Sec 8.5) */
 	if (!test_bit(SCTP_CID_INIT, (void *)map)
@@ -334,7 +334,7 @@ static int sctp_packet(struct ip_conntra
 		&& !test_bit(SCTP_CID_SHUTDOWN_ACK, (void *)map)
 		&& (sctph.vtag != conntrack->proto.sctp.vtag[CTINFO2DIR(ctinfo)])) {
 		DEBUGP("Verification tag check failed\n");
-		return -1;
+		return CONNTRACK_INVALID;
 	}
 
 	oldsctpstate = newconntrack = SCTP_CONNTRACK_MAX;
@@ -346,7 +346,7 @@ static int sctp_packet(struct ip_conntra
 			/* Sec 8.5.1 (A) */
 			if (sctph.vtag != 0) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		} else if (sch.type == SCTP_CID_ABORT) {
 			/* Sec 8.5.1 (B) */
@@ -354,7 +354,7 @@ static int sctp_packet(struct ip_conntra
 				&& !(sctph.vtag == conntrack->proto.sctp.vtag
 							[1 - CTINFO2DIR(ctinfo)])) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		} else if (sch.type == SCTP_CID_SHUTDOWN_COMPLETE) {
 			/* Sec 8.5.1 (C) */
@@ -363,13 +363,13 @@ static int sctp_packet(struct ip_conntra
 							[1 - CTINFO2DIR(ctinfo)] 
 					&& (sch.flags & 1))) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		} else if (sch.type == SCTP_CID_COOKIE_ECHO) {
 			/* Sec 8.5.1 (D) */
 			if (!(sctph.vtag == conntrack->proto.sctp.vtag[CTINFO2DIR(ctinfo)])) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		}
 
@@ -381,7 +381,7 @@ static int sctp_packet(struct ip_conntra
 			DEBUGP("ip_conntrack_sctp: Invalid dir=%i ctype=%u conntrack=%u\n",
 			       CTINFO2DIR(ctinfo), sch.type, oldsctpstate);
 			WRITE_UNLOCK(&sctp_lock);
-			return -1;
+			return CONNTRACK_INVALID;
 		}
 
 		/* If it is an INIT or an INIT ACK note down the vtag */
@@ -392,7 +392,7 @@ static int sctp_packet(struct ip_conntra
 			if (skb_copy_bits(skb, offset + sizeof (sctp_chunkhdr_t),
 				&inithdr, sizeof(inithdr)) != 0) {
 					WRITE_UNLOCK(&sctp_lock);
-					return -1;
+					return CONNTRACK_INVALID;
 			}
 			DEBUGP("Setting vtag %x for dir %d\n", 
 					inithdr.init_tag, CTINFO2DIR(ctinfo));
@@ -412,7 +412,7 @@ static int sctp_packet(struct ip_conntra
 		set_bit(IPS_ASSURED_BIT, &conntrack->status);
 	}
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */

-- 
/Martin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/2] Cleanup returnvalues of protocolhandlers

[Patrick McHardy]

Martin Josefsson wrote:

>Hi Patrick
>
>Here's the second of two patches based on Pablo Neiras patch.
>
>This adds new defines to be returned from conntrack protocol modules in
>order to make it clear what they want. Most importantly, they get rid of
>the confusing mixing of positive and negative returnvalues.
>  
>
A couple of conversion where missing in ip_conntrack_proto_icmp.c,
I've added them to the patch. BTW: shouldn't we return CONNTRACK_INVALID
for all invalid icmp packets ? icmp_error returns CONNTRACK_INVALID for
these packets, icmp_error_message (after simple 1:1 conversion) returns
CONNTRACK_CONT.

Regards
Patrick

Re: [PATCH 2/2] Cleanup returnvalues of protocolhandlers

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

Patrick McHardy wrote:

> A couple of conversion where missing in ip_conntrack_proto_icmp.c,
> I've added them to the patch. BTW: shouldn't we return CONNTRACK_INVALID
> for all invalid icmp packets ? icmp_error returns CONNTRACK_INVALID for
> these packets, icmp_error_message (after simple 1:1 conversion) returns
> CONNTRACK_CONT.

Oops, forgot to attach the new patch.

>
> Regards
> Patrick
>
>


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 17666 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/09/19 22:27:45+02:00 gandalf@wlug.westbo.se 
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_proto_udp.c
#   2004/09/19 22:27:23+02:00 gandalf@wlug.westbo.se +6 -6
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_proto_tcp.c
#   2004/09/19 22:27:23+02:00 gandalf@wlug.westbo.se +13 -13
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_proto_sctp.c
#   2004/09/19 22:27:23+02:00 gandalf@wlug.westbo.se +10 -10
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_proto_icmp.c
#   2004/09/19 22:27:23+02:00 gandalf@wlug.westbo.se +12 -12
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_proto_generic.c
#   2004/09/19 22:27:23+02:00 gandalf@wlug.westbo.se +1 -1
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_core.c
#   2004/09/19 22:27:23+02:00 gandalf@wlug.westbo.se +11 -13
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# include/linux/netfilter_ipv4/ip_conntrack.h
#   2004/09/19 22:27:23+02:00 gandalf@wlug.westbo.se +8 -0
#   [NETFILTER]: Cleanup return-values of protocol handlers
#   
#   This adds new defines to be returned from conntrack protocol modules in
#   order to make it clear what they want. Most importantly, they get rid of
#   the confusing mixing of positive and negative returnvalues.
#   
#   Based on patch by Pablo Neira.
#   
#   Signed-off-by: Martin Josefsson <gandalf@wlug.westbo.se>
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
diff -Nru a/include/linux/netfilter_ipv4/ip_conntrack.h b/include/linux/netfilter_ipv4/ip_conntrack.h
--- a/include/linux/netfilter_ipv4/ip_conntrack.h	2004-09-19 22:28:48 +02:00
+++ b/include/linux/netfilter_ipv4/ip_conntrack.h	2004-09-19 22:28:48 +02:00
@@ -10,6 +10,14 @@
 #include <linux/compiler.h>
 #include <asm/atomic.h>
 
+/* Protocol specific functions (packet and error) return one of the
+ * return values defined below. Return CONNTRACK_CONT to perform no action
+ * on a packet. */
+#define CONNTRACK_CONT -1
+#define CONNTRACK_INVALID NF_ACCEPT
+#define CONNTRACK_DROP NF_DROP
+#define CONNTRACK_REPEAT NF_REPEAT
+
 enum ip_conntrack_info
 {
 	/* Part of an established connection (either direction). */
diff -Nru a/net/ipv4/netfilter/ip_conntrack_core.c b/net/ipv4/netfilter/ip_conntrack_core.c
--- a/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-19 22:28:48 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-19 22:28:48 +02:00
@@ -722,14 +722,14 @@
 
 	proto = ip_ct_find_proto((*pskb)->nh.iph->protocol);
 
-	/* It may be an special packet, error, unclean...
-	 * inverse of the return code tells to the netfilter
-	 * core what to do with the packet. */
-	if (proto->error != NULL 
-	    && (ret = proto->error(*pskb, &ctinfo, hooknum)) <= 0) {
-		CONNTRACK_STAT_INC(error);
-		CONNTRACK_STAT_INC(invalid);
-		return -ret;
+	/* It may be an special packet, error, unclean... */
+	if (proto->error != NULL) {
+		ret = proto->error(*pskb, &ctinfo, hooknum);
+		if (ret != CONNTRACK_CONT) {
+			CONNTRACK_STAT_INC(error);
+			CONNTRACK_STAT_INC(invalid);
+			return ret;
+		}
 	}
 
 	if (!(ct = resolve_normal_ct(*pskb, proto,&set_reply,hooknum,&ctinfo))) {
@@ -747,16 +747,14 @@
 	IP_NF_ASSERT((*pskb)->nfct);
 
 	ret = proto->packet(ct, *pskb, ctinfo);
-	if (ret < 0) {
-		/* Invalid: inverse of the return code tells
-		 * the netfilter core what to do*/
+	if (ret != CONNTRACK_CONT) {
 		nf_conntrack_put((*pskb)->nfct);
 		(*pskb)->nfct = NULL;
 		CONNTRACK_STAT_INC(invalid);
-		return -ret;
+		return ret;
 	}
 
-	if (ret != NF_DROP && ct->helper) {
+	if (ct->helper != NULL) {
 		ret = ct->helper->help(*pskb, ct, ctinfo);
 		if (ret == -1) {
 			/* Invalid */
diff -Nru a/net/ipv4/netfilter/ip_conntrack_proto_generic.c b/net/ipv4/netfilter/ip_conntrack_proto_generic.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_generic.c	2004-09-19 22:28:48 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_proto_generic.c	2004-09-19 22:28:48 +02:00
@@ -53,7 +53,7 @@
 		  enum ip_conntrack_info ctinfo)
 {
 	ip_ct_refresh_acct(conntrack, ctinfo, skb, ip_ct_generic_timeout);
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */
diff -Nru a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c	2004-09-19 22:28:48 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c	2004-09-19 22:28:48 +02:00
@@ -105,7 +105,7 @@
 		ip_ct_refresh_acct(ct, ctinfo, skb, ip_ct_icmp_timeout);
 	}
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */
@@ -148,13 +148,13 @@
 
 	/* Not enough header? */
 	if (skb_copy_bits(skb, skb->nh.iph->ihl*4, &inside, sizeof(inside))!=0)
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 
 	/* Ignore ICMP's containing fragments (shouldn't happen) */
 	if (inside.ip.frag_off & htons(IP_OFFSET)) {
 		DEBUGP("icmp_error_track: fragment of proto %u\n",
 		       inside.ip.protocol);
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 	}
 
 	innerproto = ip_ct_find_proto(inside.ip.protocol);
@@ -162,14 +162,14 @@
 	/* Are they talking about one of our connections? */
 	if (!ip_ct_get_tuple(&inside.ip, skb, dataoff, &origtuple, innerproto)) {
 		DEBUGP("icmp_error: ! get_tuple p=%u", inside.ip.protocol);
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 	}
 
 	/* Ordinarily, we'd expect the inverted tupleproto, but it's
 	   been preserved inside the ICMP. */
 	if (!ip_ct_invert_tuple(&innertuple, &origtuple, innerproto)) {
 		DEBUGP("icmp_error_track: Can't invert tuple\n");
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 	}
 
 	*ctinfo = IP_CT_RELATED;
@@ -184,7 +184,7 @@
 
 		if (!h) {
 			DEBUGP("icmp_error_track: no match\n");
-			return NF_ACCEPT;
+			return CONNTRACK_CONT;
 		}
 		/* Reverse direction from that found */
 		if (DIRECTION(h) != IP_CT_DIR_REPLY)
@@ -197,7 +197,7 @@
 	/* Update skb to refer to this connection */
 	skb->nfct = &h->ctrack->ct_general;
 	skb->nfctinfo = *ctinfo;
-	return -NF_ACCEPT;
+	return CONNTRACK_INVALID;
 }
 
 /* Small and modified version of icmp_rcv */
@@ -212,7 +212,7 @@
 		if (LOG_INVALID(IPPROTO_ICMP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL,
 				      "ip_ct_icmp: short packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 
 	/* See ip_conntrack_proto_tcp.c */
@@ -226,13 +226,13 @@
 		if (LOG_INVALID(IPPROTO_ICMP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				      "ip_ct_icmp: bad HW ICMP checksum ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	case CHECKSUM_NONE:
 		if ((u16)csum_fold(skb_checksum(skb, 0, skb->len, 0))) {
 			if (LOG_INVALID(IPPROTO_ICMP))
 				nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 					      "ip_ct_icmp: bad ICMP checksum ");
-			return -NF_ACCEPT;
+			return CONNTRACK_INVALID;
 		}
 	default:
 		break;
@@ -249,7 +249,7 @@
 		if (LOG_INVALID(IPPROTO_ICMP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL,
 				      "ip_ct_icmp: invalid ICMP type ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 
 	/* Need to track icmp error message? */
@@ -258,7 +258,7 @@
 	    && icmph.type != ICMP_TIME_EXCEEDED
 	    && icmph.type != ICMP_PARAMETERPROB
 	    && icmph.type != ICMP_REDIRECT)
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 
 	return icmp_error_message(skb, ctinfo, hooknum);
 }
diff -Nru a/net/ipv4/netfilter/ip_conntrack_proto_sctp.c b/net/ipv4/netfilter/ip_conntrack_proto_sctp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-09-19 22:28:48 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-09-19 22:28:48 +02:00
@@ -322,10 +322,10 @@
 	DEBUGP("\n");
 
 	if (skb_copy_bits(skb, skb->nh.iph->ihl * 4, &sctph, sizeof(sctph)) != 0)
-		return -1;
+		return CONNTRACK_INVALID;
 
 	if (do_basic_checks(conntrack, skb, map) != 0)
-		return -1;
+		return CONNTRACK_INVALID;
 
 	/* Check the verification tag (Sec 8.5) */
 	if (!test_bit(SCTP_CID_INIT, (void *)map)
@@ -335,7 +335,7 @@
 		&& !test_bit(SCTP_CID_SHUTDOWN_ACK, (void *)map)
 		&& (sctph.vtag != conntrack->proto.sctp.vtag[CTINFO2DIR(ctinfo)])) {
 		DEBUGP("Verification tag check failed\n");
-		return -1;
+		return CONNTRACK_INVALID;
 	}
 
 	oldsctpstate = newconntrack = SCTP_CONNTRACK_MAX;
@@ -347,7 +347,7 @@
 			/* Sec 8.5.1 (A) */
 			if (sctph.vtag != 0) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		} else if (sch.type == SCTP_CID_ABORT) {
 			/* Sec 8.5.1 (B) */
@@ -355,7 +355,7 @@
 				&& !(sctph.vtag == conntrack->proto.sctp.vtag
 							[1 - CTINFO2DIR(ctinfo)])) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		} else if (sch.type == SCTP_CID_SHUTDOWN_COMPLETE) {
 			/* Sec 8.5.1 (C) */
@@ -364,13 +364,13 @@
 							[1 - CTINFO2DIR(ctinfo)] 
 					&& (sch.flags & 1))) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		} else if (sch.type == SCTP_CID_COOKIE_ECHO) {
 			/* Sec 8.5.1 (D) */
 			if (!(sctph.vtag == conntrack->proto.sctp.vtag[CTINFO2DIR(ctinfo)])) {
 				WRITE_UNLOCK(&sctp_lock);
-				return -1;
+				return CONNTRACK_INVALID;
 			}
 		}
 
@@ -382,7 +382,7 @@
 			DEBUGP("ip_conntrack_sctp: Invalid dir=%i ctype=%u conntrack=%u\n",
 			       CTINFO2DIR(ctinfo), sch.type, oldsctpstate);
 			WRITE_UNLOCK(&sctp_lock);
-			return -1;
+			return CONNTRACK_INVALID;
 		}
 
 		/* If it is an INIT or an INIT ACK note down the vtag */
@@ -393,7 +393,7 @@
 			if (skb_copy_bits(skb, offset + sizeof (sctp_chunkhdr_t),
 				&inithdr, sizeof(inithdr)) != 0) {
 					WRITE_UNLOCK(&sctp_lock);
-					return -1;
+					return CONNTRACK_INVALID;
 			}
 			DEBUGP("Setting vtag %x for dir %d\n", 
 					inithdr.init_tag, CTINFO2DIR(ctinfo));
@@ -413,7 +413,7 @@
 		set_bit(IPS_ASSURED_BIT, &conntrack->status);
 	}
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */
diff -Nru a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2004-09-19 22:28:48 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c	2004-09-19 22:28:48 +02:00
@@ -784,7 +784,7 @@
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				"ip_ct_tcp: short packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
   	}
   
 	/* Not whole TCP header or malformed packet */
@@ -792,7 +792,7 @@
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				"ip_ct_tcp: truncated/malformed packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
   
 	/* Checksum invalid? Ignore.
@@ -808,7 +808,7 @@
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: bad TCP checksum ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 
 	/* Check TCP flags. */
@@ -817,10 +817,10 @@
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: invalid TCP flag combination ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Returns verdict for packet, or -1 for invalid. */
@@ -867,7 +867,7 @@
 		    	if (del_timer(&conntrack->timeout))
 		    		conntrack->timeout.function((unsigned long)
 		    					    conntrack);
-		    	return -NF_DROP;
+		    	return CONNTRACK_DROP;
 		}
 		conntrack->proto.tcp.last_index = index;
 		conntrack->proto.tcp.last_dir = dir;
@@ -877,7 +877,7 @@
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: invalid SYN (ignored) ");
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 	case TCP_CONNTRACK_MAX:
 		/* Invalid packet */
 		DEBUGP("ip_ct_tcp: Invalid dir=%i index=%u ostate=%u\n",
@@ -887,7 +887,7 @@
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_tcp: invalid state ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	case TCP_CONNTRACK_SYN_SENT:
 		if (old_state >= TCP_CONNTRACK_TIME_WAIT) {	
 		    	/* Attempt to reopen a closed connection.
@@ -896,7 +896,7 @@
 		    	if (del_timer(&conntrack->timeout))
 		    		conntrack->timeout.function((unsigned long)
 		    					    conntrack);
-		    	return -NF_REPEAT;
+		    	return CONNTRACK_REPEAT;
 		}
 		break;
 	case TCP_CONNTRACK_CLOSE:
@@ -911,7 +911,7 @@
 			if (LOG_INVALID(IPPROTO_TCP))
 				nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 					  "ip_ct_tcp: invalid RST (ignored) ");
-			return NF_ACCEPT;
+			return CONNTRACK_CONT;
 		}
 		/* Just fall trough */
 	default:
@@ -922,7 +922,7 @@
 	if (!tcp_in_window(&conntrack->proto.tcp, dir, &index, 
 			   skb, iph, th)) {
 		WRITE_UNLOCK(&tcp_lock);
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	/* From now on we have got in-window packets */
 	
@@ -953,7 +953,7 @@
 			if (del_timer(&conntrack->timeout))
 				conntrack->timeout.function((unsigned long)
 							    conntrack);
-			return NF_ACCEPT;
+			return CONNTRACK_CONT;
 		}
 	} else if (!test_bit(IPS_ASSURED_BIT, &conntrack->status)
 		   && (old_state == TCP_CONNTRACK_SYN_RECV
@@ -966,7 +966,7 @@
 	}
 	ip_ct_refresh_acct(conntrack, ctinfo, skb, timeout);
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
  
   /* Called when a new connection for this protocol found. */
diff -Nru a/net/ipv4/netfilter/ip_conntrack_proto_udp.c b/net/ipv4/netfilter/ip_conntrack_proto_udp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_udp.c	2004-09-19 22:28:48 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_proto_udp.c	2004-09-19 22:28:48 +02:00
@@ -77,7 +77,7 @@
 	} else
 		ip_ct_refresh_acct(conntrack, ctinfo, skb, ip_ct_udp_timeout);
 
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 /* Called when a new connection for this protocol found. */
@@ -98,7 +98,7 @@
 		if (LOG_INVALID(IPPROTO_UDP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_udp: short packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	
 	/* Truncated/malformed packets */
@@ -106,12 +106,12 @@
 		if (LOG_INVALID(IPPROTO_UDP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_udp: truncated/malformed packet ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	
 	/* Packet with no checksum */
 	if (!hdr.check)
-		return NF_ACCEPT;
+		return CONNTRACK_CONT;
 
 	/* Checksum invalid? Ignore.
 	 * We skip checking packets on the outgoing path
@@ -125,10 +125,10 @@
 		if (LOG_INVALID(IPPROTO_UDP))
 			nf_log_packet(PF_INET, 0, skb, NULL, NULL, 
 				  "ip_ct_udp: bad UDP checksum ");
-		return -NF_ACCEPT;
+		return CONNTRACK_INVALID;
 	}
 	
-	return NF_ACCEPT;
+	return CONNTRACK_CONT;
 }
 
 struct ip_conntrack_protocol ip_conntrack_protocol_udp =

Re: [PATCH 2/2] Cleanup returnvalues of protocolhandlers

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

Martin Josefsson wrote:

>Hi Patrick
>
>Here's the second of two patches based on Pablo Neiras patch.
>
>This adds new defines to be returned from conntrack protocol modules in
>order to make it clear what they want. Most importantly, they get rid of
>the confusing mixing of positive and negative returnvalues.
>

We also need this patch on top of the last one to avoid ip_conntrack_in
from returning CONNTRACK_CONT (-1) to nf_hook_slow.

Regards
Patrick

>diff -urNp -x '*.orig' linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_core.c linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_core.c
>--- linux-2.6.9-rc1-bk16.stats/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-11 13:28:41.000000000 +0200
>+++ linux-2.6.9-rc1-bk16/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-11 13:43:12.000000000 +0200
>@@ -786,14 +786,14 @@ unsigned int ip_conntrack_in(unsigned in
> 
> 	proto = ip_ct_find_proto((*pskb)->nh.iph->protocol);
> 
>-	/* It may be an special packet, error, unclean...
>-	 * inverse of the return code tells to the netfilter
>-	 * core what to do with the packet. */
>-	if (proto->error != NULL 
>-	    && (ret = proto->error(*pskb, &ctinfo, hooknum)) <= 0) {
>-		CONNTRACK_STAT_INC(error);
>-		CONNTRACK_STAT_INC(invalid);
>-		return -ret;
>+	/* It may be an special packet, error, unclean... */
>+	if (proto->error != NULL) {
>+		ret = proto->error(*pskb, &ctinfo, hooknum);
>+		if (ret != CONNTRACK_CONT) {
>+			CONNTRACK_STAT_INC(error);
>+			CONNTRACK_STAT_INC(invalid);
>+			return ret;
>+		}
> 	}
> 
> 	if (!(ct = resolve_normal_ct(*pskb, proto,&set_reply,hooknum,&ctinfo))) {
>@@ -811,16 +811,14 @@ unsigned int ip_conntrack_in(unsigned in
> 	IP_NF_ASSERT((*pskb)->nfct);
> 
> 	ret = proto->packet(ct, *pskb, ctinfo);
>-	if (ret < 0) {
>-		/* Invalid: inverse of the return code tells
>-		 * the netfilter core what to do*/
>+	if (ret != CONNTRACK_CONT) {
> 		nf_conntrack_put((*pskb)->nfct);
> 		(*pskb)->nfct = NULL;
> 		CONNTRACK_STAT_INC(invalid);
>-		return -ret;
>+		return ret;
> 	}
> 
>-	if (ret != NF_DROP && ct->helper) {
>+	if (ct->helper != NULL) {
> 		ret = ct->helper->help(*pskb, ct, ctinfo);
> 		if (ret == -1) {
> 			/* Invalid */
>
>  
>


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 880 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/09/20 10:05:19+02:00 kaber@coreworks.de 
#   [NETFILTER]: Fix invalid return values from ip_conntrack_in
#   
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_core.c
#   2004/09/20 10:04:56+02:00 kaber@coreworks.de +1 -1
#   [NETFILTER]: Fix invalid return values from ip_conntrack_in
#   
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
diff -Nru a/net/ipv4/netfilter/ip_conntrack_core.c b/net/ipv4/netfilter/ip_conntrack_core.c
--- a/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-20 10:08:05 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_core.c	2004-09-20 10:08:05 +02:00
@@ -767,7 +767,7 @@
 	if (set_reply)
 		set_bit(IPS_SEEN_REPLY_BIT, &ct->status);
 
-	return ret;
+	return NF_ACCEPT;
 }
 
 int invert_tuplepr(struct ip_conntrack_tuple *inverse,

Re: [PATCH 2/2] Cleanup returnvalues of protocolhandlers

[Martin Josefsson]

On Sun, 19 Sep 2004, Patrick McHardy wrote:

> Patrick McHardy wrote:
>
> > A couple of conversion where missing in ip_conntrack_proto_icmp.c,
> > I've added them to the patch. BTW: shouldn't we return CONNTRACK_INVALID
> > for all invalid icmp packets ? icmp_error returns CONNTRACK_INVALID for
> > these packets, icmp_error_message (after simple 1:1 conversion) returns
> > CONNTRACK_CONT.
>
> Oops, forgot to attach the new patch.

Bah, that's what you get for mostly hand editing other peoples patches :)

Please ignore this second patch, I'll send you an updated version after
I've gone through it properly and fixed the problems you noticed (and
other possible problems)

Thanks

/Martin