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

[richb-hanover]

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:

• Which file needs to be edited? (In the folder I see defaults.sh, config.primary.sh, config.primary.sh.new, libs.sh, etc. Which file contains the variable I'm looking for?)
• 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?)
• Editing these files runs the risk of corrupting the shell script, making the whole thing fall apart and fail to run (likely with an inscrutable error message).

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

• It eliminates (most) syntax errors, since the UCI config file format is straightforward. Unrecognized values would be ignored.
• It makes it easy to share configs (just send /etc/config/cake-autorate instead of defaults.sh, config.primary.sh, etc...)
• It opens the way to a GUI that reads /etc/config/cake-autorate and lets you enter updated values in a web browser.
• Which in turn might make it possible to create an opkg package for installing cake-autorate.

I would be interested to hear your thoughts

[rany2]

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]

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]

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]

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]

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

[rany2]

I've finally gotten around to working on this. I'll make a new PR for this soon!

[lynxthecat]

Really nice work @rany2. It looks super elegant to me. So the cake-autorate config files get generated from the uci output and then cake-autorate is launched based on those.

[richb-hanover]

@rany2 @lynxthecat This is terrific! And exactly what I was thinking of! Thanks!

If I understand the code correctly, it reads the /etc/config/cake-autorate (copied from the example-uci-config.txt in the commit) file and produces a config file like the one shown below.

What seems to be missing (please correct me if I'm wrong) is a description of how these would be used in production. How do existing people (and newcomers) use this code? Where is it documented?

Thanks again.

# This file is automatically generated. Do not edit!
# You can edit the UCI configuration file /etc/config/cake-autorate.

dl_if='ifb-secondary'
ul_if='secondary'
adjust_dl_shaper_rate='1'
adjust_ul_shaper_rate='0'
min_dl_shaper_rate_kbps='3000'
base_dl_shaper_rate_kbps='15000'
max_dl_shaper_rate_kbps='70000'
min_ul_shaper_rate_kbps='3000'
base_ul_shaper_rate_kbps='15000'
max_ul_shaper_rate_kbps='30000'

[rany2]

@richb-hanover This is just preliminary work. I have a working POC that I'll finish up and submit later which handles migration of old configs. I'll also work on improving the documentation.

[richb-hanover]

Great news! Thanks