Skip to contentSkip to conversation

ipv4: Remove output route check in ipv4_mtu

PATCH In review Steffen Klassert 4 participants 6 messages Jan 17, 2013

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Steffen Klassert

author
#0 lore raw

PATCH 1/2 ipv4: Remove output route check in ipv4_mtu

The output route check was introduced with git commit 261663b0 (ipv4: Don't use the cached pmtu informations for input routes) during times when we cached the pmtu informations on the inetpeer. Now the pmtu informations are back in the routes, so this check is obsolete. It also had some unwanted side effects, as reported by Timo Teras and Lukas Tribus.

Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv4/route.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv4/route.c +1 -1
index 844a9ef..6e4a89c 100644
@@ -1120,7 +1120,7 @@ static unsigned int ipv4_mtu(const struct dst_entry *dst)
 	if (!mtu || time_after_eq(jiffies, rt->dst.expires))
 		mtu = dst_metric_raw(dst, RTAX_MTU);
 
-	if (mtu && rt_is_output_route(rt))
+	if (mtu)
 		return mtu;
 
 	mtu = dst->dev->mtu;
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 2/2 ipv4: Don't update the pmtu on mtu locked routes

Routes with locked mtu should not use learned pmtu informations, so do not update the pmtu on these routes.

Reported-by Julian Anastasov <[email protected]>
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv4/route.c |    3 +++
 1 file changed, 3 insertions(+)
net/ipv4/route.c +3 -0
index 6e4a89c..259cbee 100644
@@ -912,6 +912,9 @@ static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
 	struct dst_entry *dst = &rt->dst;
 	struct fib_result res;
 
+	if (dst_metric_locked(dst, RTAX_MTU))
+		return;
+
 	if (dst->dev->mtu < mtu)
 		return;
Signature · 1 line
1.7.9.5
#2 lore raw

Re: [PATCH 2/2] ipv4: Don't update the pmtu on mtu locked routes

Hello,

Quoted 25 lines · Steffen Klassert

Routes with locked mtu should not use learned pmtu informations, so do not update the pmtu on these routes.

Reported-by Julian Anastasov <[email protected]>
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv4/route.c |    3 +++
 1 file changed, 3 insertions(+)
net/ipv4/route.c +3 -0
index 6e4a89c..259cbee 100644
@@ -912,6 +912,9 @@ static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
 	struct dst_entry *dst = &rt->dst;
 	struct fib_result res;
 
+	if (dst_metric_locked(dst, RTAX_MTU))
+		return;
+
 	if (dst->dev->mtu < mtu)
 		return;
Signature · 1 line
1.7.9.5

Thanks

--
Julian Anastasov <[email protected]>

Acked-by on v1 1/2
#4 lore raw
In reply to Steffen Klassert
Quoted 9 lines · On Thu, 17 Jan 2013 07:55:01 +0100 Steffen Klassert…

The output route check was introduced with git commit 261663b0 (ipv4: Don't use the cached pmtu informations for input routes) during times when we cached the pmtu informations on the inetpeer. Now the pmtu informations are back in the routes, so this check is obsolete. It also had some unwanted side effects, as reported by Timo Teras and Lukas Tribus.

Signed-off-by Steffen Klassert <[email protected]>

---

Acked-by Timo Teräs <[email protected]>

I think this should also go to stable queues which are applicable, since the original commit caused CLAMPMSS and XFRM pmtu regressions.

The original thread of this is:
http://marc.info/?t=134208398500001&r=1&w=2

My original revert request is at:
http://marc.info/?l=linux-netdev&m=134242561624266&w=2

and additional reasoning at:
http://marc.info/?l=linux-netdev&m=134243626627716&w=2

Thanks,
Timo

maintainer
#5 lore raw
In reply to Steffen Klassert

From: Steffen Klassert <[email protected]>
Date: Thu, 17 Jan 2013 07:55:01 +0100

Quoted 8 lines

The output route check was introduced with git commit 261663b0 (ipv4: Don't use the cached pmtu informations for input routes) during times when we cached the pmtu informations on the inetpeer. Now the pmtu informations are back in the routes, so this check is obsolete. It also had some unwanted side effects, as reported by Timo Teras and Lukas Tribus.

Signed-off-by Steffen Klassert <[email protected]>

Applied and queued up for -stable.