Re: [chrony-dev] [PATCH] enable stratum setting for refclocks (take 2)

[Andreas Steinmetz <ast@xxxxxxxx>]

On Thu, 2017-10-05 at 18:35 +0200, Miroslav Lichvar wrote:
> On Thu, Oct 05, 2017 at 02:12:49PM +0200, Andreas Steinmetz wrote:
> > The attached patch allows the setting of the stratum for refclocks.
> > This is missing as e.g. a PHC refclock may not be stratum 0 (actual
> > timesource -> qemu host -> guest with chronyd using PHC).
> > 
> > Example configuration with patch applied:
> > 
> > refclock PHC /dev/ptp0 poll 3 dpoll -2 offset 0 stratum 2
> 
> Ok, this sounds good.
> 
> > @@ -756,6 +757,10 @@
> >      } else if (!strcasecmp(cmd, "maxlockage")) {
> >        if (sscanf(line, "%d%n", &max_lock_age, &n) != 1)
> >          break;
> > +    } else if (!strcasecmp(cmd, "stratum")) {
> > +      if (sscanf(line, "%d%n", &stratum, &n) != 1 ||
> > +          stratum >= NTP_MAX_STRATUM || stratum < 0)
> > +        break;
> 
> Please keep the options in alphabetical order if possible.
> 

Hm, there is no real alphabetical order. OK, still moved down.

> > --- chrony-3.2.orig/sources.h	2017-09-15 08:32:09.000000000
> > +0200
> > +++ chrony-3.2/sources.h	2017-09-20 18:40:56.873225170 +0200
> > @@ -60,7 +60,7 @@
> >     the individual source-type instance creation routines. */
> >  
> >  extern SRC_Instance SRC_CreateNewInstance(uint32_t ref_id,
> > SRC_Type type, int sel_options,
> > -                                          IPAddr *addr, int
> > min_samples, int max_samples,
> > +                                          IPAddr *addr, int
> > min_samples, int max_samples, int stratum,
> >                                            double min_delay, double
> > asymmetry);
> 
> Does this really need a new SRC parameter? I'd prefer if this was an
> option specific to refclocks and it just changed the stratum that is
> passed with the refclock sample.
> 

Now no new SRC parameter, but a new SRC function. The problem is that
the required access to the structure to set the refclock stratum for
SRC is only available to SRC.

> Also, please document the option in the chrony.conf man page.
> 

Added.

> -- 
> Miroslav Lichvar
> 
-- 
Andreas Steinmetz                       SPAMmers use robotrap@xxxxxxxx

diff -rNu chrony-3.2.orig/conf.c chrony-3.2/conf.c
--- chrony-3.2.orig/conf.c	2017-09-15 08:32:09.000000000 +0200
+++ chrony-3.2/conf.c	2017-10-05 21:07:02.967733891 +0200
@@ -680,7 +680,7 @@
 static void
 parse_refclock(char *line)
 {
-  int n, poll, dpoll, filter_length, pps_rate, min_samples, max_samples, sel_options;
+  int n, poll, dpoll, filter_length, pps_rate, min_samples, max_samples, sel_options, stratum;
   int max_lock_age, pps_forced;
   uint32_t ref_id, lock_ref_id;
   double offset, delay, precision, max_dispersion, pulse_width;
@@ -704,6 +704,7 @@
   ref_id = 0;
   max_lock_age = 2;
   lock_ref_id = 0;
+  stratum = 0;
 
   if (!*line) {
     command_parse_error();
@@ -774,6 +775,10 @@
     } else if (!strcasecmp(cmd, "maxdispersion")) {
       if (sscanf(line, "%lf%n", &max_dispersion, &n) != 1)
         break;
+    } else if (!strcasecmp(cmd, "stratum")) {
+      if (sscanf(line, "%d%n", &stratum, &n) != 1 ||
+          stratum >= NTP_MAX_STRATUM || stratum < 0)
+        break;
     } else if (!strcasecmp(cmd, "width")) {
       if (sscanf(line, "%lf%n", &pulse_width, &n) != 1)
         break;
@@ -819,6 +824,7 @@
   refclock->ref_id = ref_id;
   refclock->max_lock_age = max_lock_age;
   refclock->lock_ref_id = lock_ref_id;
+  refclock->stratum = stratum;
 }
 
 /* ================================================== */
diff -rNu chrony-3.2.orig/doc/chrony.conf.adoc chrony-3.2/doc/chrony.conf.adoc
--- chrony-3.2.orig/doc/chrony.conf.adoc	2017-09-15 08:32:09.000000000 +0200
+++ chrony-3.2/doc/chrony.conf.adoc	2017-10-05 21:28:33.133392995 +0200
@@ -516,6 +516,8 @@
 algorithm. Increasing the delay is useful to avoid having no majority in the
 source selection or to make it prefer other sources. The default is 1e-9 (1
 nanosecond).
+*stratum* _stratum_:::
+This option sets the NTP stratum of the refclock. This can be useful when the refclock provides time with a stratum other than 0. The default is 0.
 *precision* _precision_:::
 This option sets the precision of the reference clock (in seconds). The default
 value is the estimated precision of the system clock.
diff -rNu chrony-3.2.orig/doc/chrony.conf.man.in chrony-3.2/doc/chrony.conf.man.in
--- chrony-3.2.orig/doc/chrony.conf.man.in	2017-10-05 21:32:29.855702739 +0200
+++ chrony-3.2/doc/chrony.conf.man.in	2017-10-05 21:30:30.838515323 +0200
@@ -696,6 +696,12 @@
 nanosecond).
 .RE
 .sp
+\fBstratum\fP \fIstratum\fP
+.RS 4
+This option sets the NTP stratum of the refclock. This can be useful when the
+refclock provides time with a stratum other than 0. The default is 0.
+.RE
+.sp
 \fBprecision\fP \fIprecision\fP
 .RS 4
 This option sets the precision of the reference clock (in seconds). The default
diff -rNu chrony-3.2.orig/refclock.c chrony-3.2/refclock.c
--- chrony-3.2.orig/refclock.c	2017-09-15 08:32:09.000000000 +0200
+++ chrony-3.2/refclock.c	2017-10-05 21:22:34.816904393 +0200
@@ -261,6 +261,7 @@
 
   inst->source = SRC_CreateNewInstance(inst->ref_id, SRC_REFCLOCK, params->sel_options, NULL,
                                        params->min_samples, params->max_samples, 0.0, 0.0);
+  SRC_SetStratum(inst->source, params->stratum);
 
   DEBUG_LOG("refclock %s refid=%s poll=%d dpoll=%d filter=%d",
       params->driver_name, UTI_RefidToString(inst->ref_id),
diff -rNu chrony-3.2.orig/refclock.h chrony-3.2/refclock.h
--- chrony-3.2.orig/refclock.h	2017-09-15 08:32:09.000000000 +0200
+++ chrony-3.2/refclock.h	2017-10-05 21:04:37.308310872 +0200
@@ -43,6 +43,7 @@
   int max_samples;
   int sel_options;
   int max_lock_age;
+  int stratum;
   uint32_t ref_id;
   uint32_t lock_ref_id;
   double offset;
diff -rNu chrony-3.2.orig/sources.c chrony-3.2/sources.c
--- chrony-3.2.orig/sources.c	2017-09-15 08:32:09.000000000 +0200
+++ chrony-3.2/sources.c	2017-10-05 21:19:38.399193903 +0200
@@ -306,6 +306,14 @@
 /* ================================================== */
 
 void
+SRC_SetStratum(SRC_Instance instance, int stratum)
+{
+  instance->sel_info.stratum = stratum;
+}
+
+/* ================================================== */
+
+void
 SRC_SetRefid(SRC_Instance instance, uint32_t ref_id, IPAddr *addr)
 {
   instance->ref_id = ref_id;
@@ -652,7 +660,8 @@
     }
 
     si = &sources[i]->sel_info;
-    SST_GetSelectionData(sources[i]->stats, &now, &si->stratum,
+    SST_GetSelectionData(sources[i]->stats, &now,
+                         sources[i]->type == SRC_REFCLOCK ? NULL : &si->stratum,
                          &si->lo_limit, &si->hi_limit, &si->root_distance,
                          &si->std_dev, &first_sample_ago,
                          &si->last_sample_ago, &si->select_ok);
@@ -1351,6 +1360,9 @@
     /* Call stats module to fill out estimates */
     SST_DoSourceReport(src->stats, report, now);
 
+    if (src->type == SRC_REFCLOCK)
+      report->stratum = src->sel_info.stratum;
+
     return 1;
   }
 
diff -rNu chrony-3.2.orig/sources.h chrony-3.2/sources.h
--- chrony-3.2.orig/sources.h	2017-09-15 08:32:09.000000000 +0200
+++ chrony-3.2/sources.h	2017-10-05 21:15:28.123733221 +0200
@@ -73,6 +73,9 @@
 /* Function to reset a source */
 extern void SRC_ResetInstance(SRC_Instance instance);
 
+/* Function to change the source's stratum */
+extern void SRC_SetStratum(SRC_Instance instance, int stratum);
+
 /* Function to change the sources's reference ID and IP address */
 extern void SRC_SetRefid(SRC_Instance instance, uint32_t ref_id, IPAddr *addr);
 
diff -rNu chrony-3.2.orig/sourcestats.c chrony-3.2/sourcestats.c
--- chrony-3.2.orig/sourcestats.c	2017-09-15 08:32:09.000000000 +0200
+++ chrony-3.2/sourcestats.c	2017-10-05 21:04:37.309310882 +0200
@@ -653,7 +653,8 @@
   i = get_runsbuf_index(inst, inst->best_single_sample);
   j = get_buf_index(inst, inst->best_single_sample);
 
-  *stratum = inst->strata[get_buf_index(inst, inst->n_samples - 1)];
+  if (stratum)
+    *stratum = inst->strata[get_buf_index(inst, inst->n_samples - 1)];
   *std_dev = inst->std_dev;
 
   sample_elapsed = fabs(UTI_DiffTimespecsToDouble(now, &inst->sample_times[i]));