Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-15 Thread Keller, Jacob E



> -Original Message-
> From: Hal Murray 
> Sent: Tuesday, November 15, 2022 2:09 AM
> To: Keller, Jacob E 
> Cc: Maciek Machnikowski ; Hal Murray
> ; linuxptp-devel@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> 
> >> What about rcl_sock or refclock_sock? It's used in the file linked by
> Miroslav.
> > Both of those sound good to me. Slight preference to refclock_sock if its 
> > not too
> long.
> 
> How about SOCK?
> 
> In the ntp context, we already have SHM and PPS.  Both show up in the refid
> slot in packets.
> 
> Just to make sure we are on the same wavelength...  I'm looking for a term
> that can be used as a handle when the context is well known for things like
> "Try SOCK, it worked for me."
> 

Yea. I just found "sock" on its own to be generic and not tell me anything 
about what the servo did.

I don't want to get too distracted by the name though...

Thanks,
Jake

> 
> --
> These are my opinions.  I hate spam.
> 
> 



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-15 Thread Richard Cochran
On Tue, Nov 15, 2022 at 07:29:23AM +, Keller, Jacob E wrote:
> Both of those sound good to me. Slight preference to refclock_sock if its not 
> too long.

+1 refclock_sock

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-15 Thread Hal Murray
>> What about rcl_sock or refclock_sock? It's used in the file linked by 
Miroslav.
> Both of those sound good to me. Slight preference to refclock_sock if its not 
> too long.

How about SOCK?

In the ntp context, we already have SHM and PPS.  Both show up in the refid 
slot in packets.

Just to make sure we are on the same wavelength...  I'm looking for a term 
that can be used as a handle when the context is well known for things like 
"Try SOCK, it worked for me."


-- 
These are my opinions.  I hate spam.





___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Keller, Jacob E



> -Original Message-
> From: Maciek Machnikowski 
> Sent: Monday, November 14, 2022 10:55 PM
> To: Keller, Jacob E 
> Cc: Hal Murray ; Miroslav Lichvar
> ; linuxptp-devel@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> 
> On Mon, Nov 14, 2022 at 05:06:17PM +, Keller, Jacob E wrote:
> >
> >
> > > -Original Message-
> > > From: Hal Murray 
> > > Sent: Monday, November 14, 2022 4:34 AM
> > > To: Miroslav Lichvar 
> > > Cc: Keller, Jacob E ; linuxptp-
> > > de...@lists.sourceforge.net; Hal Murray 
> > > Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> > >
> > >
> > > >> How specific is this to chronyd?
> > > > AFAIK no other application implements the server side of the protocol.
> > > >> Would it make sense to call this chronysock
> > > >> instead of just sock?
> > > > Yes, that makes sense. If there are no other issues with the patches, I 
> > > > can
> > > > resend.
> > >
> > > Calling it chronysock has the disadvantage of sounding like only chrony 
> > > should
> > > use it.
> > >
> >
> > Yea, but I feel that just "sock" is vague. I'm not totally opposed to it 
> > though.
> 
> What about rcl_sock or refclock_sock? It's used in the file linked by 
> Miroslav.
> 
> Regards
> Maciek

Both of those sound good to me. Slight preference to refclock_sock if its not 
too long.

Thanks,
Jake


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Maciek Machnikowski
On Mon, Nov 14, 2022 at 05:06:17PM +, Keller, Jacob E wrote:
> 
> 
> > -Original Message-
> > From: Hal Murray 
> > Sent: Monday, November 14, 2022 4:34 AM
> > To: Miroslav Lichvar 
> > Cc: Keller, Jacob E ; linuxptp-
> > de...@lists.sourceforge.net; Hal Murray 
> > Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> > 
> > 
> > >> How specific is this to chronyd?
> > > AFAIK no other application implements the server side of the protocol.
> > >> Would it make sense to call this chronysock
> > >> instead of just sock?
> > > Yes, that makes sense. If there are no other issues with the patches, I 
> > > can
> > > resend.
> > 
> > Calling it chronysock has the disadvantage of sounding like only chrony 
> > should
> > use it.
> > 
> 
> Yea, but I feel that just "sock" is vague. I'm not totally opposed to it 
> though.

