lynxthecat / cake-autorate

Eliminate the excess latency and jitter terrorizing your 4G, 5G, LTE, Starlink or other variable rate connection!
https://www.bufferbloat.net
GNU General Public License v2.0
263 stars 24 forks source link

Feature request: read configuration from /etc/config/cake-autorate #273

Open richb-hanover opened 5 months ago

richb-hanover commented 5 months ago

One of the things I find most painful about using cake-autorate is setting its configuration. The setup.sh script makes things pretty easy, but then I hit a wall with the configuration. Here are some of the hassles:

Proposal

After cake-autorate has read in all the current configuration files, it could read a new configuration file to override any of the defaults. I propose the filename /etc/config/cake-autorate.

This file would be in standard UCI format. It could have a section for logging and one for each interface that would be controlled by cake-autorate. Any config option that matches a corresponding variable name would override the default values. A name that does not match a variable would be ignored (perhaps generating a message)

Advantages

I would be interested to hear your thoughts

rany2 commented 5 months ago

It's a gnarly file to edit. (Hmmm... I see min_dl_shaper_rate_kbps, base_dl_shaper_rate_kbps , and max_dl_shaper_rate_kbps? The values don't line up - have I used the right number of zeroes in each? Did I screw up?)

I don't disagree here, I think it might be worth renaming this to (for example) min_dl_shaper and have it take values as tc-cake expects it (i.e. you could specify 1Mbit as a value to min_dl_shaper as you could when you setup cake directly via tc). Shouldn't be too tricky to do.

After cake-autorate has read in all the current configuration files, it could read a new configuration file to override any of the defaults. I propose the filename /etc/config/cake-autorate.

This seems easy enough to do, we'd just generate the config.INSTANCE_NAME.sh on the fly by calling uci show cake-autorate and parsing the output for every instance.

rany2 commented 5 months ago

It eliminates (most) syntax errors, since the UCI config file format is straightforward. Unrecognized values would be ignored.

Nitpicking but personally I find this to be a disadvantage. If a user set something in the config and it doesn't do anything, they should be aware that it is pointlessly being set and cake-autorate should fail.

Right now a user with a pointless config entry in their config.INSTANCE_NAME.sh would know that it is being pointlessly set. This is functionality I intentionally added as a feature; by default, bash wouldn't care about something like this when sourcing a shell script!

richb-hanover commented 5 months ago

I am looking a year or so down the line, when tens (or hopefully, hundreds) of thousands of people are using cake-autorate. I don't expect those people to see any of these (raw) config files. By then, we will have constructed a pleasing way for people to enter the data.

I primarily care that we find a path to move away from editing the shell scripts to something that can serve a much larger audience. Thanks.

lynxthecat commented 5 months ago

Here are my initial thoughts.

Taking a step back and thinking about the existing configuration system in general, I think right now we have a straightforward configuration system that seems to work well - there are default values provided in defaults.sh and values set per instance in each config.${instance}.sh file that the user wants (e.g. config.primary.sh or config.secondary.sh). The config.${instance}.sh.new file simply follows the usual updated config file convention of not overwriting an existing config file on setup, but rather writing any new config file with .new appended at the end of the filename.

@rany2 already added fantastic parsing to verify that any entry in the config file finds a corresponding entry in defaults.sh file and is of the right type (string, integer, float or whatever). I think the related error messages are clear. If they are not, then let’s make them clearer.

In general I think users seem to have understood the existing configuration system and associated documentation just fine. I do not recall seeing any reports suggesting otherwise and we have seen plenty of users provide data from successful deployments, which deployments necessarily must have been configured in order to work with the correct interfaces.

But @richb-hanover it seems your proposal is not to modify the existing configuration system or make cake-autorate exclusive to OpenWrt, but rather to add an OpenWrt-specific UCI format config file wrapper to cake-autorate, and also package up cake-autorate into an OpenWrt package. Am I right? I see the benefit in this, but I lack the know how. @rany2 is this something you might be interested in and able to help with?

@moeller0 this is an area you’ve always been quite interested in. Perhaps you have some thoughts?

richb-hanover commented 5 months ago

You are correct - I do not want to touch the current config file formats. They work exactly as we need them to work. But as I noted initially, they're a pain to modify. And if you screw up, you might break your cake-autorate implementation.

I have to disagree with your assertion that the config process is "just fine". All the people who have slogged through the process are happy. This is good - it gives us a wonderful base for experimentation - and for making our own personal links better.

But looking at the README and INSTALLATION pages makes a lot of people shy away. For cake-autorate to become a household name (for example, why couldn't it be installed by default? Obviously, not enabled...), there will need to be an improved configuration facility.

Overnight, I also came up with another advantage of this wrapper configuration file: It "immunizes" us from changes to variable names within the script. We still need to think hard about the names of the configuration options within the UCI-format file, and how they might evolve over time.

But then we can make the INSTALLATION instructions far easier: just edit /etc/config/cake-autorate (or use the web GUI) with the obvious values and you're done.


If this idea might be interesting, I would be willing to create a draft config file along with the uci commands to parse it... Thanks for listening