[PATCH net-2.6] [NET] The world is not perfect patch.

[PATCH net-2.6] [NET] The world is not perfect patch.

[Rami Rosen]

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

Hi,

There are three methods in the network stack which start with
#ifndef I_WISH_WORLD_WERE_PERFECT.

These methods are:
ipip_err() in net/ipv4/ipip.c
ipgre_err() in net/ipv4/ip_gre.c
ipip6_err() in net/ipv6/sit.c

When building the kernel with -DI_WISH_WORLD_WERE_PERFECT we have
various compilation
errors, mostly due to the fact that the corresponding code was not
updated recently.

(As we all know, indeed the world is not perfect, unluckily...)

This patch fixes these errors and enables building the kernel
successfully with -DI_WISH_WORLD_WERE_PERFECT.

(Note : I am not fully sure about correctness of len = skb->len, which I added;
feedback on this point is welcomed).


Regards,
Rami Rosen


Signed-off-by: Rami Rosen <ramirose@gmail.com>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3599 bytes --]

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 2ada033..27c3938 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -399,9 +399,9 @@ out:
 	read_unlock(&ipgre_lock);
 	return;
 #else
-	struct iphdr *iph = (struct iphdr*)dp;
+	struct iphdr *iph = (struct iphdr*)skb->data;
 	struct iphdr *eiph;
-	__be16	     *p = (__be16*)(dp+(iph->ihl<<2));
+	__be16	     *p = (__be16*)(skb->data + (iph->ihl<<2));
 	const int type = icmp_hdr(skb)->type;
 	const int code = icmp_hdr(skb)->code;
 	int rel_type = 0;
@@ -413,7 +413,9 @@ out:
 	struct sk_buff *skb2;
 	struct flowi fl;
 	struct rtable *rt;
-
+	
+	int len = skb->len;
+	
 	if (p[1] != htons(ETH_P_IP))
 		return;
 
@@ -430,7 +432,7 @@ out:
 	}
 	if (len < grehlen + sizeof(struct iphdr))
 		return;
