Reply net unreachable ICMP message

PATCH In review Mitsuru Chinen 4 participants 7 messages Dec 6, 2007 – Dec 7, 2007

Subsystems (3)

Networking Network drivers IPv4/IPv6

lore

Share & tools

lore

Mitsuru Chinen

author

2007-12-06 06:31 UTC #0 lore raw

PATCH 1/1 Reply net unreachable ICMP message

IPv4 stack doesn't reply any ICMP destination unreachable message with net unreachable code when IP detagrams are being discarded because of no route could be found in the forwarding path. Incidentally, IPv6 stack replies such ICMPv6 message in the similar situation.

Signed-off-by Mitsuru Chinen <[email protected]>

---
 net/ipv4/route.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

net/ipv4/route.c +2 -0

index 6714bbc..ba85ec9 100644
@@ -1375,6 +1375,7 @@ static int ip_error(struct sk_buff *skb)
 			break;
 		case ENETUNREACH:
 			code = ICMP_NET_UNREACH;
+			IP_INC_STATS_BH(IPSTATS_MIB_INNOROUTES);
 			break;
 		case EACCES:
 			code = ICMP_PKT_FILTERED;

@@ -2004,6 +2005,7 @@ no_route:
 	RT_CACHE_STAT_INC(in_no_route);
 	spec_dst = inet_select_addr(dev, 0, RT_SCOPE_UNIVERSE);
 	res.type = RTN_UNREACHABLE;
