Re: [chrony-dev] [PATCH] local threshold option

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Mon, Mar 25, 2024 at 07:10:31PM +0000, Andy Fiddaman wrote:
> How does the attached patch look? This adds `local threshold` as you
> suggested, that defaults to 0 which means it is not used. If it's set,
> then the local reference is not activated until the root distance drops
> below the value once.
> 
> In our configuration, we can then use something like:
> 
>     local stratum 10 distance 1.0 threshold 1.0

The distance option is already described as a threshold, so I think a
different name might work better. How about trigger, start or activate?

Let's think a bit about possible future options. We might need a
timeout-based activation, e.g. wait for 10 minutes before activating
local. It could be a second parameter of this option or a separate
option. What naming would be most consistent?

> --- a/candm.h
> +++ b/candm.h
> @@ -237,6 +237,7 @@ typedef struct {
>    int32_t stratum;
>    Float distance;
>    int32_t orphan;
> +  Float threshold;
>    int32_t EOR;
>  } REQ_Local;

This is an incompatible change in the request, so there should be also
a new code defined (REQ_LOCAL3). You could also add a few reserved
fields to the command, so next time it could keep the code if the
original behavior is preserved by interpreting the old reserved
field.

> @@ -1699,6 +1699,11 @@ An example of the directive is:
>  ----
>  local stratum 10 orphan distance 0.1
>  ----
> +*threshold* _distance_:::
> +This option sets an activating threshold for the local reference. The local
> +reference will not be used until the root distance drops below the configured
> +threshold once. This can be used to prevent the local reference being activated
> +on a server which has never been synchronised with an upstream server.

This should mention the special value of 0 and that it is the default.

> @@ -1158,7 +1163,11 @@ REF_GetReferenceParams
>      *root_delay = our_root_delay;
>      *root_dispersion = dispersion;
>  
> -  } else if (enable_local_stratum) {
> +    if (distance > 0 && distance < local_threshold)
> +      local_threshold_met = 1;

This should be earlier in the function so the new condition below can
trigger on the first pass?

> +
> +  } else if (enable_local_stratum &&
> +             (local_threshold == 0 || local_threshold_met)) {

Please use 0.0 in places where a floating-point variable is compared.

Thanks,

-- 
Miroslav Lichvar


-- 
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.