[PATCH 1/1] sysctl configurable icmperror sourceaddress

[PATCH 1/1] sysctl configurable icmperror sourceaddress

[J. Simonetti]

This patch alows you to change the source address of icmp error
messages. It applies cleanly to 2.6.11.11 and retains the default
behaviour.

In the old (default) behaviour icmp error messages are sent with the ip
of the exiting interface.
The new behaviour (when the sysctl variable is toggled on), it will send
the message with the ip of the interface that received the packet that
caused the icmp error. This is the behaviour network administrators will
expect from a router. It makes debugging complicated network layouts
much easier. Also, all 'vendor routers' I know of have the later
behaviour.

Regards,

Jeroen Simonetti
--
"Absolutely nothing should be concluded from these figures except that
no conclusion can be drawn from them."
(By Joseph L. Brothers, Linux/PowerPC Project)

Re: [PATCH 1/1] sysctl configurable icmperror sourceaddress

[J. Simonetti]

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

On Tue, 2005-06-07 at 11:26 +0200, J. Simonetti wrote:
> This patch alows you to change the source address of icmp error
> messages. It applies cleanly to 2.6.11.11 and retains the default
> behaviour.

I swear I had attached it... really... ;)

Jeroen Simonetti
--
character density, n.:
	The number of very weird people in the office.