What about rcl_sock or refclock_sock? It's used in the file linked by Miroslav.

Regards
Maciek


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Keller, Jacob E



> -Original Message-
> From: Hal Murray 
> Sent: Monday, November 14, 2022 4:34 AM
> To: Miroslav Lichvar 
> Cc: Keller, Jacob E ; linuxptp-
> de...@lists.sourceforge.net; Hal Murray 
> Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> 
> 
> >> How specific is this to chronyd?
> > AFAIK no other application implements the server side of the protocol.
> >> Would it make sense to call this chronysock
> >> instead of just sock?
> > Yes, that makes sense. If there are no other issues with the patches, I can
> > resend.
> 
> Calling it chronysock has the disadvantage of sounding like only chrony should
> use it.
> 

Yea, but I feel that just "sock" is vague. I'm not totally opposed to it though.

Thanks,
Jake

> >> The implementation seems fine but its using an interface that was defined 
> >> by
> >> chrony. I suppose another application could implement the same interface
> >> though..
> 
> > ntpsec might be interested in implementing it. We'll see.
> 
> Is there a URL for the spec?  I don't want an RFC.  Good comments in a header
> file may be enough.  A separate document may be better if there are
> complications that need explaining.
> 
> is there a version number?
> 
> 
> 
> I agree that the current SHM setup is far from wonderful.  There is a clean
> way to make SHM read-only by receivers so you can have multiple receivers.
> That would let you run gpsmon while chronyd/ntpd is running.
> 
> 
> --
> These are my opinions.  I hate spam.
> 
> 



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Miroslav Lichvar
On Mon, Nov 14, 2022 at 04:34:05AM -0800, Hal Murray wrote:
> Is there a URL for the spec?  I don't want an RFC.  Good comments in a header 
> file may be enough.  A separate document may be better if there are 
> complications that need explaining.

Here is the structure of the message with some comments:
https://git.tuxfamily.org/chrony/chrony.git/tree/refclock_sock.c#n38

If you started with a copy of the SHM driver in ntpd, it would need to
be modified to use a datagram Unix socket instead of the shared memory
segment, listen for messages on the socket (io_addclock()) instead of
polling in the _timer() function, and interpret the fields in the
message like this:

- "tv" field in SOCK is the same as receiveTimeStamp in SHM
- tv + offset is clockTimestamp
- messages with non-zero pulse can be ignored in the initial
  implementation as nothing really uses it
- leap has the same meaning
- magic is a protocol sanity+version check, should be always
  0x534f434b

The missing fields precision and samples can be set to -30 and 1
respectively.

That should be it. I can help with the patch if interested.

> is there a version number?

No. If there was a need to make an incompatible change, we can change
the magic number.

-- 
Miroslav Lichvar



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Hal Murray
>> How specific is this to chronyd?
> AFAIK no other application implements the server side of the protocol.
>> Would it make sense to call this chronysock
>> instead of just sock?
> Yes, that makes sense. If there are no other issues with the
> patches, I can resend.

Calling it chronysock has the disadvantage of sounding like only chrony should 
use it.

>> The implementation seems fine but its using an interface that was defined by
>> chrony. I suppose another application could implement the same interface
>> though..

> ntpsec might be interested in implementing it. We'll see.

Is there a URL for the spec?  I don't want an RFC.  Good comments in a header 
file may be enough.  A separate document may be better if there are 
complications that need explaining.

Is there a version number?  (or plan for how to update things)



I agree that the current SHM setup is far from wonderful.  There is a clean 
way to make SHM read-only by receivers so you can have multiple receivers.  
That would let you run gpsmon while chronyd/ntpd is running.





-- 
These are my opinions.  I hate spam.





___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-14 Thread Miroslav Lichvar
On Thu, Nov 10, 2022 at 09:51:52AM -0800, Jacob Keller wrote:
> How specific is this to chronyd?

AFAIK no other application implements the server side of the protocol.

> Would it make sense to call this chronysock
> instead of just sock?

Yes, that makes sense. If there are no other issues with the patches,
I can resend.

> The implementation seems fine but its using an interface that was defined by
> chrony. I suppose another application could implement the same interface
> though..

ntpsec might be interested in implementing it. We'll see.

-- 
Miroslav Lichvar