-	eiph = (struct iphdr*)(dp + grehlen);
+	eiph = (struct iphdr*)(skb->data + grehlen);
 
 	switch (type) {
 	default:
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 149111f..4474f41 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -338,7 +338,7 @@ out:
 	read_unlock(&ipip_lock);
 	return err;
 #else
-	struct iphdr *iph = (struct iphdr*)dp;
+	struct iphdr *iph = (struct iphdr*)skb->data;
 	int hlen = iph->ihl<<2;
 	struct iphdr *eiph;
 	const int type = icmp_hdr(skb)->type;
@@ -351,9 +351,11 @@ out:
 	struct flowi fl;
 	struct rtable *rt;
 
+	int len = skb->len;
+
 	if (len < hlen + sizeof(struct iphdr))
 		return 0;
-	eiph = (struct iphdr*)(dp + hlen);
+	eiph = (struct iphdr*)(skb->data + hlen);
 
 	switch (type) {
 	default:
@@ -414,10 +416,10 @@ out:
 
 	/* Try to guess incoming interface */
 	memset(&fl, 0, sizeof(fl));
-	fl.fl4_daddr = eiph->saddr;
+	fl.fl4_dst = eiph->saddr;
 	fl.fl4_tos = RT_TOS(eiph->tos);
 	fl.proto = IPPROTO_IPIP;
-	if (ip_route_output_key(dev_net(skb->dev), &rt, &key)) {
+	if (ip_route_output_key(dev_net(skb->dev), &rt, &fl)) {
 		kfree_skb(skb2);
 		return 0;
 	}
@@ -427,7 +429,7 @@ out:
 	if (rt->rt_flags&RTCF_LOCAL) {
 		ip_rt_put(rt);
 		rt = NULL;
-		fl.fl4_daddr = eiph->daddr;
+		fl.fl4_dst = eiph->daddr;
 		fl.fl4_src = eiph->saddr;
 		fl.fl4_tos = eiph->tos;
 		if (ip_route_output_key(dev_net(skb->dev), &rt, &fl) ||
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 5a6fab9..c347c67 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -463,7 +463,7 @@ out:
 	read_unlock(&ipip6_lock);
 	return err;
 #else
-	struct iphdr *iph = (struct iphdr*)dp;
+	struct iphdr *iph = (struct iphdr*)skb->data;
 	int hlen = iph->ihl<<2;
 	struct ipv6hdr *iph6;
 	const int type = icmp_hdr(skb)->type;
@@ -473,17 +473,19 @@ out:
 	int rel_info = 0;
 	struct sk_buff *skb2;
 	struct rt6_info *rt6i;
-
+	
+	int len = skb->len;
+	
 	if (len < hlen + sizeof(struct ipv6hdr))
-		return;
-	iph6 = (struct ipv6hdr*)(dp + hlen);
+		return 0;
+	iph6 = (struct ipv6hdr*)(skb->data + hlen);
 
 	switch (type) {
 	default:
-		return;
+		return 0;
 	case ICMP_PARAMETERPROB:
 		if (icmp_hdr(skb)->un.gateway < hlen)
-			return;
+			return 0;
 
 		/* So... This guy found something strange INSIDE encapsulated
 		   packet. Well, he is fool, but what can we do ?
@@ -497,10 +499,10 @@ out:
 		case ICMP_SR_FAILED:
 		case ICMP_PORT_UNREACH:
 			/* Impossible event. */
-			return;
+			return 0;
 		case ICMP_FRAG_NEEDED:
 			/* Too complicated case ... */
-			return;
+			return 0;
 		default:
 			/* All others are translated to HOST_UNREACH.
 			   rfc2003 contains "deep thoughts" about NET_UNREACH,
@@ -513,7 +515,7 @@ out:
 		break;
 	case ICMP_TIME_EXCEEDED:
 		if (code != ICMP_EXC_TTL)
-			return;
+			return 0;
 		rel_type = ICMPV6_TIME_EXCEED;
 		rel_code = ICMPV6_EXC_HOPLIMIT;
 		break;

Re: [PATCH net-2.6] [NET] The world is not perfect patch.

[David Miller]

From: "Rami Rosen" <ramirose@gmail.com>
Date: Tue, 20 May 2008 12:29:26 +0300

> There are three methods in the network stack which start with
> #ifndef I_WISH_WORLD_WERE_PERFECT.

I think it is better to delete this code.  It is impossible
for anyone to build test it without making source modifications,
therefore it will continute to break over and over again over
time.

Re: [PATCH net-2.6] [NET] The world is not perfect patch.

[Rami Rosen]

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

Hi,
  Unless there will be any objection here, I suggest consider the
following patch which simply removes the code for the
-DI_WISH_WORLD_WERE_PERFECT in the three methods which use it.

The compilation errors we get when using -DI_WISH_WORLD_WERE_PERFECT
show that this code was not built and not used for really a long time.

Regards,
Rami Rosen


Signed-off-by: Rami Rosen <ramirose@gmail.com>


On Wed, May 21, 2008 at 12:33 AM, David Miller <davem@davemloft.net> wrote:
> From: "Rami Rosen" <ramirose@gmail.com>
> Date: Tue, 20 May 2008 12:29:26 +0300
>
>> There are three methods in the network stack which start with
>> #ifndef I_WISH_WORLD_WERE_PERFECT.
>
> I think it is better to delete this code.  It is impossible
> for anyone to build test it without making source modifications,
> therefore it will continute to break over and over again over
> time.
>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 11150 bytes --]

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 2ada033..4342cba 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -313,9 +313,8 @@ static void ipgre_tunnel_uninit(struct net_device *dev)
 
 static void ipgre_err(struct sk_buff *skb, u32 info)
 {
-#ifndef I_WISH_WORLD_WERE_PERFECT
 
-/* It is not :-( All the routers (except for Linux) return only
+/* All the routers (except for Linux) return only
    8 bytes of packet payload. It means, that precise relaying of
    ICMP in the real Internet is absolutely infeasible.
 
@@ -398,149 +397,6 @@ static void ipgre_err(struct sk_buff *skb, u32 info)
 out:
 	read_unlock(&ipgre_lock);
 	return;
-#else
-	struct iphdr *iph = (struct iphdr*)dp;
-	struct iphdr *eiph;
-	__be16	     *p = (__be16*)(dp+(iph->ihl<<2));
-	const int type = icmp_hdr(skb)->type;
-	const int code = icmp_hdr(skb)->code;
-	int rel_type = 0;
-	int rel_code = 0;
-	__be32 rel_info = 0;
-	__u32 n = 0;
-	__be16 flags;
-	int grehlen = (iph->ihl<<2) + 4;
-	struct sk_buff *skb2;
-	struct flowi fl;
-	struct rtable *rt;
-
-	if (p[1] != htons(ETH_P_IP))
-		return;
-
-	flags = p[0];
-	if (flags&(GRE_CSUM|GRE_KEY|GRE_SEQ|GRE_ROUTING|GRE_VERSION)) {
-		if (flags&(GRE_VERSION|GRE_ROUTING))
-			return;
-		if (flags&GRE_CSUM)
-			grehlen += 4;
-		if (flags&GRE_KEY)
-			grehlen += 4;
-		if (flags&GRE_SEQ)
-			grehlen += 4;
-	}
-	if (len < grehlen + sizeof(struct iphdr))
-		return;
-	eiph = (struct iphdr*)(dp + grehlen);
-
-	switch (type) {
-	default:
-		return;
-	case ICMP_PARAMETERPROB:
-		n = ntohl(icmp_hdr(skb)->un.gateway) >> 24;
-		if (n < (iph->ihl<<2))
-			return;
-
-		/* So... This guy found something strange INSIDE encapsulated
-		   packet. Well, he is fool, but what can we do ?
-		 */
-		rel_type = ICMP_PARAMETERPROB;
-		n -= grehlen;
-		rel_info = htonl(n << 24);
-		break;
-
-	case ICMP_DEST_UNREACH:
-		switch (code) {
-		case ICMP_SR_FAILED:
-		case ICMP_PORT_UNREACH:
-			/* Impossible event. */
-			return;
-		case ICMP_FRAG_NEEDED:
-			/* And it is the only really necessary thing :-) */
-			n = ntohs(icmp_hdr(skb)->un.frag.mtu);
-			if (n < grehlen+68)
-				return;
-			n -= grehlen;
-			/* BSD 4.2 MORE DOES NOT EXIST IN NATURE. */
-			if (n > ntohs(eiph->tot_len))
-				return;
-			rel_info = htonl(n);
-			break;
-		default:
-			/* All others are translated to HOST_UNREACH.
-			   rfc2003 contains "deep thoughts" about NET_UNREACH,
-			   I believe, it is just ether pollution. --ANK
-			 */
-			rel_type = ICMP_DEST_UNREACH;
-			rel_code = ICMP_HOST_UNREACH;
-			break;
-		}
-		break;
-	case ICMP_TIME_EXCEEDED:
-		if (code != ICMP_EXC_TTL)
-			return;
-		break;
-	}
-
-	/* Prepare fake skb to feed it to icmp_send */
-	skb2 = skb_clone(skb, GFP_ATOMIC);
-	if (skb2 == NULL)
-		return;
-	dst_release(skb2->dst);
-	skb2->dst = NULL;
-	skb_pull(skb2, skb->data - (u8*)eiph);
-	skb_reset_network_header(skb2);
-
-	/* Try to guess incoming interface */
-	memset(&fl, 0, sizeof(fl));
-	fl.fl4_dst = eiph->saddr;
-	fl.fl4_tos = RT_TOS(eiph->tos);
-	fl.proto = IPPROTO_GRE;
-	if (ip_route_output_key(dev_net(skb->dev), &rt, &fl)) {
-		kfree_skb(skb2);
-		return;
-	}
-	skb2->dev = rt->u.dst.dev;
-
-	/* route "incoming" packet */
-	if (rt->rt_flags&RTCF_LOCAL) {
-		ip_rt_put(rt);
-		rt = NULL;
-		fl.fl4_dst = eiph->daddr;
-		fl.fl4_src = eiph->saddr;
-		fl.fl4_tos = eiph->tos;
-		if (ip_route_output_key(dev_net(skb->dev), &rt, &fl) ||
-		    rt->u.dst.dev->type != ARPHRD_IPGRE) {
-			ip_rt_put(rt);
-			kfree_skb(skb2);
-			return;
-		}
-	} else {
-		ip_rt_put(rt);
-		if (ip_route_input(skb2, eiph->daddr, eiph->saddr, eiph->tos, skb2->dev) ||
-		    skb2->dst->dev->type != ARPHRD_IPGRE) {
-			kfree_skb(skb2);
-			return;
-		}
-	}
-
-	/* change mtu on this route */
-	if (type == ICMP_DEST_UNREACH && code == ICMP_FRAG_NEEDED) {
-		if (n > dst_mtu(skb2->dst)) {
-			kfree_skb(skb2);
-			return;
-		}
-		skb2->dst->ops->update_pmtu(skb2->dst, n);
-	} else if (type == ICMP_TIME_EXCEEDED) {
-		struct ip_tunnel *t = netdev_priv(skb2->dev);
-		if (t->parms.iph.ttl) {
-			rel_type = ICMP_DEST_UNREACH;
-			rel_code = ICMP_HOST_UNREACH;
-		}
-	}
-
-	icmp_send(skb2, rel_type, rel_code, rel_info);
-	kfree_skb(skb2);
-#endif
 }
 
 static inline void ipgre_ecn_decapsulate(struct iphdr *iph, struct sk_buff *skb)
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 149111f..af5cb53 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -278,9 +278,8 @@ static void ipip_tunnel_uninit(struct net_device *dev)
 
 static int ipip_err(struct sk_buff *skb, u32 info)
 {
-#ifndef I_WISH_WORLD_WERE_PERFECT
 
-/* It is not :-( All the routers (except for Linux) return only
+/* All the routers (except for Linux) return only
    8 bytes of packet payload. It means, that precise relaying of
    ICMP in the real Internet is absolutely infeasible.
  */
@@ -337,133 +336,6 @@ static int ipip_err(struct sk_buff *skb, u32 info)
 out:
 	read_unlock(&ipip_lock);
 	return err;
-#else
-	struct iphdr *iph = (struct iphdr*)dp;
-	int hlen = iph->ihl<<2;
-	struct iphdr *eiph;
-	const int type = icmp_hdr(skb)->type;
-	const int code = icmp_hdr(skb)->code;
-	int rel_type = 0;
-	int rel_code = 0;
-	__be32 rel_info = 0;
-	__u32 n = 0;
-	struct sk_buff *skb2;
-	struct flowi fl;
-	struct rtable *rt;
-
-	if (len < hlen + sizeof(struct iphdr))
-		return 0;
-	eiph = (struct iphdr*)(dp + hlen);
-
-	switch (type) {
-	default:
-		return 0;
-	case ICMP_PARAMETERPROB:
-		n = ntohl(icmp_hdr(skb)->un.gateway) >> 24;
-		if (n < hlen)
-			return 0;
-
-		/* So... This guy found something strange INSIDE encapsulated
-		   packet. Well, he is fool, but what can we do ?
-		 */
-		rel_type = ICMP_PARAMETERPROB;
-		rel_info = htonl((n - hlen) << 24);
-		break;
-
-	case ICMP_DEST_UNREACH:
-		switch (code) {
-		case ICMP_SR_FAILED:
-		case ICMP_PORT_UNREACH:
-			/* Impossible event. */
-			return 0;
-		case ICMP_FRAG_NEEDED:
-			/* And it is the only really necessary thing :-) */
-			n = ntohs(icmp_hdr(skb)->un.frag.mtu);
-			if (n < hlen+68)
-				return 0;
-			n -= hlen;
-			/* BSD 4.2 MORE DOES NOT EXIST IN NATURE. */
-			if (n > ntohs(eiph->tot_len))
-				return 0;
-			rel_info = htonl(n);
-			break;
-		default:
-			/* All others are translated to HOST_UNREACH.
-			   rfc2003 contains "deep thoughts" about NET_UNREACH,
-			   I believe, it is just ether pollution. --ANK
-			 */
-			rel_type = ICMP_DEST_UNREACH;
-			rel_code = ICMP_HOST_UNREACH;
-			break;
-		}
-		break;
-	case ICMP_TIME_EXCEEDED:
-		if (code != ICMP_EXC_TTL)
-			return 0;
-		break;
-	}
-
-	/* Prepare fake skb to feed it to icmp_send */
-	skb2 = skb_clone(skb, GFP_ATOMIC);
-	if (skb2 == NULL)
-		return 0;
-	dst_release(skb2->dst);
-	skb2->dst = NULL;
-	skb_pull(skb2, skb->data - (u8*)eiph);
-	skb_reset_network_header(skb2);
-
-	/* Try to guess incoming interface */
-	memset(&fl, 0, sizeof(fl));
-	fl.fl4_daddr = eiph->saddr;
-	fl.fl4_tos = RT_TOS(eiph->tos);
-	fl.proto = IPPROTO_IPIP;
-	if (ip_route_output_key(dev_net(skb->dev), &rt, &key)) {
-		kfree_skb(skb2);
-		return 0;
-	}
-	skb2->dev = rt->u.dst.dev;
-
-	/* route "incoming" packet */
-	if (rt->rt_flags&RTCF_LOCAL) {
-		ip_rt_put(rt);
-		rt = NULL;
-		fl.fl4_daddr = eiph->daddr;
-		fl.fl4_src = eiph->saddr;
-		fl.fl4_tos = eiph->tos;
-		if (ip_route_output_key(dev_net(skb->dev), &rt, &fl) ||
-		    rt->u.dst.dev->type != ARPHRD_TUNNEL) {
-			ip_rt_put(rt);
-			kfree_skb(skb2);
-			return 0;
-		}
-	} else {
-		ip_rt_put(rt);
-		if (ip_route_input(skb2, eiph->daddr, eiph->saddr, eiph->tos, skb2->dev) ||
-		    skb2->dst->dev->type != ARPHRD_TUNNEL) {
-			kfree_skb(skb2);
-			return 0;
-		}
-	}
-
-	/* change mtu on this route */
-	if (type == ICMP_DEST_UNREACH && code == ICMP_FRAG_NEEDED) {
-		if (n > dst_mtu(skb2->dst)) {
-			kfree_skb(skb2);
-			return 0;
-		}
-		skb2->dst->ops->update_pmtu(skb2->dst, n);
-	} else if (type == ICMP_TIME_EXCEEDED) {
-		struct ip_tunnel *t = netdev_priv(skb2->dev);
-		if (t->parms.iph.ttl) {
-			rel_type = ICMP_DEST_UNREACH;
-			rel_code = ICMP_HOST_UNREACH;
-		}
-	}
-
-	icmp_send(skb2, rel_type, rel_code, rel_info);
-	kfree_skb(skb2);
-	return 0;
-#endif
 }
 
 static inline void ipip_ecn_decapsulate(const struct iphdr *outer_iph,
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 5a6fab9..3de6ffd 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -403,9 +403,8 @@ static void ipip6_tunnel_uninit(struct net_device *dev)
 
 static int ipip6_err(struct sk_buff *skb, u32 info)
 {
-#ifndef I_WISH_WORLD_WERE_PERFECT
 
-/* It is not :-( All the routers (except for Linux) return only
+/* All the routers (except for Linux) return only
    8 bytes of packet payload. It means, that precise relaying of
    ICMP in the real Internet is absolutely infeasible.
  */
@@ -462,92 +461,6 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
 out:
 	read_unlock(&ipip6_lock);
 	return err;
-#else
-	struct iphdr *iph = (struct iphdr*)dp;
-	int hlen = iph->ihl<<2;
-	struct ipv6hdr *iph6;
-	const int type = icmp_hdr(skb)->type;
-	const int code = icmp_hdr(skb)->code;
-	int rel_type = 0;
-	int rel_code = 0;
-	int rel_info = 0;
-	struct sk_buff *skb2;
-	struct rt6_info *rt6i;
-
-	if (len < hlen + sizeof(struct ipv6hdr))
-		return;
-	iph6 = (struct ipv6hdr*)(dp + hlen);
-
-	switch (type) {
-	default:
-		return;
-	case ICMP_PARAMETERPROB:
-		if (icmp_hdr(skb)->un.gateway < hlen)
-			return;
-
-		/* So... This guy found something strange INSIDE encapsulated
-		   packet. Well, he is fool, but what can we do ?
-		 */
-		rel_type = ICMPV6_PARAMPROB;
-		rel_info = icmp_hdr(skb)->un.gateway - hlen;
-		break;
-
-	case ICMP_DEST_UNREACH:
-		switch (code) {
-		case ICMP_SR_FAILED:
-		case ICMP_PORT_UNREACH:
-			/* Impossible event. */
-			return;
-		case ICMP_FRAG_NEEDED:
-			/* Too complicated case ... */
-			return;
-		default:
-			/* All others are translated to HOST_UNREACH.
-			   rfc2003 contains "deep thoughts" about NET_UNREACH,
-			   I believe, it is just ether pollution. --ANK
-			 */
-			rel_type = ICMPV6_DEST_UNREACH;
-			rel_code = ICMPV6_ADDR_UNREACH;
-			break;
-		}
-		break;
-	case ICMP_TIME_EXCEEDED:
-		if (code != ICMP_EXC_TTL)
-			return;
-		rel_type = ICMPV6_TIME_EXCEED;
-		rel_code = ICMPV6_EXC_HOPLIMIT;
-		break;
-	}
-
-	/* Prepare fake skb to feed it to icmpv6_send */
-	skb2 = skb_clone(skb, GFP_ATOMIC);
-	if (skb2 == NULL)
-		return 0;
-	dst_release(skb2->dst);
-	skb2->dst = NULL;
-	skb_pull(skb2, skb->data - (u8*)iph6);
-	skb_reset_network_header(skb2);
-
-	/* Try to guess incoming interface */
-	rt6i = rt6_lookup(dev_net(skb->dev), &iph6->saddr, NULL, NULL, 0);
-	if (rt6i && rt6i->rt6i_dev) {
-		skb2->dev = rt6i->rt6i_dev;
-
-		rt6i = rt6_lookup(dev_net(skb->dev),
-				&iph6->daddr, &iph6->saddr, NULL, 0);
-
-		if (rt6i && rt6i->rt6i_dev && rt6i->rt6i_dev->type == ARPHRD_SIT) {
-			struct ip_tunnel *t = netdev_priv(rt6i->rt6i_dev);
-			if (rel_type == ICMPV6_TIME_EXCEED && t->parms.iph.ttl) {
-				rel_type = ICMPV6_DEST_UNREACH;
-				rel_code = ICMPV6_ADDR_UNREACH;
-			}
-			icmpv6_send(skb2, rel_type, rel_code, rel_info, skb2->dev);
-		}
-	}
-	kfree_skb(skb2);
-	return 0;
-#endif
 }
 
 static inline void ipip6_ecn_decapsulate(struct iphdr *iph, struct sk_buff *skb)

Re: [PATCH net-2.6] [NET] The world is not perfect patch.

[David Miller]

From: "Rami Rosen" <ramirose@gmail.com>
Date: Wed, 21 May 2008 10:33:35 +0300

>   Unless there will be any objection here, I suggest consider the
> following patch which simply removes the code for the
> -DI_WISH_WORLD_WERE_PERFECT in the three methods which use it.
> 
> The compilation errors we get when using -DI_WISH_WORLD_WERE_PERFECT
> show that this code was not built and not used for really a long time.

Applied, thanks for following up on this Rami.