netdev - Re: [PATCH] net:bridge:bridge mtu auto tuning does not always work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 9 Apr 2019 12:20:00 +0300
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Huang Rui <huangruippp@...il.com>, davem@...emloft.net
Cc:     lirongqing@...du.com, netdev@...r.kernel.org,
        Roopa Prabhu <roopa@...ulusnetworks.com>
Subject: Re: [PATCH] net:bridge:bridge mtu auto tuning does not always work

On 09/04/2019 10:36, Huang Rui wrote:
> If someone setup a bridge and add a port(for example: eth0)
> into the bridge, but configure the bridge's mtu which is equal
> to eth0's mtu, the auto tuning flag will not be set true. But
> the meaning of the auto tuning flag is that it will be set true
> if a user configure bridge's mtu.
> 
> Signed-off-by: Huang Rui <huangruiPPP@...il.com>
> ---
>  net/core/dev.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2b67f2a..ba410d7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7670,8 +7670,12 @@ int dev_set_mtu_ext(struct net_device *dev, int new_mtu,
>  {
>  	int err, orig_mtu;
>  
> -	if (new_mtu == dev->mtu)
> -		return 0;
> +	if (new_mtu == dev->mtu) {
> +		if (dev->priv_flags & IFF_EBRIDGE)
> +			return __dev_set_mtu(dev, new_mtu);
> +		else
> +			return 0;
> +	}
>  
>  	/* MTU must be positive, and in range */
>  	if (new_mtu < 0 || new_mtu < dev->min_mtu) {
> 

Nacked-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>

No, it is not. If the user specified the MTU then auto-tuning is disabled
on purpose because that is what the user desired as MTU. We've had so many
MTU issues over the years, it's hard to get it right and the auto-min/max
policies work only for some cases, to workaround that we disable the auto
policy when the user explicitly specifies the MTU[1].

Also please CC bridge maintainers when sending bridge patches.

Thanks,
 Nik

[1] This has been the behaviour since:
commit 804b854d374e
Author: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Date:   Fri Mar 30 13:46:19 2018 +0300

    net: bridge: disable bridge MTU auto tuning if it was set manually
    
    As Roopa noted today the biggest source of problems when configuring
    bridge and ports is that the bridge MTU keeps changing automatically on
    port events (add/del/changemtu). That leads to inconsistent behaviour
    and network config software needs to chase the MTU and fix it on each
    such event. 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.
    
    Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>