| Message ID | 1513272000-30848-1-git-send-email-alexey.kodanev@oracle.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2] vxlan: restore dev->mtu setting based on lower device | expand |
On Thu, 14 Dec 2017 20:20:00 +0300 Alexey Kodanev <alexey.kodanev@oracle.com> wrote: > Stefano Brivio says: > 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. > > This patch restores the previous behaviour on newlink operation. Since > max_mtu can be negative and we update dev->mtu directly, also check it > for valid minimum. > > Reported-by: Junhan Yan <juyan@redhat.com> > Fixes: a985343ba906 ("vxlan: refactor verification and application of configuration") > Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> Thanks for re-posting this. Acked-by: Stefano Brivio <sbrivio@redhat.com> Or even (but I don't know what's the current netdev practice for these particular cases): Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
From: Alexey Kodanev <alexey.kodanev@oracle.com> Date: Thu, 14 Dec 2017 20:20:00 +0300 > Stefano Brivio says: > 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. > > This patch restores the previous behaviour on newlink operation. Since > max_mtu can be negative and we update dev->mtu directly, also check it > for valid minimum. > > Reported-by: Junhan Yan <juyan@redhat.com> > Fixes: a985343ba906 ("vxlan: refactor verification and application of configuration") > Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> Applied and queued up for -stable, thanks.
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)
Stefano Brivio says: 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. This patch restores the previous behaviour on newlink operation. Since max_mtu can be negative and we update dev->mtu directly, also check it for valid minimum. Reported-by: Junhan Yan <juyan@redhat.com> Fixes: a985343ba906 ("vxlan: refactor verification and application of configuration") Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> --- v2: Based on initial patch from Stefano Brivio: "vxlan: Restore initial MTU setting based on lower device" In this version, 'dev->mtu' is set from 'max_mtu' that already has the the needed initial MTU value based on the lower device. Plus, it is adding extra check for valid minimum of 'max_mtu'. drivers/net/vxlan.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)