[PATCH v3] net: Fix behaviour of unreachable, blackhole and prohibit routes

Subsystems: networking [general], networking [ipv4/ipv6], the rest

STALE4070d

3 messages, 3 authors, 2015-09-03 · open the first message on its own page

[PATCH v3] net: Fix behaviour of unreachable, blackhole and prohibit routes

From: Nikola Forró <hidden>
Date: 2015-09-03 10:05:25

Man page of ip-route(8) says following about route types:

  unreachable - these destinations are unreachable.  Packets are dis‐
  carded and the ICMP message host unreachable is generated.  The local
  senders get an EHOSTUNREACH error.

  blackhole - these destinations are unreachable.  Packets are dis‐
  carded silently.  The local senders get an EINVAL error.

  prohibit - these destinations are unreachable.  Packets are discarded
  and the ICMP message communication administratively prohibited is
  generated.  The local senders get an EACCES error.

In the inet6 address family, this was correct, except the local senders
got ENETUNREACH error instead of EHOSTUNREACH in case of unreachable route.
In the inet address family, all three route types generated ICMP message
net unreachable, and the local senders got ENETUNREACH error.

In both address families all three route types now behave consistently
with documentation.

Signed-off-by: Nikola Forró <redacted>
---
 include/net/ip_fib.h | 22 +++++++++++++++++-----
 net/ipv4/route.c     |  6 ++++--
 net/ipv6/route.c     |  4 +++-
 3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 5fa643b..8e7b3e1 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -233,8 +233,11 @@ static inline int fib_lookup(struct net *net, const struct flowi4 *flp,
 	rcu_read_lock();
 
 	tb = fib_get_table(net, RT_TABLE_MAIN);
-	if (tb && !fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF))
-		err = 0;
+	if (tb) {
+		err = fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF);
+		if (err == -EAGAIN)
+			err = -ENETUNREACH;
+	}
 
 	rcu_read_unlock();
 
@@ -267,11 +270,20 @@ static inline int fib_lookup(struct net *net, struct flowi4 *flp,
 
 	for (err = 0; !err; err = -ENETUNREACH) {
 		tb = rcu_dereference_rtnl(net->ipv4.fib_main);
-		if (tb && !fib_table_lookup(tb, flp, res, flags))
-			break;
+		if (tb) {
+			err = fib_table_lookup(tb, flp, res, flags);
+			if (!err)
+				break;
+		}
 
 		tb = rcu_dereference_rtnl(net->ipv4.fib_default);
-		if (tb && !fib_table_lookup(tb, flp, res, flags))
+		if (tb) {
+			err = fib_table_lookup(tb, flp, res, flags);
+			if (!err)
+				break;
+		}
+
+		if (err && err != -EAGAIN)
 			break;
 	}
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e681b85..4ce3f87 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2020,6 +2020,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
 	struct fib_result res;
 	struct rtable *rth;
 	int orig_oif;
+	int err = ENETUNREACH;
 
 	res.tclassid	= 0;
 	res.fi		= NULL;
@@ -2123,7 +2124,8 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
 		goto make_route;
 	}
 
