Packages: mwan3: Should check_quality be enabled on the default WAN interface configurations?

[jamesmacwhite]

Maintainer: @feckert
Environment: N/A

Description:

The default mwan3 config has the four check_quality configuration options set on the wan and wanb interfaces.

https://github.com/openwrt/packages/blob/master/net/mwan3/files/etc/config/mwan3#L15-L18
https://github.com/openwrt/packages/blob/master/net/mwan3/files/etc/config/mwan3#L47-L50

However because check_quality is not set this would default to be off and therefore having these configuration options set in the config is redundant currently, as they aren't doing anything, as they'd never be used here, with the boolean flag being off.

https://github.com/openwrt/packages/blob/master/net/mwan3/files/usr/sbin/mwan3track#L220-L254

Should we set option check_quality '1' in the default configuration for these interfaces or remove them entirely?

A minor change but just opening any discussion on it being a default configuration.

[aaronjg]

Makes sense to me to turn them off. While we are at it, why not remove everything after track_ip and let the defaults work.
https://github.com/openwrt/packages/blob/master/net/mwan3/files/usr/sbin/mwan3track#L137-L152

The default values in luci should also be updated so that they match the defaults in mwan3track:
https://github.com/openwrt/luci/blob/master/applications/luci-app-mwan3/luasrc/model/cbi/mwan/interfaceconfig.lua

[jamesmacwhite]

@aaronjg Funny you mention default values. I found the same thing:

https://github.com/openwrt/luci/issues/4389

Several configuration options for the interface configuration were missing in the OpenWrt docs and then noticed the default values not being the same in LuCI compared to mwan3track, so I opened an issue on that luci-app-mwan3.

[feckert]

I have fixed the default values in luci-app-mwan3 for failure_loss and recovery_loss.

In the OpenWrt there is no clear line how we should proceed here.
Either we use only the minimal setting, or we list all options as comments on the options we have, because not everyone uses LuCI.
And if an option is set/changed via LuCI then all comments with a # are deleted in mwan3.

Edit:

Should we set option check_quality '1' in the default configuration for these interfaces or remove them entirely?

I would not do that. For most users the ping check is enough.

[jamesmacwhite]

Understood. I would propose the four check_quality config options are removed from the default configuration in /etc/config/mwan3 on the wan and wanb interfaces, given they are currently not doing anything without check_quality being enabled and if not specified they can fallback to the defaults set in mwan3track unless configured by the user, now that the defaults in LuCI match.

I would agree, that the ping check alone is probably the most common method used, so having failure_latency, recovery_latency, failure_loss and recovery_loss in the default config doesn't seem necessary.

The official documentation now has them listed in the table under "Interface configuration".

[jamesmacwhite]

@feckert We can close this, the commit will likely go in #13169, I made a new branch for @aaronjg to cherry pick the config updates.