+	err = -ENETUNREACH;
 	goto local_input;
 
 	/*

Signature · 1 line

1.5.3.4

David Miller

maintainer

2007-12-06 06:33 UTC #1 lore raw

From: Mitsuru Chinen <[email protected]>
Date: Thu, 6 Dec 2007 15:31:05 +0900

Quoted 7 lines

IPv4 stack doesn't reply any ICMP destination unreachable message with net unreachable code when IP detagrams are being discarded because of no route could be found in the forwarding path. Incidentally, IPv6 stack replies such ICMPv6 message in the similar situation.
Signed-off-by Mitsuru Chinen <[email protected]>

Also applied, thank you.

Jarek Poplawski

2007-12-06 07:44 UTC #2 lore raw

In reply to Mitsuru Chinen

Quoted 24 lines · Mitsuru Chinen

IPv4 stack doesn't reply any ICMP destination unreachable message with net unreachable code when IP detagrams are being discarded because of no route could be found in the forwarding path. Incidentally, IPv6 stack replies such ICMPv6 message in the similar situation.
Signed-off-by Mitsuru Chinen <[email protected]>
---
 net/ipv4/route.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
net/ipv4/route.c +1 -0
index 6714bbc..ba85ec9 100644
@@ -1375,6 +1375,7 @@ static int ip_error(struct sk_buff *skb)
 			break;
 		case ENETUNREACH:
 			code = ICMP_NET_UNREACH;
+			IP_INC_STATS_BH(IPSTATS_MIB_INNOROUTES);
 			break;
 		case EACCES:
 			code = ICMP_PKT_FILTERED;

net/ipv4/route.c no_route: · line 2011

Quoted 7 lines

net/ipv4/route.c +1 -0

@@ -2004,6 +2005,7 @@ no_route:
 	RT_CACHE_STAT_INC(in_no_route);
 	spec_dst = inet_select_addr(dev, 0, RT_SCOPE_UNIVERSE);
 	res.type = RTN_UNREACHABLE;
+	err = -ENETUNREACH;
 	goto local_input;
 
 	/*

This patch seems to be wrong. It overrides err codes from fib_lookup, where such decisions should be made.

Regards,
Jarek P.

Rami Rosen

2007-12-06 08:12 UTC #3 lore raw

Hello, Jarek,

I am sorry, but I think I am nor sure I underatand exactly what you mean when you say:
"It overrides err codes from fib_lookup, where such decisions should be made."

What is incorrect here ?

There are two lines added in this patch;

IP_INC_STATS_BH(IPSTATS_MIB_INNOROUTES);
and err = -ENETUNREACH;

The first one is, regardless to say, not relevant to err codes.

The second, err = -ENETUNREACH, is from: ip_route_input_slow(). (net/ipv4/route.c).

Assigning values to err is done more than once in this method; for example,
e_hostunreach:

	err = -EHOSTUNREACH;

e_inval:

	err = -EINVAL;

e_nobufs:

	err = -ENOBUFS;

So I don't think anything is incorrect here.

Regards,
Rami Rosen

Quoted 45 lines · Jarek Poplawski

Quoted 24 lines · Mitsuru Chinen
IPv4 stack doesn't reply any ICMP destination unreachable message with net unreachable code when IP detagrams are being discarded because of no route could be found in the forwarding path. Incidentally, IPv6 stack replies such ICMPv6 message in the similar situation.
Signed-off-by Mitsuru Chinen <[email protected]>
---
 net/ipv4/route.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
net/ipv4/route.c +1 -0
index 6714bbc..ba85ec9 100644
@@ -1375,6 +1375,7 @@ static int ip_error(struct sk_buff *skb)
                      break;
              case ENETUNREACH:
                      code = ICMP_NET_UNREACH;
+                     IP_INC_STATS_BH(IPSTATS_MIB_INNOROUTES);
                      break;
              case EACCES:
                      code = ICMP_PKT_FILTERED;
net/ipv4/route.c no_route: · line 2011
Quoted 7 lines
net/ipv4/route.c +1 -0
@@ -2004,6 +2005,7 @@ no_route:
      RT_CACHE_STAT_INC(in_no_route);
      spec_dst = inet_select_addr(dev, 0, RT_SCOPE_UNIVERSE);
      res.type = RTN_UNREACHABLE;
+     err = -ENETUNREACH;
      goto local_input;
 
      /*
This patch seems to be wrong. It overrides err codes from fib_lookup, where such decisions should be made.
Regards,
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Mitsuru Chinen

author

2007-12-06 08:14 UTC #4 lore raw

In reply to Jarek Poplawski

Quoted 31 lines · On Thu, 6 Dec 2007 08:49:47 +0100 Jarek Poplawski <[email protected]> wrote:

Quoted 30 lines · Mitsuru Chinen
IPv4 stack doesn't reply any ICMP destination unreachable message with net unreachable code when IP detagrams are being discarded because of no route could be found in the forwarding path. Incidentally, IPv6 stack replies such ICMPv6 message in the similar situation.
Signed-off-by Mitsuru Chinen <[email protected]>
---
 net/ipv4/route.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
net/ipv4/route.c +2 -0
index 6714bbc..ba85ec9 100644
@@ -1375,6 +1375,7 @@ static int ip_error(struct sk_buff *skb)
 			break;
 		case ENETUNREACH:
 			code = ICMP_NET_UNREACH;
+			IP_INC_STATS_BH(IPSTATS_MIB_INNOROUTES);
 			break;
 		case EACCES:
 			code = ICMP_PKT_FILTERED;
@@ -2004,6 +2005,7 @@ no_route:
 	RT_CACHE_STAT_INC(in_no_route);
 	spec_dst = inet_select_addr(dev, 0, RT_SCOPE_UNIVERSE);
 	res.type = RTN_UNREACHABLE;
+	err = -ENETUNREACH;
 	goto local_input;
 

Quoted 4 lines

net/ipv4/route.c no_route: · line 2011
Quoted 1 line
net/ipv4/route.c +0 -0
 	/*
This patch seems to be wrong. It overrides err codes from fib_lookup, where such decisions should be made.

fib_lookup() replies -ESRCH in this situation.
It is necessary to override the variable by the suitable error number like the code under e_hostunreach label.

Best Regards,
----
Mitsuru Chinen <[email protected]>

Jarek Poplawski

2007-12-06 08:42 UTC #5 lore raw

Quoted 9 lines · Mitsuru Chinen

Quoted 6 lines · On Thu, 6 Dec 2007 08:49:47 +0100 Jarek Poplawski <[email protected]> wrote:
Quoted 5 lines · Mitsuru Chinen
IPv4 stack doesn't reply any ICMP destination unreachable message with net unreachable code when IP detagrams are being discarded because of no route could be found in the forwarding path. Incidentally, IPv6 stack replies such ICMPv6 message in the similar situation.

...

Quoted 6 lines

Quoted 2 lines
This patch seems to be wrong. It overrides err codes from fib_lookup, where such decisions should be made.
fib_lookup() replies -ESRCH in this situation.
It is necessary to override the variable by the suitable error number like the code under e_hostunreach label.

Probably I miss something, but I can't see how can you be sure it's only -ESRCH possible here? Isn't opt->action() in fib_rules_lookup() supposed to return this -ENETUNREACH when needed?

Jarek P.

Mitsuru Chinen

author

2007-12-07 04:24 UTC #6 lore raw

Quoted 14 lines · On Thu, 6 Dec 2007 09:47:33 +0100 Jarek Poplawski <[email protected]> wrote:

Quoted 9 lines · Mitsuru Chinen
Quoted 6 lines · On Thu, 6 Dec 2007 08:49:47 +0100 Jarek Poplawski <[email protected]> wrote:
Quoted 5 lines · Mitsuru Chinen
IPv4 stack doesn't reply any ICMP destination unreachable message with net unreachable code when IP detagrams are being discarded because of no route could be found in the forwarding path. Incidentally, IPv6 stack replies such ICMPv6 message in the similar situation.
...
Quoted 3 lines
Quoted 2 lines
This patch seems to be wrong. It overrides err codes from fib_lookup, where such decisions should be made.

Quoted 7 lines

Quoted 3 lines
fib_lookup() replies -ESRCH in this situation.
It is necessary to override the variable by the suitable error number like the code under e_hostunreach label.
Probably I miss something, but I can't see how can you be sure it's only -ESRCH possible here? Isn't opt->action() in fib_rules_lookup() supposed to return this -ENETUNREACH when needed?

Oh, excuse me. I did mistake.
fib_rules_lookup() replies -ESRCH when no route is found. The case it replies -ENETUNREACH is that user adds unreachable route. However, if the err value is override with no check, a blackhole or prohibit route is treated as a unreachable route.

As the patch is already applied, I will send another patch to add a check for it.
Thank you very much for pointing out the issue!

Best Regards,
----
Mitsuru Chinen <[email protected]>