On 12/14/2017 01:10 AM, Stefano Brivio wrote:
> On Thu, 14 Dec 2017 00:57:32 +0100
> Matthias Schiffer <mschif...@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.Ah, thank you for the explanation, I was missing the context that this was about higher rather than lower MTUs. Personally, I would prefer a change like the following, as it does not introduce another duplication of the MTU calculation (not tested at all): > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -3105,7 +3105,7 @@ static void vxlan_config_apply(struct net_device *dev, > VXLAN_HEADROOM); > } > > - if (dev->mtu > max_mtu) > + if (dev->mtu > max_mtu || (!changelink && !conf->mtu)) > dev->mtu = max_mtu; > > if (use_ipv6 || conf->flags & VXLAN_F_COLLECT_METADATA) Regards, Matthias
signature.asc
Description: OpenPGP digital signature
