[RFC PATCH] net: blackhole route should always be recalculated

[RFC PATCH] net: blackhole route should always be recalculated

[Nicolas Dichtel]

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

Hi all,

I got a problem with IKE when the first packet is dropped, kernel does not 
invalidate the routing cache for next packets.

It seems to come from commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92, which 
check validy of routes. With this patch, blackhole routes are not recalculated 
(when route table is not updated).
But this kind of routes are used when xfrm_lookup() returns -EREMOTE, so it 
seems logical to check the route again for next packets, and then get the right 
route.

Maybe my approach is wrong, any comments are welcome.


Regards,
Nicolas

[-- Attachment #2: 0001-net-blackhole-route-should-always-be-recalculated.patch --]
[-- Type: text/x-diff, Size: 1446 bytes --]

>From 1683b838a42429af30d6ab76d2d15d267c93c455 Mon Sep 17 00:00:00 2001
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Fri, 27 Aug 2010 17:22:17 +0200
Subject: [PATCH] net: blackhole route should always be recalculated

Blackhole routes are used when xfrm_lookup() returns -EREMOTE (error
triggered by IKE for example), hence this kind of route is always
temporary and so we should check if a better route exists for next
packets.
Bug has been introduced by commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92.

Signed-off-by: Jianzhao Wang <jianzhao.wang@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv4/route.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 3f56b6e..6298f75 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2738,6 +2738,11 @@ slow_output:
 }
 EXPORT_SYMBOL_GPL(__ip_route_output_key);
 
+static struct dst_entry *ipv4_blackhole_dst_check(struct dst_entry *dst, u32 cookie)
+{
+	return NULL;
+}
+
 static void ipv4_rt_blackhole_update_pmtu(struct dst_entry *dst, u32 mtu)
 {
 }
@@ -2746,7 +2751,7 @@ static struct dst_ops ipv4_dst_blackhole_ops = {
 	.family			=	AF_INET,
 	.protocol		=	cpu_to_be16(ETH_P_IP),
 	.destroy		=	ipv4_dst_destroy,
-	.check			=	ipv4_dst_check,
+	.check			=	ipv4_blackhole_dst_check,
 	.update_pmtu		=	ipv4_rt_blackhole_update_pmtu,
 	.entries		=	ATOMIC_INIT(0),
 };
-- 
1.5.4.5

Re: [RFC PATCH] net: blackhole route should always be recalculated

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Fri, 27 Aug 2010 17:47:40 +0200

> It seems to come from commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92,
> which check validy of routes. With this patch, blackhole routes are
> not recalculated (when route table is not updated).
> But this kind of routes are used when xfrm_lookup() returns -EREMOTE,
> so it seems logical to check the route again for next packets, and
> then get the right route.

Thanks Nicolas.  At first glance I think you're approach is correct,
but I will investigate things more closely to make certain of this.

Blackhole routes never had a check operation, because before the XFRM
stuff they were never expected to revalidate in the future.  But with
XFRM now they can, so we likely need to force revalidation now as your
patch does.

Re: [RFC PATCH] net: blackhole route should always be recalculated

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Fri, 27 Aug 2010 17:47:40 +0200

>>From 1683b838a42429af30d6ab76d2d15d267c93c455 Mon Sep 17 00:00:00 2001
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Fri, 27 Aug 2010 17:22:17 +0200
> Subject: [PATCH] net: blackhole route should always be recalculated
> 
> Blackhole routes are used when xfrm_lookup() returns -EREMOTE (error
> triggered by IKE for example), hence this kind of route is always
> temporary and so we should check if a better route exists for next
> packets.
> Bug has been introduced by commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92.
> 
> Signed-off-by: Jianzhao Wang <jianzhao.wang@6wind.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Applied and queued up for -stable, thanks!