Re: 答复: [PATCH][net-next] bridge: clean up mtu_set_by_user setting to false and comments

[Nikolay Aleksandrov <nikolay@cumulusnetworks.com>]

On 13/07/18 12:11, Li,Rongqing wrote:
> 
> 
>> -----邮件原件-----
>> 发件人: Nikolay Aleksandrov [mailto:nikolay@cumulusnetworks.com]
>> 发送时间: 2018年7月13日 16:01
>> 收件人: Li,Rongqing <lirongqing@baidu.com>; netdev@vger.kernel.org
>> 主题: Re: [PATCH][net-next] bridge: clean up mtu_set_by_user setting to
>> false and comments
>>
>> 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 */
>>
> But after this comment, mtu_set_by_user is set to true, and br_mtu_auto_adjust
>  will not truly be run, how to set mtu_set_by_user to false?

It will be run if the mtu_set_by_user is set to "false".

> 
> 230     /* this flag will be cleared if the MTU was automatically adjusted */
> 231     br->mtu_set_by_user = true;
> 
> And the line 457  is useless, since it run only if it is false?

dev_set_mtu() calls ndo_change_mtu() which makes mtu_set_by_user = true but that is
not really _true_ since this is an automatic MTU adjustment so we need to revert the
value to false so the automatic adjustment will continue to work.
But if you go ahead and set the MTU yourself manually, then mtu_set_by_user will be
equal to true and will stay that way, thus disabling the auto adjust behaviour.

This is used to differentiate when auto adjust is used and when user has set the MTU.
As I already said everything is working as expected and you should not remove this code.

> 
> 445 void br_mtu_auto_adjust(struct net_bridge *br)
> 446 {
> 447     ASSERT_RTNL();
> 448 
> 449     /* if the bridge MTU was manually configured don't mess with it */
> 450     if (br->mtu_set_by_user)
> 451         return;
> 452 
> 453     /* change to the minimum MTU and clear the flag which was set by
> 454      * the bridge ndo_change_mtu callback
> 455      */
> 456     dev_set_mtu(br->dev, br_mtu_min(br));
> 457     br->mtu_set_by_user = false;
> 458 }
> 
> 
> -R
>