These speedtests appear to have been made using Safari, I would ask you to repeat these measurements with firefox instead (and preferably not from a phone but from a wired device).
Safari is known to have some issues with measuring time precisely (potentially intended as workaround against side-channel CPU attacks) and measuring over WiFi will add any additional WiFi issue on top of any internet access bufferbloat so IMHO it is better to tackle these issues one by one, so it is clear what to do.
1 Like
Do we need some rudimentary error checking in the prefix string? It could be lethal...
... should we perhaps just add a toggle to enable a particular prefix of @patrakov's choice?
Nah, "you break it, you keep it" seems fine for that, maybe check the ping invocation for its return status and write an ERROR/DEBUG message into the log?
I think that functionality might generally be useful so why not generalize it?
(@patrakov might once this actually works enable a second instance likely requiring:
ping_prefix_string="mwan3 use gpon exec"
or similar so hard coding seems a bit inflexible, but we could/should this as an example in the description text for 'ping_prefix_string' in the config file.
In good unix tradition, give users enough rope to shot themselves in the foot 
1 Like
I'd say no. Just claim no support if this option is used at all. Even for mwan3, because it produces an extra line of its own output (see this bug) in the beginning.
Definitely no. I use mwan3, but this is not the only policy-based routing package available, especially if we consider unofficial packages. Someone else may prefer something else, for good reasons, and I don't want to lock them out.
1 Like
OK thanks - I'll add this prefix change in as well as the config file based cake instance launching (with /var/run/cake-autorate/X/ being set based on cake-autorate_config.X.sh, and it would be ace if you were able to check concurrent instances of cake-autorate work on your combined connections. I guess one will stall and wait for load whilst the other is up and then resume upon load/ping response continuation, but it will require testing and perhaps some ironing out of any issues.
I would propose we do this in two steps, first ping_prefix_string and tackle multiple concurrent instances later. Because we need to first convince ourselves that this actually fixes the issue, no?
That I agree with, hence let's do this in steps, please?
1 Like
First one here (see testing branch):
@patrakov can you test?
OK, will test it now...
1 Like
I tested this by removing the mentioning of mwan3 from the initscript, setting ping_prefix_string="mwan3 use lte exec" in the config file, and setting a low value for sustained_idle_sleep_thr_s. I confirm that the script still can parse the output of fping despite the junk line added by mwan3, and stops it correctly when the sleep logic kicks in.
Could you also test with iputils ping, please, so that we have tested both existing pingers?
Thanks for the suggestion - iputils-ping doesn't work for me. The error is:
/root/cake-autorate/cake-autorate.sh: line 552: local: `': not a valid identifier
The line is:
local -n pinger_pid=$2
However, this doesn't seem to be specific to the use of ping_prefix_string. Can't debug it further, I have a meeting in a few minutes, and will have to work later then.
Thanks will try the iputils-pinger tonight if no one beats me to it
OK, so (line 522 is local -n pinger_pid=$2):
start_pinger_next_pinger_time_slot()
{
# wait until next pinger time slot and start pinger in its slot
# this allows pingers to be stopped and started (e.g. during sleep or reflector rotation)
# whilst ensuring pings will remain spaced out appropriately to maintain granularity
local pinger=$1
local -n pinger_pid=$2
is called without the second argument ATM:
start_pingers_ping()
{
# Initiate pingers
for ((pinger=0; pinger<$no_pingers; pinger++))
do
start_pinger_next_pinger_time_slot $pinger
done
}
So we "just" need to pass a second argument to collect pinger_pid into or make the local -n pinger_pid=$2 line dependent on having 2 input arguments?
This issue and also the issue you identified on GitHub relating to the missing ping binary extra arguments have been fixed here I think:
Looks like we already retain the iputils-ping PID elsewhere:
As you identified above, it seems that start_pingers_ping() calls start_pinger_next_pinger_time_slot() with just one argument.
And it seems that start_pinger_next_pinger_time_slot() itself calls:
start_pinger_binary_$pinger_binary $pinger
So I think we don't need the 'pinger_pid' function argument passed by reference inside start_pinger_next_time_slot() to keep track of the PID. As that's handled in lines 501/504 anyway, and I believe the pinger_pids array is still accessible from within maintain_pingers() so that the subsequent trapped kill within the maintain_pingers() context will correctly kill the background ping instances.
The various functions and call sequence is just part of keeping it all generic for handling different ping binaries including those like iputils-ping that ping a single reflector and need spacing out or fping that manage multiple reflectors round robin.
As an aside, concerning:
start_pinger_binary_$pinger_binary $pinger
actually that's a little sloppy isn't it because it relies on bash to expand out $pinger_binary and not just $pinger - we should really use:
start_pinger_binary_${pinger_binary} $pinger
It would be great if anyone could test that both pinger_binary set to 'ping' and separately 'fping' work fine and that even on reflector rotation and/or starting and stopping cake-autorate there are no pinger processes left lingering.
This whole generalized approach in terms of handling pinger binaries will hopefully allow easily slotting in further pinger binaries such as @Lochnair's new C-based binary with round robin ICMP type 13 requests.
And once we've got this settled I'm keen to push my new multiple cake-autorate instance handling launcher and associated amended code.
sorry for the delay responding ... it is basically load balancing between two (or more) ifb's with different backends (fiber, lte, adsl etc) .. for DR but also to take the most of the available bandwitdh at one point ...
But in any event you'd benefit from multiple cake-autorate instances?
1 Like
thank you . let me try ... i presume it is setting up different services each with a different cfg file pointing to each ifb correct?
It's not supported yet but just around the corner since I worked on this last night for your use case:
It should be ready within the next few days.
1 Like
I will run a test, but it will be pretty basic, as I can not really exercise any of the fancy features.... started the fping test...
1 Like
So my I tested both:
pinger_binary=fping
[...]
ping_extra_args=("--tos=0x02") #fping
and
pinger_binary=ping
[...]
ping_extra_args=("-Q 0x02") #ping
In both cases htop revealed that $pinger_binary was executed with the requested extra argument. On exit I saw no leftover processes, but I had no issues in the past either and I run cake-autorate manually in a screen session anyway so might not have tested the critical condition ot begin with.