[chrony-dev] SOCK refclock system time resolution

[Josef 'Jeff' Sipek <jeffpc@xxxxxxxxxxxxxx>]

I'm playing with feeding data to the SOCK refclock with GPS time myself
(without using gpsd, etc.).  I saw that the samples contain the host/system
timestamp as a struct timeval.  I changed it to allow nanosecond time of
measurements but before I try to polish it and send it for inclusion, I
wanted to check if there would there be interest in that?  The current diff
is below, but it needs a bit more cleanup and testing.

Thanks,

Jeff.

---

diff --git a/refclock_sock.c b/refclock_sock.c
index 2da57ef..add17e2 100644
--- a/refclock_sock.c
+++ b/refclock_sock.c
@@ -35,11 +35,13 @@
 #include "sched.h"
 #include "socket.h"
 
-#define SOCK_MAGIC 0x534f434b
+#define SOCK_MAGIC_TV 0x534f434b
+#define SOCK_MAGIC_TS 0x534f434c
 
-struct sock_sample {
+struct sock_sample_tv64 {
   /* Time of the measurement (system time) */
-  struct timeval tv;
+  int64_t time_sec;
+  int64_t time_usec;
 
   /* Offset between the true time and the system time (in seconds) */
   double offset;
@@ -54,36 +56,54 @@ struct sock_sample {
   /* Padding, ignored */
   int _pad;
 
-  /* Protocol identifier (0x534f434b) */
+  /* Protocol identifier (SOCK_MAGIC_TV) */
   int magic;
 };
 
-/* On 32-bit glibc-based systems enable conversion between timevals using
-   32-bit and 64-bit time_t to support SOCK clients compiled with different
-   time_t size than chrony */
-#ifdef __GLIBC_PREREQ
-#if __GLIBC_PREREQ(2, 34) && __TIMESIZE == 32
-#define CONVERT_TIMEVAL 1
-#if defined(_TIME_BITS) && _TIME_BITS == 64
-typedef int32_t alt_time_t;
-typedef int32_t alt_suseconds_t;
-#else
-typedef int64_t alt_time_t;
-typedef int64_t alt_suseconds_t;
-#endif
-struct alt_timeval {
-  alt_time_t tv_sec;
-  alt_suseconds_t tv_usec;
+struct sock_sample_tv32 {
+  int32_t time_sec;
+  int32_t time_usec;
+  double offset;
+  int pulse;
+  int leap;
+  int _pad;
+  int magic;
+};
+
+struct sock_sample_ts {
+  uint64_t time_sec;
+  uint64_t time_nsec;
+  double offset;
+  uint32_t pulse;
+  uint32_t leap;
+  uint32_t _pad;
+
+  /* Protocol identifier (SOCK_MAGIC_TS) */
+  uint32_t magic;
+};
+
+_Static_assert(sizeof (struct sock_sample_tv64) == 40,
+	       "Size of sock_sample_tv64 changed");
+_Static_assert(sizeof (struct sock_sample_tv32) == 32,
+	       "Size of sock_sample_tv32 changed");
+_Static_assert(sizeof (struct sock_sample_ts) == 40,
+	       "Size of sock_sample_ts changed");
+
+union sock_sample {
+  struct sock_sample_tv32 tv32;
+  struct sock_sample_tv64 tv64;
+  struct sock_sample_ts ts;
 };
-#endif
-#endif
 
 static void read_sample(int sockfd, int event, void *anything)
 {
-  char buf[sizeof (struct sock_sample) + 16];
+  char buf[sizeof (union sock_sample) + 1];
   struct timespec sys_ts, ref_ts;
-  struct sock_sample sample;
+  union sock_sample sample;
   RCL_Instance instance;
+  double offset;
+  int pulse;
+  int leap;
   int s;
 
   instance = (RCL_Instance)anything;
@@ -95,49 +115,65 @@ static void read_sample(int sockfd, int event, void *anything)
     return;
   }
 
-  if (s == sizeof (sample)) {
-    memcpy(&sample, buf, sizeof (sample));
-#ifdef CONVERT_TIMEVAL
-  } else if (s == sizeof (sample) - sizeof (struct timeval) + sizeof (struct alt_timeval)) {
-    struct alt_timeval atv;
-    memcpy(&atv, buf, sizeof (atv));
-#ifndef HAVE_LONG_TIME_T
-    if (atv.tv_sec > INT32_MAX || atv.tv_sec < INT32_MIN ||
-        atv.tv_usec > INT32_MAX || atv.tv_usec < INT32_MIN) {
-      DEBUG_LOG("Could not convert 64-bit timeval");
-      return;
-    }
-#endif
-    sample.tv.tv_sec = atv.tv_sec;
-    sample.tv.tv_usec = atv.tv_usec;
-    DEBUG_LOG("Converted %d-bit timeval", 8 * (int)sizeof (alt_time_t));
-    memcpy((char *)&sample + sizeof (struct timeval), buf + sizeof (struct alt_timeval),
-           sizeof (sample) - sizeof (struct timeval));
-#endif
-  } else {
-    DEBUG_LOG("Unexpected length of SOCK sample : %d != %ld",
+  if (s > sizeof (sample)) {
+    DEBUG_LOG("Unexpected length of SOCK sample : %d > %ld",
               s, (long)sizeof (sample));
     return;
   }
 
-  if (sample.magic != SOCK_MAGIC) {
-    DEBUG_LOG("Unexpected magic number in SOCK sample : %x != %x",
-              (unsigned int)sample.magic, (unsigned int)SOCK_MAGIC);
+  memcpy(&sample, buf, sizeof (sample.ts));
+
+  _Static_assert(sizeof (struct sock_sample_ts) == sizeof (struct sock_sample_tv64),
+		 "sock_sample_ts and sock_sample_tv64 mismatch in size");
+  if (s == sizeof (sample.ts) || s == sizeof (sample.tv64)) {
+    if (sample.ts.magic == SOCK_MAGIC_TS) {
+      sys_ts.tv_sec = sample.ts.time_sec;
+      sys_ts.tv_nsec = sample.ts.time_nsec;
+      offset = sample.ts.offset;
+      pulse = sample.ts.pulse;
+      leap = sample.ts.leap;
+    } else if (sample.tv64.magic == SOCK_MAGIC_TV) {
+      sys_ts.tv_sec = sample.tv64.time_sec;
+      sys_ts.tv_nsec = sample.tv64.time_usec * 1000;
+      offset = sample.tv64.offset;
+      pulse = sample.tv64.pulse;
+      leap = sample.tv64.leap;
+    } else {
+      DEBUG_LOG("Unexpected magic number in SOCK sample : %x != %x or %x != %x",
+                (unsigned int)sample.ts.magic, (unsigned int)SOCK_MAGIC_TS,
+		(unsigned int)sample.tv64.magic, (unsigned int)SOCK_MAGIC_TV);
+      return;
+    }
+  } else if (s == sizeof (sample.tv32) && sample.tv32.magic == SOCK_MAGIC_TV) {
+    if (sample.tv32.magic != SOCK_MAGIC_TV) {
+      DEBUG_LOG("Unexpected magic number in SOCK sample : %x != %x",
+                (unsigned int)sample.tv32.magic, (unsigned int)SOCK_MAGIC_TV);
+      return;
+    }
+
+    sys_ts.tv_sec = sample.tv32.time_sec;
+    sys_ts.tv_nsec = sample.tv32.time_usec * 1000;
+    offset = sample.tv32.offset;
+    pulse = sample.tv32.pulse;
+    leap = sample.tv32.leap;
+  } else {
+    DEBUG_LOG("Unexpected length of SOCK sample : %d != %ld or %ld or %ld",
+              s, (long)sizeof (sample.ts), (long)sizeof (sample.tv64),
+	      (long)sizeof (sample.tv32));
     return;
   }
 
-  UTI_TimevalToTimespec(&sample.tv, &sys_ts);
   UTI_NormaliseTimespec(&sys_ts);
 
-  if (!UTI_IsTimeOffsetSane(&sys_ts, sample.offset))
+  if (!UTI_IsTimeOffsetSane(&sys_ts, offset))
     return;
 
-  UTI_AddDoubleToTimespec(&sys_ts, sample.offset, &ref_ts);
+  UTI_AddDoubleToTimespec(&sys_ts, offset, &ref_ts);
 
-  if (sample.pulse) {
-    RCL_AddPulse(instance, &sys_ts, sample.offset);
+  if (pulse) {
+    RCL_AddPulse(instance, &sys_ts, offset);
   } else {
-    RCL_AddSample(instance, &sys_ts, &ref_ts, sample.leap);
+    RCL_AddSample(instance, &sys_ts, &ref_ts, leap);
   }
 }
 

-- 
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.