Re: [PATCH net] ipv6: Reflect MTU changes on PMTU of exceptions for MTU-less routes

[Stefano Brivio <sbrivio@redhat.com>]

Hi Maciej,

On Fri, 2 Mar 2018 10:54:36 -0800
Maciej Żenczykowski <zenczykowski@gmail.com> wrote:

> I spend a significant fraction of my time making sure we never rely on PMTUD.

Thanks for your comments.

I see your point, but here we are not blindly relying on PMTUD,
rather reflecting an MTU administrative change on the PMTU, and making
the behaviour consistent between regular routes and exceptions, which
is nothing else than a bug fix.

This behaviour reflects RFC 8201, par. 3:

	The basic idea is that a source node initially assumes that
	the PMTU of a path is the (known) MTU of the first hop in the path.

and the need for it is clearly explained by the existing comment in
rt6_mtu_change_route():

	/* For administrative MTU increase, there is no way to discover
	   IPv6 PMTU increase, so PMTU increase should be updated here.
	   Since RFC 1981 doesn't include administrative MTU increase
	   update PMTU increase is a MUST. (i.e. jumbo frame)
	 */

Letting that aside for a moment, a PMTU increase due to my fix is only
possible if the old local MTU (administratively set) was the lowest in
the path, no PMTUD happened meanwhile (but we have an exception route
in place e.g. due to a tunnel calling skb_dst_update_mtu()), and we get
a subsequent administrative change of the local MTU.

Relying on some old value set by the user is simply a bug, and breaks
the natural user assumption that increasing the MTU will have an
effect, if PMTU is not otherwise constrained.

If PMTUD is not working, we will rely on the MTU values set by the
user. This looks like the only sane thing to do.

> Debugging MTU related blackholes is a constant bane of my existence.
> 
> [btw. we're considering adding a hack to always fragment UDP to
> min(1280, dev/route/path mtu)...]
> 
> Basically: lower is always better because it's more likely to work...

This is not directly related to my fix, but I wonder if we shouldn't,
in general, simply comply with RFCs, and provide ways out in case the
network is broken, instead of breaking expected behaviours by default,
or making things work "by mistake". The way out, here, is as simple as
setting 1280 as MTU for the local interface.

Somebody might say higher is better because you avoid fragmentation. So
I would just keep the implementation compliant (and, perhaps more
importantly, consistent).

-- 
Stefano