From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthias Schiffer Subject: Re: [PATCH net] vxlan: Restore initial MTU setting based on lower device Date: Thu, 14 Dec 2017 00:57:32 +0100 Message-ID: References: <0c6caef03156aa51673c50ebb59889fc001b74be.1513198761.git.sbrivio@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="edCAQMAPuvesG9caV4bfRRRjcwTJstFnt" Cc: "David S . Miller" , netdev@vger.kernel.org, Junhan Yan , Jiri Benc , Hangbin Liu To: Stefano Brivio Return-path: Received: from chaos.universe-factory.net ([31.24.148.19]:43492 "EHLO chaos.universe-factory.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750795AbdLMX5e (ORCPT ); Wed, 13 Dec 2017 18:57:34 -0500 In-Reply-To: <0c6caef03156aa51673c50ebb59889fc001b74be.1513198761.git.sbrivio@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --edCAQMAPuvesG9caV4bfRRRjcwTJstFnt Content-Type: multipart/mixed; boundary="9E4alxML5LFf8INdSSgaOEbu6dqk1FCJp"; protected-headers="v1" From: Matthias Schiffer To: Stefano Brivio Cc: "David S . Miller" , netdev@vger.kernel.org, Junhan Yan , Jiri Benc , Hangbin Liu Message-ID: Subject: Re: [PATCH net] vxlan: Restore initial MTU setting based on lower device References: <0c6caef03156aa51673c50ebb59889fc001b74be.1513198761.git.sbrivio@redhat.com> In-Reply-To: <0c6caef03156aa51673c50ebb59889fc001b74be.1513198761.git.sbrivio@redhat.com> --9E4alxML5LFf8INdSSgaOEbu6dqk1FCJp Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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(): >=20 > if (!conf->mtu) > dev->mtu =3D lowerdev->mtu - > (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM); >=20 > 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). >=20 > This breaks userspace expectations in case the MTU of > the lower device is higher than 1500 bytes minus headroom. >=20 > 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. >=20 > Reported-by: Junhan Yan > Fixes: a985343ba906 ("vxlan: refactor verification and application of c= onfiguration") > Signed-off-by: Stefano Brivio > --- > I guess this should be queued up for -stable, back to 4.13. >=20 > I'm actually introducing the third occurrence of this calculation (ther= e'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 o= f 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 =3D lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM := VXLAN_HEADROOM); } if (dev->mtu > max_mtu) dev->mtu =3D max_mtu; [...] Unless I'm overlooking something, this should already do the same thing a= nd your patch is redundant. Regards, Matthias >=20 > drivers/net/vxlan.c | 3 +++ > 1 file changed, 3 insertions(+) >=20 > 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, > =20 > if (conf->mtu) > dev->mtu =3D conf->mtu; > + else if (lowerdev) > + dev->mtu =3D lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM : > + VXLAN_HEADROOM); > =20 > vxlan->net =3D src_net; > } >=20 --9E4alxML5LFf8INdSSgaOEbu6dqk1FCJp-- --edCAQMAPuvesG9caV4bfRRRjcwTJstFnt Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEZmTnvaa2aYgexS51Fu8/ZMsgHZwFAloxvmwACgkQFu8/ZMsg HZxZchAAmfvM4hZjCAypriNkPcc2tFMy9uayzz5Rug1rezqI0Pd30B5jGXo4BR2U 4t2iOz2sVyYBcScLwrTxoYEl8A4EgFZfFYycGnQM4O+kHru0zalwtO+AipEXdNnF WcrlB+9kv/dVaplLS5tCbrPOIdNOnDXNe1PmHIj0qtCjb+03h/5iy7AdUCTtFz2M /UVyEPdjpSF9XuDPQ7Ow8YRXlsEwNVnk0bzivZRdzAJe9Ni8rPL4GIvxFHDbnFop KMY0zvWaYMtVSWeEZjwQgRy25zK+DJkZ3YpLPHHLL+41njKXcVbOlCA5akiPVo4S dcIvEt9seQZqUutHfpKcoxr0Qqr57bqUIn2ciJvglf3IU07rclR5j6cVUzStwCXN PpPHO+l7PXZ8z9aFojEuKbP7VYoIMAIziVggEd2klbRvnbiU/ceTqkbsLIUGXqr7 laPLgMd8z1y3iP+guzPQ3FB4EVZw5iiqCjjmEGE7IGMszl0KhDwibP+fAoztlLDY zfYa+PjNwcWBXS+G/yp9RrHFdAE4H/9fKjIL3z2A+71Y+slxrUltIhPR373JLBzj IAt/sCdflN+oSYFbmB2vT3DFhsKxajfVw526WSR1f1szhud544k6wT6mwQ1A63/l JlkauxczIDK+cHgBQ1er5XXnK3B+t8ZsK+E+AEQ3jnA5GcZYEq0= =if1B -----END PGP SIGNATURE----- --edCAQMAPuvesG9caV4bfRRRjcwTJstFnt--