From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexey Kodanev <alexey.kodanev@oracle.com>
Subject: Re: [PATCH net] vxlan: Restore initial MTU setting based on lower
 device
Date: Thu, 14 Dec 2017 19:26:20 +0300
Message-ID: <c60b66a0-0fa1-f298-9192-ef90954e292f@oracle.com>
References: <0c6caef03156aa51673c50ebb59889fc001b74be.1513198761.git.sbrivio@redhat.com>
 <f7ce892a-60e7-4dfd-31d4-ee8fa06c9c1f@universe-factory.net>
 <20171214011042.6b4a2e8b@elisabeth>
 <42862507-a573-af7a-d4aa-fd8cdd89c01e@universe-factory.net>
 <20171214013148.3da52cc9@elisabeth>
 <b7ab9d2f-3916-221b-7763-c1445fa65e2c@oracle.com>
 <20171214133647.1c1b82d1@elisabeth>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: Matthias Schiffer <mschiffer@universe-factory.net>,
        "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: Stefano Brivio <sbrivio@redhat.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from userp2120.oracle.com ([156.151.31.85]:51276 "EHLO
        userp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753090AbdLNQ1a (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 14 Dec 2017 11:27:30 -0500
In-Reply-To: <20171214133647.1c1b82d1@elisabeth>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 12/14/2017 03:36 PM, Stefano Brivio wrote:
> On Thu, 14 Dec 2017 14:23:36 +0300
> Alexey Kodanev <alexey.kodanev@oracle.com> wrote:
> 
>> On 12/14/2017 03:31 AM, Stefano Brivio wrote:
...
>>
>> if we move it up in "if (lowerdev) { ..." branch we will be checking the presence
>> of "lowerdev" and also not calculating it again. Also I would check max_mtu for
>> minimum as it might happen to be negative, though unlikely corner case...
> 
> Indeed it might happen to be negative (only for IPv6, down to -2), good
> catch.
> 
> For the benefit of others: it took me a few minutes to see how this is
> *not* unrelated, because we are introducing a direct assignment of
> dev->mtu to set max_mtu, whereas earlier it was just used in
> comparisons, so it didn't matter whether it was negative.
> 
>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>> index 19b9cc5..1000b0e 100644
>> --- a/drivers/net/vxlan.c
>> +++ b/drivers/net/vxlan.c
>> @@ -3103,6 +3103,11 @@ static void vxlan_config_apply(struct net_device *dev,
>>
>>                 max_mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM :
>>                                            VXLAN_HEADROOM);
>> +               if (max_mtu < ETH_MIN_MTU)
>> +                       max_mtu = ETH_MIN_MTU;
>> +
>> +               if (!changelink && !conf->mtu)
>> +                       dev->mtu = max_mtu;
> 
> I don't really have a strong preference here. On one hand, you're
> hiding this a bit from the "device creation" path. On the other hand,
> it's a bit more compact. So I'm also fine with this.
> 
> Can you perhaps submit a formal patch?
> 

OK, I'll send a patch.

Thanks,
Alexey