From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <netdev-owner@vger.kernel.org>
Received: from mail-pl0-f48.google.com ([209.85.160.48]:44438 "EHLO
        mail-pl0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753978AbeCGEqy (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 6 Mar 2018 23:46:54 -0500
Received: by mail-pl0-f48.google.com with SMTP id 9-v6so684246ple.11
        for <netdev@vger.kernel.org>; Tue, 06 Mar 2018 20:46:54 -0800 (PST)
Subject: Re: [PATCH net v2 RESEND] ipv6: Reflect MTU changes on PMTU of
 exceptions for MTU-less routes
To: Stefano Brivio <sbrivio@redhat.com>,
        "David S . Miller" <davem@davemloft.net>
Cc: Wei Wang <weiwan@google.com>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        =?UTF-8?Q?Maciej_=c5=bbenczykowski?= <maze@google.com>,
        Xiumei Mu <xmu@redhat.com>, netdev@vger.kernel.org
References: <20180306101019.74424-1-sbrivio@redhat.com>
From: David Ahern <dsahern@gmail.com>
Message-ID: <8418ad7a-170e-b2c9-5a9b-28add4b678db@gmail.com>
Date: Tue, 6 Mar 2018 21:46:51 -0700
MIME-Version: 1.0
In-Reply-To: <20180306101019.74424-1-sbrivio@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 3/6/18 3:10 AM, Stefano Brivio wrote:
> Currently, administrative MTU changes on a given netdevice are
> not reflected on route exceptions for MTU-less routes, with a
> set PMTU value, for that device:
> 
>  # ip -6 route get 2001:db8::b
>  2001:db8::b from :: dev vti_a proto kernel src 2001:db8::a metric 256 pref medium
>  # ping6 -c 1 -q -s10000 2001:db8::b > /dev/null
>  # ip netns exec a ip -6 route get 2001:db8::b
>  2001:db8::b from :: dev vti_a src 2001:db8::a metric 0
>      cache expires 571sec mtu 4926 pref medium
>  # ip link set dev vti_a mtu 3000
>  # ip -6 route get 2001:db8::b
>  2001:db8::b from :: dev vti_a src 2001:db8::a metric 0
>      cache expires 571sec mtu 4926 pref medium
>  # ip link set dev vti_a mtu 9000
>  # ip -6 route get 2001:db8::b
>  2001:db8::b from :: dev vti_a src 2001:db8::a metric 0
>      cache expires 571sec mtu 4926 pref medium
> 
> The first issue is that since commit fb56be83e43d ("net-ipv6: on
> device mtu change do not add mtu to mtu-less routes") we don't
> call rt6_exceptions_update_pmtu() from rt6_mtu_change_route(),
> which handles administrative MTU changes, if the regular route
> is MTU-less.
> 
> However, PMTU exceptions should be always updated, as long as
> RTAX_MTU is not locked. Keep the check for MTU-less main route,
> as introduced by that commit, but, for exceptions,
> call rt6_exceptions_update_pmtu() regardless of that check.
> 
> Once that is fixed, one problem remains: MTU changes are not
> reflected if the new MTU is higher than the previous one,
> because rt6_exceptions_update_pmtu() doesn't allow that. We
> should instead allow PMTU increase if the old PMTU matches the
> local MTU, as that implies that the old MTU was the lowest in the
> path, and PMTU discovery might lead to different results.
> 
> The existing check in rt6_mtu_change_route() correctly took that
> case into account (for regular routes only), so factor it out
> and re-use it also in rt6_exceptions_update_pmtu().
> 
> While at it, fix comments style and grammar, and try to be a bit
> more descriptive.
> 
> Reported-by: Xiumei Mu <xmu@redhat.com>
> Fixes: fb56be83e43d ("net-ipv6: on device mtu change do not add mtu to mtu-less routes")
> Fixes: f5bbe7ee79c2 ("ipv6: prepare rt6_mtu_change() for exception table")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---

Acked-by: David Ahern <dsahern@gmail.com>