___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-10 Thread Jacob Keller




On 11/10/2022 7:19 AM, Miroslav Lichvar wrote:

Add a second servo that provides samples to other processes in order to
control the clock. The chrony SOCK refclock uses a Unix domain socket
instead of a shared memory segment.

The main advantage over the NTP SHM refclock is better security as the
socket can be located in a directory with restricted access, while the
shared memory segment (using a predictable key) can be created by
untrusted users or applications if they can run before ptp4l/phc2sys and
chronyd/ntpd. There doesn't seem to a backward-compatible fix of the
protocol as both sides are expected to be able to create the segment if
it doesn't exist yet, possibly under a non-root owner, there is no
authentication of messages, and the protocol cannot be restarted if one
side decides to remove and recreate the segment.

Signed-off-by: Miroslav Lichvar 
---
  chronysock.c| 163 
  chronysock.h|  26 +++
  config.c|   2 +
  configs/default.cfg |   1 +
  makefile|   2 +-
  phc2sys.8   |   9 ++-
  phc2sys.c   |   3 +
  ptp4l.8 |  12 ++--
  servo.c |   4 ++
  servo.h |   1 +
  10 files changed, 216 insertions(+), 7 deletions(-)
  create mode 100644 chronysock.c
  create mode 100644 chronysock.h

