On 09/04/2019 12:20, Nikolay Aleksandrov wrote:
> 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 <huangrui...@gmail.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 <niko...@cumulusnetworks.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 <niko...@cumulusnetworks.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 <niko...@cumulusnetworks.com>
> Signed-off-by: David S. Miller <da...@davemloft.net>
>
Ahh wait, looking again at the change I get why you're doing that. The commit
message is wrong though, you're trying to disable auto-tuning always even
when setting the same MTU, right ?
Or IOW, I guess by the auto-tuning flag you mean BROPT_MTU_SET_BY_USER ?
I don't like device-specific hacks in the generic code, specifically this
code will not generate an event but will affect state. I don't see a better
approach currently though.
At the very least please add a better explanation and better subject line.
It is not "auto tuning does not always work", but maybe something similar
to "always disable auto-tuning when the user specified MTU" with details
of the case where setting the same MTU doesn't disable auto-tuning currently.
Thanks
