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 +0 -0

index 844a9ef..6e4a89c 100644

net/ipv4/route.c +0 -0

 	mtu = dst->dev->mtu;

Signature · 1 line

1.7.9.5

This fix looks good to me. But I see that we
have another problem here. doc/ip-cref.tex in iproute2 claims that a locked MTU value has priority and PMTU should not be considered.

IIRC, rt_pmtu is valid only for output routes but
we do not check the lock flag here. What about such variant:

static unsigned int ipv4_mtu(const struct dst_entry *dst)

{
	const struct rtable *rt = (const struct rtable *) dst;
	unsigned int mtu = rt->rt_pmtu;

	if (unlikely(dst_metric_locked(dst, RTAX_MTU))) {
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (!mtu) {
			mtu = dst->dev->mtu;
			if (rt->rt_uses_gateway && mtu > 576)
				mtu = 576;
		}
	} else if (!mtu || time_after_eq(jiffies, rt->dst.expires)) {
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (!mtu)
			mtu = dst->dev->mtu;
	}
	if (mtu > IP_MAX_MTU)
		mtu = IP_MAX_MTU;

	return mtu;
}

I.e. order becomes:

mtu lock non-zero => fixed fib_mtu
mtu lock 0 => device MTU, up to 576 if via GW, ignore PMTU PMTU (output routes)
mtu non-zero => fib_mtu
device MTU

Also, it seems __ip_rt_update_pmtu should start
with such dst_metric_locked(dst, RTAX_MTU) check?

Quoted 33 lines · Julian Anastasov

This fix looks good to me. But I see that we
have another problem here. doc/ip-cref.tex in iproute2 claims that a locked MTU value has priority and PMTU should not be considered.

IIRC, rt_pmtu is valid only for output routes but
we do not check the lock flag here. What about such variant:

static unsigned int ipv4_mtu(const struct dst_entry *dst)

{
	const struct rtable *rt = (const struct rtable *) dst;
	unsigned int mtu = rt->rt_pmtu;

	if (unlikely(dst_metric_locked(dst, RTAX_MTU))) {
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (!mtu) {
			mtu = dst->dev->mtu;
			if (rt->rt_uses_gateway && mtu > 576)
				mtu = 576;
		}
	} else if (!mtu || time_after_eq(jiffies, rt->dst.expires)) {
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (!mtu)
			mtu = dst->dev->mtu;
	}
	if (mtu > IP_MAX_MTU)
		mtu = IP_MAX_MTU;

	return mtu;
}

Quoted 7 lines

I.e. order becomes:

mtu lock non-zero => fixed fib_mtu
mtu lock 0 => device MTU, up to 576 if via GW, ignore PMTU PMTU (output routes)
mtu non-zero => fib_mtu
device MTU

If this is the desired order, I'm fine with the above variant.

Quoted 4 lines

Also, it seems __ip_rt_update_pmtu should start
with such dst_metric_locked(dst, RTAX_MTU) check?

Not absolutely sure what you want to do if the mtu is locked. But if you don't want to genetate a nh exception and don't update rt->rt_pmtu in this case, rt->rt_pmtu is never set on routes with locked mtu. We probably would not need to change ipv4_mtu() to the above variant.

Quoted 6 lines · Steffen Klassert

Not absolutely sure what you want to do if the mtu is locked. But if you don't want to genetate a nh exception and don't update rt->rt_pmtu in this case, rt->rt_pmtu is never set on routes with locked mtu. We probably would not need to change ipv4_mtu() to the above variant.

Yes, we have to exit __ip_rt_update_pmtu if
fib_mtu is locked. Then rt_pmtu should stay 0 and your variant of ipv4_mtu should be ok. I hope you can simply extend your patch with such additional check in __ip_rt_update_pmtu:

	if (dst_metric_locked(dst, RTAX_MTU))
		return;