On 9/27/23 11:10, Nikolay Aleksandrov wrote:
On 9/27/23 10:57, Trent Lloyd wrote:
Commit 804b854d374e ("net: bridge: disable bridge MTU auto tuning if it was set manually") disabled auto-tuning of the bridge MTU when the MTU was explicitly set by the user, however that would only happen when the MTU was set after creation. This commit ensures auto-tuning is also disabled when the MTU is set during bridge creation.
Currently when the br_netdev_ops br_change_mtu function is called, the flag BROPT_MTU_SET_BY_USER is set. However this function is only called when the MTU is changed after interface creation and is not called if the MTU is specified during creation with IFLA_MTU (br_dev_newlink).
br_change_mtu also does not get called if the MTU is set to the same value it currently has, which makes it difficult to work around this issue (especially for the default MTU of 1500) as you have to first change the MTU to some other value and then back to the desired value.
Yep, I think I also described this in the commit message of my patch.
Add new selftests to ensure the bridge MTU is handled correctly: - Bridge created with user-specified MTU (1500) - Bridge created with user-specified MTU (2000) - Bridge created without user-specified MTU - Bridge created with user-specified MTU set after creation (2000)
Regression risk: Any workload which erroneously specified an MTU during creation but accidentally relied upon auto-tuning to a different value may be broken by this change.
Hmm, you're right. There's a risk of regression. Also it acts differently when set to 1500 as you've mentioned. I think they should act the same, also bridge's fake rtable RTAX_MTU is not set.
Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2034099 Fixes: 804b854d374e ("net: bridge: disable bridge MTU auto tuning if it was set manually") Signed-off-by: Trent Lloyd trent.lloyd@canonical.com
So I've been thinking about this and to elaborate - my concerns are two first is inconsistency between setting MTU at create vs later when it's the default (i.e. this way disables auto-tuning, while later it doesn't) and second is potential breakage as some network managers always set the mtu when creating devices. That would suddenly start disabling auto-tuning and that will surprise some people which expect it to work.
I think if you make them both act the same and leave out that case with a big comment why, this would be good for -net. To fully solve the problem without breaking anyone I think we should add control over the autotuning flag so it can be turned on/off by the users. That would be explicit and will work for all cases, but that is net-next material.
Thanks, Nik