From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikolay Aleksandrov Subject: Re: [PATCH][net-next] bridge: clean up mtu_set_by_user setting to false and comments Date: Fri, 13 Jul 2018 11:00:56 +0300 Message-ID: <780d2dff-c03c-8e0a-f2c0-2b3d5974512d@cumulusnetworks.com> References: <1531464444-26517-1-git-send-email-lirongqing@baidu.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit To: Li RongQing , netdev@vger.kernel.org Return-path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:36020 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731884AbeGMIOb (ORCPT ); Fri, 13 Jul 2018 04:14:31 -0400 Received: by mail-wm0-f68.google.com with SMTP id s14-v6so8498371wmc.1 for ; Fri, 13 Jul 2018 01:01:00 -0700 (PDT) In-Reply-To: <1531464444-26517-1-git-send-email-lirongqing@baidu.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 13/07/18 09:47, Li RongQing wrote: > Once mtu_set_by_user is set to true, br_mtu_auto_adjust will > not run, and no chance to clear mtu_set_by_user. > ^^ This was by design, there is no error here and no "cleanup" is needed. If you read the ndo_change_mtu() call you'll see the comment: /* this flag will be cleared if the MTU was automatically adjusted */ It is the only way we can know that the MTU was automatically adjusted or set by the user manually in which case we need to _stop_ automatically adjusting MTU. This was done to be backwards compatible as much as possible but still give the option to have user-configured MTU which doesn't disappear (and is not overwritten). So please next time read the original commit. >>From the original commit 804b854d374e ("net: bridge: disable bridge MTU auto tuning if it was set manually"): " ... Let's improve on that situation and allow for the user to set any MTU within ETH_MIN/MAX limits, but once manually configured it is the user's responsibility to keep it correct afterwards. In case the MTU isn't manually set - the behaviour reverts to the previous and the bridge follows the minimum MTU. ... " > and br_mtu_auto_adjust will run only if mtu_set_by_user is > false, so not need to set it to false again > > Cc: Nikolay Aleksandrov > Signed-off-by: Li RongQing > --- > net/bridge/br_device.c | 1 - > net/bridge/br_if.c | 4 ---- > 2 files changed, 5 deletions(-) > > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c > index e682a668ce57..c636bc2749c2 100644 > --- a/net/bridge/br_device.c > +++ b/net/bridge/br_device.c > @@ -227,7 +227,6 @@ static int br_change_mtu(struct net_device *dev, int new_mtu) > > dev->mtu = new_mtu; > > - /* this flag will be cleared if the MTU was automatically adjusted */ > br->mtu_set_by_user = true; > #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) > /* remember the MTU in the rtable for PMTU */ > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > index 05e42d86882d..47c65da4b1be 100644 > --- a/net/bridge/br_if.c > +++ b/net/bridge/br_if.c > @@ -450,11 +450,7 @@ void br_mtu_auto_adjust(struct net_bridge *br) > if (br->mtu_set_by_user) > return; > > - /* change to the minimum MTU and clear the flag which was set by > - * the bridge ndo_change_mtu callback > - */ > dev_set_mtu(br->dev, br_mtu_min(br)); > - br->mtu_set_by_user = false; > } > > static void br_set_gso_limits(struct net_bridge *br) > Nacked-by: Nikolay Aleksandrov