-	if (fib_lookup(net, fl4, &res, 0)) {
+	err = fib_lookup(net, fl4, &res, 0);
+	if (err) {
 		res.fi = NULL;
 		res.table = NULL;
 		if (fl4->flowi4_oif) {
@@ -2151,7 +2153,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
 			res.type = RTN_UNICAST;
 			goto make_route;
 		}
-		rth = ERR_PTR(-ENETUNREACH);
+		rth = ERR_PTR(err);
 		goto out;
 	}
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d155864..d33a6a5 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1847,9 +1847,11 @@ int ip6_route_add(struct fib6_config *cfg)
 			rt->dst.input = ip6_pkt_prohibit;
 			break;
 		case RTN_THROW:
+		case RTN_UNREACHABLE:
 		default:
 			rt->dst.error = (cfg->fc_type == RTN_THROW) ? -EAGAIN
-					: -ENETUNREACH;
+					: (cfg->fc_type == RTN_UNREACHABLE)
+					? -EHOSTUNREACH : -ENETUNREACH;
 			rt->dst.output = ip6_pkt_discard_out;
 			rt->dst.input = ip6_pkt_discard;
 			break;
-- 
2.4.3

Re: [PATCH v3] net: Fix behaviour of unreachable, blackhole and prohibit routes

From: Alexander Duyck <hidden>
Date: 2015-09-03 15:14:55

On 09/03/2015 03:05 AM, Nikola Forró wrote:
quoted hunk
Man page of ip-route(8) says following about route types:

   unreachable - these destinations are unreachable.  Packets are dis‐
   carded and the ICMP message host unreachable is generated.  The local
   senders get an EHOSTUNREACH error.

   blackhole - these destinations are unreachable.  Packets are dis‐
   carded silently.  The local senders get an EINVAL error.

   prohibit - these destinations are unreachable.  Packets are discarded
   and the ICMP message communication administratively prohibited is
   generated.  The local senders get an EACCES error.

In the inet6 address family, this was correct, except the local senders
got ENETUNREACH error instead of EHOSTUNREACH in case of unreachable route.
In the inet address family, all three route types generated ICMP message
net unreachable, and the local senders got ENETUNREACH error.

In both address families all three route types now behave consistently
with documentation.

Signed-off-by: Nikola Forró <redacted>
---
  include/net/ip_fib.h | 22 +++++++++++++++++-----
  net/ipv4/route.c     |  6 ++++--
  net/ipv6/route.c     |  4 +++-
  3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 5fa643b..8e7b3e1 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -233,8 +233,11 @@ static inline int fib_lookup(struct net *net, const struct flowi4 *flp,
  	rcu_read_lock();
  
  	tb = fib_get_table(net, RT_TABLE_MAIN);
-	if (tb && !fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF))
-		err = 0;
+	if (tb) {
+		err = fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF);

6 more quoted lines

The likelihood of tb being NULL is next to 0%.  You would probably be better off pulling out the EAGAIN check from the if statement and just placing it before the rcu_read_unlock with the correct indentation.
quoted hunk
@@ -267,11 +270,20 @@ static inline int fib_lookup(struct net *net, struct flowi4 *flp,
  
  	for (err = 0; !err; err = -ENETUNREACH) {
  		tb = rcu_dereference_rtnl(net->ipv4.fib_main);
-		if (tb && !fib_table_lookup(tb, flp, res, flags))
-			break;
+		if (tb) {
+			err = fib_table_lookup(tb, flp, res, flags);
+			if (!err)
+				break;
+		}
  
  		tb = rcu_dereference_rtnl(net->ipv4.fib_default);
-		if (tb && !fib_table_lookup(tb, flp, res, flags))
+		if (tb) {
+			err = fib_table_lookup(tb, flp, res, flags);
+			if (!err)
+				break;
+		}
+
+		if (err && err != -EAGAIN)
  			break;
  	}
  
The loop stuff can just be dropped.  This code is now getting a bit too tangled up to justify it.  Probably just use some goto labels instead.  I would probably just initialize err to -ENETUNREACH and drop the check for err at the end and just handle the -EAGAIN case.

I would also probably just pull the -EAGAIN check and place it at the end before the unlock and after whatever label it is you add. No point in optimizing for unlikely cases.
quoted hunk
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e681b85..4ce3f87 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2020,6 +2020,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
  	struct fib_result res;
  	struct rtable *rth;
  	int orig_oif;
+	int err = ENETUNREACH;
  
  	res.tclassid	= 0;
  	res.fi		= NULL;
@@ -2123,7 +2124,8 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
  		goto make_route;
  	}
  
-	if (fib_lookup(net, fl4, &res, 0)) {
+	err = fib_lookup(net, fl4, &res, 0);
+	if (err) {
  		res.fi = NULL;
  		res.table = NULL;
  		if (fl4->flowi4_oif) {
@@ -2151,7 +2153,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
  			res.type = RTN_UNICAST;
  			goto make_route;
  		}
-		rth = ERR_PTR(-ENETUNREACH);
+		rth = ERR_PTR(err);
  		goto out;
  	}
  
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d155864..d33a6a5 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1847,9 +1847,11 @@ int ip6_route_add(struct fib6_config *cfg)
  			rt->dst.input = ip6_pkt_prohibit;
  			break;
  		case RTN_THROW:
+		case RTN_UNREACHABLE:

8 more quoted lines

Re: [PATCH v3] net: Fix behaviour of unreachable, blackhole and prohibit routes

From: David Miller <davem@davemloft.net>
Date: 2015-09-03 17:31:10

From: Nikola Forró <redacted>
Date: Thu, 03 Sep 2015 12:05:22 +0200
quoted hunk
@@ -233,8 +233,11 @@ static inline int fib_lookup(struct net *net, const struct flowi4 *flp,
 	rcu_read_lock();
 
 	tb = fib_get_table(net, RT_TABLE_MAIN);
-	if (tb && !fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF))
-		err = 0;
+	if (tb) {
+		err = fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF);
+		if (err == -EAGAIN)
+			err = -ENETUNREACH;
+	}
 
You resubmitted this so quickly, there is no way you did any testing
of this code path even after fixing up the curly brace issue.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help