From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Brivio <sbrivio@redhat.com>
Subject: Re: [PATCH net] vxlan: Restore initial MTU setting based on lower
 device
Date: Thu, 14 Dec 2017 01:10:42 +0100
Message-ID: <20171214011042.6b4a2e8b@elisabeth>
References: <0c6caef03156aa51673c50ebb59889fc001b74be.1513198761.git.sbrivio@redhat.com>
        <f7ce892a-60e7-4dfd-31d4-ee8fa06c9c1f@universe-factory.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: "David S . Miller" <davem@davemloft.net>, netdev@vger.kernel.org,
        Junhan Yan <juyan@redhat.com>, Jiri Benc <jbenc@redhat.com>,
        Hangbin Liu <haliu@redhat.com>
To: Matthias Schiffer <mschiffer@universe-factory.net>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:35030 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751839AbdLNAKx (ORCPT <rfc822;netdev@vger.kernel.org>);
        Wed, 13 Dec 2017 19:10:53 -0500
In-Reply-To: <f7ce892a-60e7-4dfd-31d4-ee8fa06c9c1f@universe-factory.net>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Thu, 14 Dec 2017 00:57:32 +0100
Matthias Schiffer <mschiffer@universe-factory.net> wrote:

> As you note, there is another occurrence of this calculation in
> vxlan_config_apply():
> 
> 
> [...]
>         if (lowerdev) {
> [...]
>                 max_mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM :
>                                            VXLAN_HEADROOM);
>         }
> 
>         if (dev->mtu > max_mtu)
>                 dev->mtu = max_mtu;
> [...]
> 
> 
> Unless I'm overlooking something, this should already do the same thing and
> your patch is redundant.

The code above sets max_mtu, and only if dev->mtu exceeds that, the
latter is then clamped.

What my patch does is to actually set dev->mtu to that value, no matter
what's the previous value set by ether_setup() (only on creation, and
only if lowerdev is there), just like the previous behaviour used to be.

Let's consider these two cases, on the existing code:

1. lowerdev->mtu is 1500:
   - ether_setup(), called by vxlan_setup(), sets dev->mtu to 1500
   - here max_mtu is 1450
   - we enter the second if clause above (dev->mtu > max_mtu)
   - at the end of vxlan_config_apply(), dev->mtu will be 1450

which is consistent with the previous behaviour.

2. lowerdev->mtu is 9000:
   - ether_setup(), called by vxlan_setup(), sets dev->mtu to 1500
   - here max_mtu is 8950
   - we do not enter the second if clause above (dev->mtu < max_mtu)
   - at the end of vxlan_config_apply(), dev->mtu will still be 1500

which is not consistent with the previous behaviour, where it used to
be 8950 instead.

-- 
Stefano