[2.6 PATCH] ipvs - avoid returning NF_DROP from the packet schedulers

[2.6 PATCH] ipvs - avoid returning NF_DROP from the packet schedulers

[Julian Anastasov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 215 bytes --]


	Hello,

	It seems my NF_DROP approach leads to new bugs. So, better
free skb, return NF_STOLEN and add some comments. If someone
wants to fix it in another way, go ahead.

Regards

--
Julian Anastasov <ja@ssi.bg>

[-- Attachment #2: Better return NF_STOLEN --]
[-- Type: TEXT/PLAIN, Size: 3902 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1501  -> 1.1502 
#	net/ipv4/ipvs/ip_vs_core.c	1.5     -> 1.6    
#	 include/net/ip_vs.h	1.6     -> 1.7    
#	net/ipv4/ipvs/ip_vs_xmit.c	1.3     -> 1.4    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/10/08	ja@ssi.bg	1.1502
# [IPVS]: avoid returning NF_DROP from the packet schedulers
# --------------------------------------------
#
diff -Nru a/include/net/ip_vs.h b/include/net/ip_vs.h
--- a/include/net/ip_vs.h	Wed Oct  8 10:59:05 2003
+++ b/include/net/ip_vs.h	Wed Oct  8 10:59:05 2003
@@ -525,8 +525,8 @@
 	atomic_t                in_pkts;        /* incoming packet counter */
 
 	/* packet transmitter for different forwarding methods.  If it
-	   mangles the packet, it must return NF_DROP or NF_STOLEN, otherwise
-	   this must be changed to a sk_buff **.
+	   mangles the packet, it must return NF_DROP or better NF_STOLEN,
+	   otherwise this must be changed to a sk_buff **.
 	 */
 	int (*packet_xmit)(struct sk_buff *skb, struct ip_vs_conn *cp,
 			   struct ip_vs_protocol *pp);
diff -Nru a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
--- a/net/ipv4/ipvs/ip_vs_core.c	Wed Oct  8 10:59:05 2003
+++ b/net/ipv4/ipvs/ip_vs_core.c	Wed Oct  8 10:59:05 2003
@@ -485,6 +485,7 @@
 
 		/* transmit the first SYN packet */
 		ret = cp->packet_xmit(skb, cp, pp);
+		/* do not touch skb anymore */
 
 		atomic_inc(&cp->in_pkts);
 		ip_vs_conn_put(cp);
@@ -822,7 +823,8 @@
 
   drop:
 	ip_vs_conn_put(cp);
-	return NF_DROP;
+	kfree_skb(*pskb);
+	return NF_STOLEN;
 }
 
 
@@ -939,6 +941,7 @@
 	if (IPPROTO_TCP == ciph.protocol || IPPROTO_UDP == ciph.protocol)
 		offset += 2 * sizeof(__u16);
 	verdict = ip_vs_icmp_xmit(skb, cp, pp, offset);
+	/* do not touch skb anymore */
 
   out:
 	__ip_vs_conn_put(cp);
@@ -1032,6 +1035,7 @@
 	restart = ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pp);
 	if (cp->packet_xmit)
 		ret = cp->packet_xmit(skb, cp, pp);
+		/* do not touch skb anymore */
 	else {
 		IP_VS_DBG_RL("warning: packet_xmit is null");
 		ret = NF_ACCEPT;
diff -Nru a/net/ipv4/ipvs/ip_vs_xmit.c b/net/ipv4/ipvs/ip_vs_xmit.c
--- a/net/ipv4/ipvs/ip_vs_xmit.c	Wed Oct  8 10:59:05 2003
+++ b/net/ipv4/ipvs/ip_vs_xmit.c	Wed Oct  8 10:59:05 2003
@@ -143,6 +143,7 @@
 ip_vs_null_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 		struct ip_vs_protocol *pp)
 {
+	/* we do not touch skb and do not need pskb ptr */
 	return NF_ACCEPT;
 }
 
@@ -214,8 +215,9 @@
  tx_error_icmp:
 	dst_link_failure(skb);
  tx_error:
+	kfree_skb(skb);
 	LeaveFunction(10);
-	return NF_DROP;
+	return NF_STOLEN;
 }
 
 
@@ -292,7 +294,8 @@
 	dst_link_failure(skb);
   tx_error:
 	LeaveFunction(10);
-	return NF_DROP;
+	kfree_skb(skb);
+	return NF_STOLEN;
   tx_error_put:
 	ip_rt_put(rt);
 	goto tx_error;
@@ -375,8 +378,9 @@
 			skb_realloc_headroom(skb, max_headroom);
 		if (!new_skb) {
 			ip_rt_put(rt);
+			kfree_skb(skb);
 			IP_VS_ERR_RL("ip_vs_tunnel_xmit(): no memory\n");
-			return NF_DROP;
+			return NF_STOLEN;
 		}
 		kfree_skb(skb);
 		skb = new_skb;
@@ -429,8 +433,9 @@
   tx_error_icmp:
 	dst_link_failure(skb);
   tx_error:
+	kfree_skb(skb);
 	LeaveFunction(10);
-	return NF_DROP;
+	return NF_STOLEN;
 }
 
 
@@ -488,8 +493,9 @@
   tx_error_icmp:
 	dst_link_failure(skb);
   tx_error:
+	kfree_skb(skb);
 	LeaveFunction(10);
-	return NF_DROP;
+	return NF_STOLEN;
 }
 
 
@@ -515,6 +521,7 @@
 			rc = cp->packet_xmit(skb, cp, pp);
 		else
 			rc = NF_ACCEPT;
+		/* do not touch skb anymore */
 		atomic_inc(&cp->in_pkts);
 		__ip_vs_conn_put(cp);
 		goto out;

Re: [2.6 PATCH] ipvs - avoid returning NF_DROP from the packet schedulers

[David S. Miller]

On Wed, 8 Oct 2003 11:09:20 +0300 (EEST)
Julian Anastasov <ja@ssi.bg> wrote:

> 	It seems my NF_DROP approach leads to new bugs. So, better
> free skb, return NF_STOLEN and add some comments. If someone
> wants to fix it in another way, go ahead.

Applied, thanks.