diff mbox

[RFC,net-next,08/10] ipv6: Do not use inetpeer when creating RTF_CACHE route for /128 via gateway entry

Message ID 1428717576-1040383-9-git-send-email-kafai@fb.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Martin KaFai Lau April 11, 2015, 1:59 a.m. UTC 
When there is a pmtu exception on /128 via gateway route, we need to
create a separate metrics copy for the newly created RTF_CACHE route instead
of reusing the inetpeer cache.  Otherwise, the original mtu will be
over-written and the mtu update will stay after the expiration.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/route.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

Steffen Klassert April 13, 2015, 11:06 a.m. UTC | #1 
On Fri, Apr 10, 2015 at 06:59:34PM -0700, Martin KaFai Lau wrote:
> When there is a pmtu exception on /128 via gateway route, we need to
> create a separate metrics copy for the newly created RTF_CACHE route instead
> of reusing the inetpeer cache.

Maybe we should remove the caching of the metrics on the inetpeer
completely. After your patchset only static hostroutes using this,
and this is exactly the case where it is buggy. If a second route
to the same host is added, the metrics of the first will be
overwritten.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Martin KaFai Lau April 13, 2015, 5:51 p.m. UTC | #2 
On Mon, Apr 13, 2015 at 01:06:32PM +0200, Steffen Klassert wrote:
> On Fri, Apr 10, 2015 at 06:59:34PM -0700, Martin KaFai Lau wrote:
> > When there is a pmtu exception on /128 via gateway route, we need to
> > create a separate metrics copy for the newly created RTF_CACHE route instead
> > of reusing the inetpeer cache.
>
> Maybe we should remove the caching of the metrics on the inetpeer
> completely. After your patchset only static hostroutes using this,
The RTF_CACHE copied from "plen <128 via gateway" route will also use
the inetpeer.

> and this is exactly the case where it is buggy. If a second route
> to the same host is added, the metrics of the first will be
> overwritten.
I agree.  The current upstream also has similar bug.
I had thought about changes as you suggested but decided to
use a separate patch instead.  I will try to consider it in v2.

Thanks,
--Martin
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 91c80bc..61ce45e 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -322,8 +322,16 @@  static void ip6_dst_destroy(struct dst_entry *dst)
 	struct rt6_info *rt = (struct rt6_info *)dst;
 	struct inet6_dev *idev = rt->rt6i_idev;
 	struct dst_entry *from = dst->from;
+	unsigned long peer_metrics = 0;
 
-	if (!(rt->dst.flags & DST_HOST))
+	if (rt6_has_peer(rt)) {
+		struct inet_peer *peer = rt6_peer_ptr(rt);
+
+		peer_metrics = (unsigned long)peer->metrics;
+		inet_putpeer(peer);
+	}
+
+	if (peer_metrics != dst->_metrics)
 		dst_destroy_metrics_generic(dst);
 
 	if (idev) {
@@ -333,11 +341,6 @@  static void ip6_dst_destroy(struct dst_entry *dst)
 
 	dst->from = NULL;
 	dst_release(from);
-
-	if (rt6_has_peer(rt)) {
-		struct inet_peer *peer = rt6_peer_ptr(rt);
-		inet_putpeer(peer);
-	}
 }
 
 static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
@@ -1956,6 +1959,8 @@  static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 
 		rt->rt6i_dst.addr = *dest;
 		rt->rt6i_dst.plen = 128;
+		if (ort->dst.flags & DST_HOST)
+			dst_cow_metrics_generic(&rt->dst, rt->dst._metrics);
 		dst_copy_metrics(&rt->dst, &ort->dst);
 		rt->dst.error = ort->dst.error;
 		rt->rt6i_idev = ort->rt6i_idev;