On 12/14/2017 03:36 PM, Stefano Brivio wrote:
> On Thu, 14 Dec 2017 14:23:36 +0300
> Alexey Kodanev <alexey.koda...@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