diff --git a/chronysock.c b/chronysock.c
new file mode 100644
index 000..a8c0033
--- /dev/null
+++ b/chronysock.c
@@ -0,0 +1,163 @@
+/**
+ * @file chronysock.c
+ * @brief Implements a servo providing samples to chronyd over socket.
+ * @note Copyright (C) 2022 Miroslav Lichvar 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include "chronysock.h"
+#include "config.h"
+#include "print.h"
+#include "servo_private.h"
+
+#define LEAP_NORMAL 0
+#define LEAP_INSERT 1
+#define LEAP_DELETE 2
+#define SOCK_MAGIC 0x534f434b
+
+/* Copied from chrony-3.2/refclock_sock.c */
+struct sock_sample {
+   /* Time of the measurement (system time) */
+   struct timeval tv;
+
+   /* Offset between the true time and the system time (in seconds) */
+   double offset;
+
+   /* Non-zero if the sample is from a PPS signal, i.e. another source
+  is needed to obtain seconds */
+   int pulse;
+
+   /* 0 - normal, 1 - insert leap second, 2 - delete leap second */
+   int leap;
+
+   /* Padding, ignored */
+   int _pad;
+
+   /* Protocol identifier (0x534f434b) */
+   int magic;
+};
+
+struct sock_servo {
+   struct servo servo;
+   int sock_fd;
+   int leap;
+};
+
+static void chronysock_destroy(struct servo *servo)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+   free(s);
+}
+
+static double chronysock_sample(struct servo *servo,
+   int64_t offset,
+   uint64_t local_ts,
+   double weight,
+   enum servo_state *state)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+   struct sock_sample sample;
+
+   memset(&sample, 0, sizeof(sample));
+   sample.tv.tv_sec = local_ts / 10ULL;
+   sample.tv.tv_usec = local_ts % 10ULL / 1000U;
+   sample.offset = -offset / 1e9;
+   sample.magic = SOCK_MAGIC;
+
+   switch (s->leap) {
+   case -1:
+   sample.leap = LEAP_DELETE;
+   break;
+   case 1:
+   sample.leap = LEAP_INSERT;
+   break;
+   default:
+   sample.leap = LEAP_NORMAL;
+   }
+
+   if (send(s->sock_fd, &sample, sizeof sample, 0) != sizeof sample) {
+   pr_err("chronysock: send failed: %m");
+   return 0;
+   }
+
+   *state = SERVO_UNLOCKED;
+   return 0.0;
+}
+
+static void chronysock_sync_interval(struct servo *servo, double interval)
+{
+}
+
+static void chronysock_reset(struct servo *servo)
+{
+}
+
+static void chronysock_leap(struct servo *servo, int leap)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+
+   s->leap = leap;
+}
+
+struct servo *chronysock_servo_create(struct config *cfg)
+{
+   char *addr = config_get_string(cfg

[Linuxptp-devel] [PATCH 2/4] Add sock servo.

2022-11-10 Thread Miroslav Lichvar
Add a second servo that provides samples to other processes in order to
control the clock. The chrony SOCK refclock uses a Unix domain socket
instead of a shared memory segment.

The main advantage over the NTP SHM refclock is better security as the
socket can be located in a directory with restricted access, while the
shared memory segment (using a predictable key) can be created by
untrusted users or applications if they can run before ptp4l/phc2sys and
chronyd/ntpd. There doesn't seem to a backward-compatible fix of the
protocol as both sides are expected to be able to create the segment if
it doesn't exist yet, possibly under a non-root owner, there is no
authentication of messages, and the protocol cannot be restarted if one
side decides to remove and recreate the segment.

Signed-off-by: Miroslav Lichvar 
---
 chronysock.c| 163 
 chronysock.h|  26 +++
 config.c|   2 +
 configs/default.cfg |   1 +
 makefile|   2 +-
 phc2sys.8   |   9 ++-
 phc2sys.c   |   3 +
 ptp4l.8 |  12 ++--
 servo.c |   4 ++
 servo.h |   1 +
 10 files changed, 216 insertions(+), 7 deletions(-)
 create mode 100644 chronysock.c
 create mode 100644 chronysock.h

diff --git a/chronysock.c b/chronysock.c
new file mode 100644
index 000..a8c0033
--- /dev/null
+++ b/chronysock.c
@@ -0,0 +1,163 @@
+/**
+ * @file chronysock.c
+ * @brief Implements a servo providing samples to chronyd over socket.
+ * @note Copyright (C) 2022 Miroslav Lichvar 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include "chronysock.h"
+#include "config.h"
+#include "print.h"
+#include "servo_private.h"
+
+#define LEAP_NORMAL 0
+#define LEAP_INSERT 1
+#define LEAP_DELETE 2
+#define SOCK_MAGIC 0x534f434b
+
+/* Copied from chrony-3.2/refclock_sock.c */
+struct sock_sample {
+   /* Time of the measurement (system time) */
+   struct timeval tv;
+
+   /* Offset between the true time and the system time (in seconds) */
+   double offset;
+
+   /* Non-zero if the sample is from a PPS signal, i.e. another source
+  is needed to obtain seconds */
+   int pulse;
+
+   /* 0 - normal, 1 - insert leap second, 2 - delete leap second */
+   int leap;
+
+   /* Padding, ignored */
+   int _pad;
+
+   /* Protocol identifier (0x534f434b) */
+   int magic;
+};
+
+struct sock_servo {
+   struct servo servo;
+   int sock_fd;
+   int leap;
+};
+
+static void chronysock_destroy(struct servo *servo)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+   free(s);
+}
+
+static double chronysock_sample(struct servo *servo,
+   int64_t offset,
+   uint64_t local_ts,
+   double weight,
+   enum servo_state *state)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+   struct sock_sample sample;
+
+   memset(&sample, 0, sizeof(sample));
+   sample.tv.tv_sec = local_ts / 10ULL;
+   sample.tv.tv_usec = local_ts % 10ULL / 1000U;
+   sample.offset = -offset / 1e9;
+   sample.magic = SOCK_MAGIC;
+
+   switch (s->leap) {
+   case -1:
+   sample.leap = LEAP_DELETE;
+   break;
+   case 1:
+   sample.leap = LEAP_INSERT;
+   break;
+   default:
+   sample.leap = LEAP_NORMAL;
+   }
+
+   if (send(s->sock_fd, &sample, sizeof sample, 0) != sizeof sample) {
+   pr_err("chronysock: send failed: %m");
+   return 0;
+   }
+
+   *state = SERVO_UNLOCKED;
+   return 0.0;
+}
+
+static void chronysock_sync_interval(struct servo *servo, double interval)
+{
+}
+
+static void chronysock_reset(struct servo *servo)
+{
+}
+
+static void chronysock_leap(struct servo *servo, int leap)
+{
+   struct sock_servo *s = container_of(servo, struct sock_servo, servo);
+
+   s->leap = leap;
+}
+
+struct servo *chronysock_servo_create(struct config *cfg)
+{
+   char *addr = config_get_string(cfg, NULL, "chrony_sock_address");
+   struct sockaddr_un sa;
+