| Message ID | 0c6caef03156aa51673c50ebb59889fc001b74be.1513198761.git.sbrivio@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] vxlan: Restore initial MTU setting based on lower device | expand |
On Wed, 13 Dec 2017 23:37:00 +0100 Stefano Brivio <sbrivio@redhat.com> wrote: > Commit a985343ba906 ("vxlan: refactor verification and > application of configuration") introduced a change in the > behaviour of initial MTU setting: earlier, the MTU for a link > created on top of a given lower device, without an initial MTU > specification, was set to the MTU of the lower device minus > headroom as a result of this path in vxlan_dev_configure(): > > if (!conf->mtu) > dev->mtu = lowerdev->mtu - > (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM); > > which is now gone. Now, the initial MTU, in absence of a > configured value, is simply set by ether_setup() to ETH_DATA_LEN > (1500 bytes). > > This breaks userspace expectations in case the MTU of > the lower device is higher than 1500 bytes minus headroom. > > Restore the previous behaviour by calculating, for a new link, > the MTU from the lower device, if present, and if no value is > explicitly configured. > > Reported-by: Junhan Yan <juyan@redhat.com> > Fixes: a985343ba906 ("vxlan: refactor verification and application of configuration") > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > --- Good catch. Acked-by: Stephen Hemminger <stephen@networkplumber.org>
On 12/13/2017 11:37 PM, Stefano Brivio wrote: > Commit a985343ba906 ("vxlan: refactor verification and > application of configuration") introduced a change in the > behaviour of initial MTU setting: earlier, the MTU for a link > created on top of a given lower device, without an initial MTU > specification, was set to the MTU of the lower device minus > headroom as a result of this path in vxlan_dev_configure(): > > if (!conf->mtu) > dev->mtu = lowerdev->mtu - > (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM); > > which is now gone. Now, the initial MTU, in absence of a > configured value, is simply set by ether_setup() to ETH_DATA_LEN > (1500 bytes). > > This breaks userspace expectations in case the MTU of > the lower device is higher than 1500 bytes minus headroom. > > Restore the previous behaviour by calculating, for a new link, > the MTU from the lower device, if present, and if no value is > explicitly configured. > > Reported-by: Junhan Yan <juyan@redhat.com> > Fixes: a985343ba906 ("vxlan: refactor verification and application of configuration") > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > --- > I guess this should be queued up for -stable, back to 4.13. > > I'm actually introducing the third occurrence of this calculation (there's > another one in vxlan_config_apply(), and one in vxlan_change_mtu()). I would > anyway fix the userspace breakage first, and then plan on getting rid of several > bits of MTU logic duplication, which spans further than this. 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. Regards, Matthias > > drivers/net/vxlan.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index 19b9cc51079e..3a7e36cdf2c7 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -3085,6 +3085,9 @@ static void vxlan_config_apply(struct net_device *dev, > > if (conf->mtu) > dev->mtu = conf->mtu; > + else if (lowerdev) > + dev->mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM : > + VXLAN_HEADROOM); > > vxlan->net = src_net; > } >
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.
On 12/14/2017 01:10 AM, Stefano Brivio wrote: > 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. 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
On Thu, 14 Dec 2017 01:25:40 +0100 Matthias Schiffer <mschiffer@universe-factory.net> wrote: > On 12/14/2017 01:10 AM, Stefano Brivio wrote: > > 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. > > 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; You would also need to check that lowerdev is present, though. Otherwise, you're changing the behaviour again, that is, if lowerdev is not present, we want to keep 1500 and not set ETH_MAX_MTU (65535). Sure you can change the if condition to reflect that, but IMHO it becomes quite awkward.
On 12/14/2017 01:31 AM, Stefano Brivio wrote: > On Thu, 14 Dec 2017 01:25:40 +0100 > Matthias Schiffer <mschiffer@universe-factory.net> wrote: > >> On 12/14/2017 01:10 AM, Stefano Brivio wrote: >>> 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. >> >> 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; > > You would also need to check that lowerdev is present, though. > > Otherwise, you're changing the behaviour again, that is, if lowerdev is > not present, we want to keep 1500 and not set ETH_MAX_MTU (65535). > > Sure you can change the if condition to reflect that, but IMHO it > becomes quite awkward. > Hmm, right. So here's a Reviewed-by: Matthias Schiffer <mschiffer@universe-factory.net> for the minimal change for -stable.
On 12/14/2017 03:31 AM, Stefano Brivio wrote: > On Thu, 14 Dec 2017 01:25:40 +0100 > Matthias Schiffer <mschiffer@universe-factory.net> wrote: > >> On 12/14/2017 01:10 AM, Stefano Brivio wrote: >>> 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. >> >> 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; > > You would also need to check that lowerdev is present, though. > 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... 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; } if (dev->mtu > max_mtu) Thanks, Alexey > Otherwise, you're changing the behaviour again, that is, if lowerdev is > not present, we want to keep 1500 and not set ETH_MAX_MTU (65535). > > Sure you can change the if condition to reflect that, but IMHO it > becomes quite awkward. >
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: > > On Thu, 14 Dec 2017 01:25:40 +0100 > > Matthias Schiffer <mschiffer@universe-factory.net> wrote: > > > >> On 12/14/2017 01:10 AM, Stefano Brivio wrote: > >>> 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. > >> > >> 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; > > > > You would also need to check that lowerdev is present, though. > > > > > 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?
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
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 19b9cc51079e..3a7e36cdf2c7 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -3085,6 +3085,9 @@ static void vxlan_config_apply(struct net_device *dev, if (conf->mtu) dev->mtu = conf->mtu; + else if (lowerdev) + dev->mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM : + VXLAN_HEADROOM); vxlan->net = src_net; }
Commit a985343ba906 ("vxlan: refactor verification and application of configuration") introduced a change in the behaviour of initial MTU setting: earlier, the MTU for a link created on top of a given lower device, without an initial MTU specification, was set to the MTU of the lower device minus headroom as a result of this path in vxlan_dev_configure(): if (!conf->mtu) dev->mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM); which is now gone. Now, the initial MTU, in absence of a configured value, is simply set by ether_setup() to ETH_DATA_LEN (1500 bytes). This breaks userspace expectations in case the MTU of the lower device is higher than 1500 bytes minus headroom. Restore the previous behaviour by calculating, for a new link, the MTU from the lower device, if present, and if no value is explicitly configured. Reported-by: Junhan Yan <juyan@redhat.com> Fixes: a985343ba906 ("vxlan: refactor verification and application of configuration") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- I guess this should be queued up for -stable, back to 4.13. I'm actually introducing the third occurrence of this calculation (there's another one in vxlan_config_apply(), and one in vxlan_change_mtu()). I would anyway fix the userspace breakage first, and then plan on getting rid of several bits of MTU logic duplication, which spans further than this. drivers/net/vxlan.c | 3 +++ 1 file changed, 3 insertions(+)