[-- Attachment #2: linux-2.6.11.11-icmperrors.patch --]
[-- Type: text/x-patch, Size: 1807 bytes --]

--- include/linux/sysctl.h.orig	2004-12-24 22:34:58.000000000 +0100
+++ include/linux/sysctl.h	2005-06-07 10:16:39.730585288 +0200
@@ -345,6 +345,7 @@
 	NET_TCP_MODERATE_RCVBUF=106,
 	NET_TCP_TSO_WIN_DIVISOR=107,
	NET_TCP_BIC_BETA=108,
+	NET_IPV4_ICMP_ERRORS_USE_INBOUND_IFADDR=109,
 };
 
 enum {
--- net/ipv4/icmp.c.orig	2004-12-24 22:35:28.000000000 +0100
+++ net/ipv4/icmp.c	2005-06-07 10:15:42.645263576 +0200
@@ -207,6 +207,7 @@
 
 int sysctl_icmp_ratelimit = 1 * HZ;
 int sysctl_icmp_ratemask = 0x1818;
+int sysctl_icmp_errors_use_inbound_ifaddr = 0;
 
 /*
  *	ICMP control array. This specifies what to do with each ICMP.
@@ -511,8 +512,12 @@
 	 */
 
 	saddr = iph->daddr;
-	if (!(rt->rt_flags & RTCF_LOCAL))
-		saddr = 0;
+	if (!(rt->rt_flags & RTCF_LOCAL)) {
+		if(sysctl_icmp_errors_use_inbound_ifaddr)
+			saddr = inet_select_addr(skb_in->dev, 0, RT_SCOPE_LINK);
+		else
+			saddr = 0;
+	}
 
 	tos = icmp_pointers[type].error ? ((iph->tos & IPTOS_TOS_MASK) |
 					   IPTOS_PREC_INTERNETCONTROL) :
--- net/ipv4/sysctl_net_ipv4.c.orig	2004-12-24 22:35:23.000000000 +0100
+++ net/ipv4/sysctl_net_ipv4.c	2005-06-07 10:19:44.538490216 +0200
@@ -23,6 +23,7 @@
 extern int sysctl_icmp_echo_ignore_all;
 extern int sysctl_icmp_echo_ignore_broadcasts;
 extern int sysctl_icmp_ignore_bogus_error_responses;
+extern int sysctl_icmp_errors_use_inbound_ifaddr;
 
 /* From ip_fragment.c */
 extern int sysctl_ipfrag_low_thresh;
@@ -396,6 +397,14 @@
 		.proc_handler	= &proc_dointvec
 	},
 	{
+		.ctl_name	= NET_IPV4_ICMP_ERRORS_USE_INBOUND_IFADDR,
+		.procname	= "icmp_errors_use_inbound_ifaddr",
+		.data		= &sysctl_icmp_errors_use_inbound_ifaddr,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec
+	},
+	{
 		.ctl_name	= NET_IPV4_ROUTE,
 		.procname	= "route",
 		.maxlen		= 0,

Re: [PATCH 1/1] sysctl configurable icmperror sourceaddress

[Lennert Buytenhek]

On Tue, Jun 07, 2005 at 11:26:23AM +0200, J. Simonetti wrote:

> This patch alows you to change the source address of icmp error
> messages. It applies cleanly to 2.6.11.11 and retains the default
> behaviour.
> 
> In the old (default) behaviour icmp error messages are sent with the ip
> of the exiting interface.
> The new behaviour (when the sysctl variable is toggled on), it will send
> the message with the ip of the interface that received the packet that
> caused the icmp error. This is the behaviour network administrators will
> expect from a router. It makes debugging complicated network layouts
> much easier. Also, all 'vendor routers' I know of have the later
> behaviour.

Can this patch go in, pretty please?

Here's the patch again for reference:


--- include/linux/sysctl.h.orig	2004-12-24 22:34:58.000000000 +0100
+++ include/linux/sysctl.h	2005-06-07 10:16:39.730585288 +0200
@@ -345,6 +345,7 @@
 	NET_TCP_MODERATE_RCVBUF=106,
 	NET_TCP_TSO_WIN_DIVISOR=107,
	NET_TCP_BIC_BETA=108,
+	NET_IPV4_ICMP_ERRORS_USE_INBOUND_IFADDR=109,
 };
 
 enum {
--- net/ipv4/icmp.c.orig	2004-12-24 22:35:28.000000000 +0100
+++ net/ipv4/icmp.c	2005-06-07 10:15:42.645263576 +0200
@@ -207,6 +207,7 @@
 
 int sysctl_icmp_ratelimit = 1 * HZ;
 int sysctl_icmp_ratemask = 0x1818;
+int sysctl_icmp_errors_use_inbound_ifaddr = 0;
 
 /*
  *	ICMP control array. This specifies what to do with each ICMP.
@@ -511,8 +512,12 @@
 	 */
 
 	saddr = iph->daddr;
-	if (!(rt->rt_flags & RTCF_LOCAL))
-		saddr = 0;
+	if (!(rt->rt_flags & RTCF_LOCAL)) {
+		if(sysctl_icmp_errors_use_inbound_ifaddr)
+			saddr = inet_select_addr(skb_in->dev, 0, RT_SCOPE_LINK);
+		else
+			saddr = 0;
+	}
 
 	tos = icmp_pointers[type].error ? ((iph->tos & IPTOS_TOS_MASK) |
 					   IPTOS_PREC_INTERNETCONTROL) :
--- net/ipv4/sysctl_net_ipv4.c.orig	2004-12-24 22:35:23.000000000 +0100
+++ net/ipv4/sysctl_net_ipv4.c	2005-06-07 10:19:44.538490216 +0200
@@ -23,6 +23,7 @@
 extern int sysctl_icmp_echo_ignore_all;
 extern int sysctl_icmp_echo_ignore_broadcasts;
 extern int sysctl_icmp_ignore_bogus_error_responses;
+extern int sysctl_icmp_errors_use_inbound_ifaddr;
 
 /* From ip_fragment.c */
 extern int sysctl_ipfrag_low_thresh;
@@ -396,6 +397,14 @@
 		.proc_handler	= &proc_dointvec
 	},
 	{
+		.ctl_name	= NET_IPV4_ICMP_ERRORS_USE_INBOUND_IFADDR,
+		.procname	= "icmp_errors_use_inbound_ifaddr",
+		.data		= &sysctl_icmp_errors_use_inbound_ifaddr,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec
+	},
+	{
 		.ctl_name	= NET_IPV4_ROUTE,
 		.procname	= "route",
 		.maxlen		= 0,

Re: [PATCH 1/1] sysctl configurable icmperror sourceaddress

[David S. Miller]

From: Lennert Buytenhek <buytenh@wantstofly.org>
Date: Wed, 8 Jun 2005 23:45:40 +0200

> Can this patch go in, pretty please?
> 
> Here's the patch again for reference:

We have a similar sysctl for ARP handling, why don't
we make use of it?

Re: [PATCH 1/1] sysctl configurable icmperror sourceaddress

[Lennert Buytenhek]

On Wed, Jun 08, 2005 at 02:49:41PM -0700, David S. Miller wrote:

> > Can this patch go in, pretty please?
> > 
> > Here's the patch again for reference:
> 
> We have a similar sysctl for ARP handling, why don't
> we make use of it?

Which one do you mean, the arp_{filter,announce,ignore} ones?  IMHO
this is a very different issue, this patch just selects which source
address we use when we reply to a packet with an ICMP.

In the case below, if all routers are linux routers, a traceroute
from source to dest will show something like this:
	1 10.0.0.254 x.xxx ms x.xxx ms x.xxx ms
	2 12.0.0.100 x.xxx ms x.xxx ms x.xxx ms
	3 12.0.0.1 x.xxx ms x.xxx ms x.xxx ms

Whereas we'd prefer seeing this, partly because it makes more sense,
partly because a very large fraction of networking hardware does it
this way too:
	1 10.0.0.254 x.xxx ms x.xxx ms x.xxx ms
	2 11.0.0.100 x.xxx ms x.xxx ms x.xxx ms   <===
	3 12.0.0.1 x.xxx ms x.xxx ms x.xxx ms

I used to work at an ISP and there are a number of practical cases
where the linux behavior is rather confusing.


cheers,
Lennert


	+------------+
	|   source   |
	|  10.0.0.1  |
	+-----+------+
	      |
	      V
	      |
	      |
	+-----+------+
	| 10.0.0.254 |
	|            |
	|   router1  +---------------------+
	|            |                     |
	| 11.0.0.254 |                     |
	+-----+------+                     |
	      |                            |
	      V                            ^ asymmetric route
	      |                            |  back to source
	      |                            |
	+-----+------+               +-----+------+
	| 11.0.0.100 |               | 13.0.0.13  |
	|            |               |            |
	|   router2  |               |   router3  |
	|            |               |            |
	| 12.0.0.100 |               | 12.0.0.101 |
	+-----+------+               +-----+------+
	      |                            |
	      +------------->--------------+
	      V
	      |
	+-----+------+
	|    dest    |
	|  12.0.0.1  |
	+------------+