Re: [systemd-devel] [systemd-commits] src/timesync

2015-02-05 Thread Miroslav Lichvar
On Wed, Feb 04, 2015 at 06:28:59PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
 On Wed, Feb 04, 2015 at 06:24:13PM +0100, Lennart Poettering wrote:
  - If we did not manage to get a successful sync, try again
immediately, but not any more often than once per 10s or so...
 I think we should fall back here too, maybe more slowly. In case we can't
 connect, we shouldn't spam the network too much.

Yes, unless sendto() is failing (i.e. no packet was sent) the polling
interval should be increasing exponentially up to the maximum (4096 s)
to prevent overloading network or servers. Once per 10 seconds is way
too frequent.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 5/8] timesyncd: use longer PLL time constant

2014-09-02 Thread Miroslav Lichvar
On Fri, Aug 29, 2014 at 06:29:54PM +0200, Kay Sievers wrote:
 On Wed, Aug 27, 2014 at 4:47 PM, Miroslav Lichvar mlich...@redhat.com wrote:
  The shortest time constant that is stable with the kernel PLL (compiled
  with SHIFT_PLL=2) is about log2 of update interval - 3. Set the constant
  to poll - 2 to make room for one missed update.
 
 Experimentation did show, that the adjustment of the kernel was too
 gentle to reach the target time in the desired window, so we made it
 more stiff.

That's the problem with PLL, it adapts to frequency changes very
slowly.

 All timesyncd really cares is that we are not jumping, we
 do not really need to smooth out the adjustments.

It would probably help if the frequency was estimated over a fixed
interval on start before enabling the PLL mode (similarly to ntpd), or
at least the frequency was restored from a drift file, so it doesn't
always have to start from zero.

 We would re-check what happens with your patch, I'll hold back
 applying this for now.

It should take about four times longer to converge to zero offset.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 8/8] timesyncd: wait before reconnecting to first server

2014-09-02 Thread Miroslav Lichvar
On Fri, Aug 29, 2014 at 06:45:24PM +0200, Kay Sievers wrote:
 On Wed, Aug 27, 2014 at 4:47 PM, Miroslav Lichvar mlich...@redhat.com wrote:
  When all servers are exhausted, wait for one poll interval before trying
  to connect again to the first server in the list. Also, keep increasing
  the polling interval to make sure a client not getting any valid replies
  will not send requests to any server more frequently than is allowed by
  the maximum polling interval.
 
 What is the reason to keep track if we asked all servers, and not just
 decrease the frequency of switching them?

Delaying every reconnection by the current polling interval would work
nicely and there could even be an assert for the actual interval
between sending requests for extra safety, but my understanding is
that on start you want to find a good server quickly, or not?

If the first three servers in the list reply as unsynchronized, it
would take 3*32 seconds to switch to the fourth server. With this
patch, the reselection happens immediately after receiving the reply.

Is that acceptable? I can rewrite the patch if you want.

 There is also some code in src/timesync/timesyncd-manager.c +209:
   /* re-arm timer with increasing timeout,
   in case the packets never arrive back */
 
 which used to work but maybe broke with other changes.

In the current code that doesn't work as the 10s reconnect timeout is
always reached first.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 6/8] timesyncd: allow two missed replies before reselecting server

2014-09-02 Thread Miroslav Lichvar
On Fri, Aug 29, 2014 at 06:31:10PM +0200, Kay Sievers wrote:
 On Wed, Aug 27, 2014 at 4:47 PM, Miroslav Lichvar mlich...@redhat.com wrote:
  After receiving a reply from the server, allow two missed replies before
  switching to another server to avoid unnecessary clock hopping when
  packets are getting lost in the network.
 
 Hmm, what's the reason to do this? The servers in the list are not
 better or worse or sorted, are they?

No, they are not, but offsets measured from different NTP servers are
usually different (as the clocks are never exactly accurate, there are
asymmetries in the network, etc.), so reselection introduces an offset
that has to be corrected, possibly by stepping the clock. Also the
polling interval may be shortened unnecessarily.

Allowing few missed replies avoids reselection on every lost packet.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 7/8] timesyncd: don't reset polling interval when reselecting server

2014-09-02 Thread Miroslav Lichvar
On Fri, Aug 29, 2014 at 06:38:49PM +0200, Kay Sievers wrote:
 On Wed, Aug 27, 2014 at 4:47 PM, Miroslav Lichvar mlich...@redhat.com wrote:
  ---
   src/timesync/timesyncd-manager.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
 
 It did not apply without the earlier patch and in systemd we try to
 avoid using ! to test integers for 0. Added the fix.

Hm, I don't see this in the current git.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 8/8] timesyncd: wait before reconnecting to first server

2014-09-02 Thread Miroslav Lichvar
On Tue, Sep 02, 2014 at 02:32:37PM +0200, Kay Sievers wrote:
 On Tue, Sep 2, 2014 at 1:15 PM, Miroslav Lichvar mlich...@redhat.com wrote:
  There is also some code in src/timesync/timesyncd-manager.c +209:
/* re-arm timer with increasing timeout,
in case the packets never arrive back */
 
  which used to work but maybe broke with other changes.
 
  In the current code that doesn't work as the 10s reconnect timeout is
  always reached first.
 
 I see, I've removed it now.

With the other patch allowing missed replies included it's now getting
stuck as there is no timer to send the 2nd and 3rd request. Can you
please add it back? Sorry I didn't make that clear before.

Thanks,

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] timedated: add configure option to set name of controlled NTP service

2014-08-27 Thread Miroslav Lichvar
On Tue, Aug 26, 2014 at 09:08:47AM -0700, Marcel Holtmann wrote:
 ConnMan is a single daemon solution doing NTP, DHCP and DNS all in
 one place. Any sort of callouts are costing time. And that is time
 that has a visible user impact. There is nothing that justifies to
 have a bit more nanosecond accuracy of synchronized time than making
 the user wait for extra milliseconds to get their network connection
 and time.

You need the first clock update to happen milliseconds after the
network is up? Seriously? I agree that's not possible with chronyd or
ntpd even if they were listening to networkd, but I don't think it's a
requirement on any desktop system.

 You might realize that a theme shows up here. If you are building a
 server, then by all means install ntpd or Chrony and configure it.
 You are the admin, you know what you are doing. If you do not know
 what are doing or do not care, then keep it simple.

I'm not convinced. I think uninformed users should be using the best
tool for the typical use case they have at hand.

Trading falseticker detection, stable clock control with intermittent
connections, ability to drift through when network is congested,
ability to deal with broken clocks (as in some virtual machines) and
monotonic time just for a super fast update seems like a bad choice to
me.

I'm sure timesyncd will be significantly improved over time, but
currently I'd not describe it as more than appropriate for most
installations.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [BUG] time-sync.target reached prematurely

2014-08-27 Thread Miroslav Lichvar
On Tue, Aug 26, 2014 at 10:28:54PM +0200, Lennart Poettering wrote:
 On Tue, 26.08.14 22:11, Lukasz Stelmach (stl...@poczta.fm) wrote:
 
  Greetings.
  
  According to systemd.special(7) all services where correct time is
  essential should be ordered after time-sync.target. Implicitly this
  means that if systemd-timesyncd is enabled services ordered after the
  target should also get a usable network connection because the daemon
  uses the network (not a GPS receiver like ntpd could do) to synchronise
  the clock. However, this isn't actually the case as systemd-timesyncd
  reports READY=1 [1] before even checking if network is online *and*
  querying servers. The target is reached *before* time is synced.
  
  How would you suggest to fix this?
 
 Well, I guess similar to the network.target story it isn't really clear
 what time-sync.target is supposed to mean...

IIRC it was originally added for the ntpdate, ntp-wait and chrony-wait
services. There were two meanings merged into one target. One was that
the clock was set from NTP (ntpdate) and the other was that the
synchronization of the clock has reached a stable state a no more
jumps in the time are expected to happen (ntp-wait, chrony-wait).

timesyncd setting it before NTP query doesn't fit well into that.

 I mean, I am pretty sure we should never delay the boot by default for
 external conditions, such as network connectivity. hence, by default
 waiting for a network interface before we finish boot, and even waiting
 for a way to connect to an NTP server is not OK.

What services with dependency on the target do we use by default? Do
they really need it?

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 1/8] timesyncd: check if stratum is valid

2014-08-27 Thread Miroslav Lichvar
---
 src/timesync/timesyncd-manager.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index d80c72f..60f39c6 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -574,7 +574,8 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 return manager_connect(m);
 }
 
-if (NTP_FIELD_LEAP(ntpmsg.field) == NTP_LEAP_NOTINSYNC) {
+if (NTP_FIELD_LEAP(ntpmsg.field) == NTP_LEAP_NOTINSYNC ||
+ntpmsg.stratum == 0 || ntpmsg.stratum = 16) {
 log_debug(Server is not synchronized. Disconnecting.);
 return manager_connect(m);
 }
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 7/8] timesyncd: don't reset polling interval when reselecting server

2014-08-27 Thread Miroslav Lichvar
---
 src/timesync/timesyncd-manager.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index 9f12149..d1f77a8 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -735,7 +735,8 @@ static int manager_begin(Manager *m) {
 assert_return(m-current_server_address, -EHOSTUNREACH);
 
 m-missed_replies = NTP_MAX_MISSED_REPLIES;
-m-poll_interval_usec = NTP_POLL_INTERVAL_MIN_SEC * USEC_PER_SEC;
+if (!m-poll_interval_usec)
+m-poll_interval_usec = NTP_POLL_INTERVAL_MIN_SEC * 
USEC_PER_SEC;
 
 server_address_pretty(m-current_server_address, pretty);
 log_info(Using NTP server %s (%s)., strna(pretty), 
m-current_server_name-string);
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 0/8] timesyncd bugfixing and improvements

2014-08-27 Thread Miroslav Lichvar
When I was looking at the timesyncd code I noticed few issues that I
thought could be easily fixed and you might be interested in.

The last two patches is an attempt to fix the problem with frequent
polling. I'm not sure if this is robust enough, I feel like a minor
change in the code could break it easily, a more general approach
might be needed, maybe track timing of requests to each address
separately.

I didn't try to fix the crash in resolving as Lennart said he would
look into it, in my testing I just commented out the call flushing the
addresses.

Also, there seems to be a problem in the function adjusting the polling
interval, it doesn't consider jitter. When the jitter is large it
goes for a shorter interval instead of longer, making everything
worse. My suggestion would be to include the approach used in ntpd
(search for CLOCK_PGATE in the sources).

Miroslav Lichvar (8):
  timesyncd: check if stratum is valid
  timesyncd: fix calculation of transmit time
  timesyncd: get kernel timestamp in nanoseconds
  timesyncd: check root distance
  timesyncd: use longer PLL time constant
  timesyncd: allow two missed replies before reselecting server
  timesyncd: don't reset polling interval when reselecting server
  timesyncd: wait before reconnecting to first server

 src/timesync/timesyncd-manager.c | 91 +---
 src/timesync/timesyncd-manager.h |  2 +
 2 files changed, 69 insertions(+), 24 deletions(-)

-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 8/8] timesyncd: wait before reconnecting to first server

2014-08-27 Thread Miroslav Lichvar
When all servers are exhausted, wait for one poll interval before trying
to connect again to the first server in the list. Also, keep increasing
the polling interval to make sure a client not getting any valid replies
will not send requests to any server more frequently than is allowed by
the maximum polling interval.
---
 src/timesync/timesyncd-manager.c | 24 +++-
 src/timesync/timesyncd-manager.h |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index d1f77a8..3fd221e 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -882,6 +882,7 @@ int manager_connect(Manager *m) {
 manager_set_server_name(m, 
m-current_server_name-names_next);
 else {
 ServerName *f;
+bool restart = true;
 
 /* Our current server name list is exhausted,
  * let's find the next one to iterate. First
@@ -898,6 +899,8 @@ int manager_connect(Manager *m) {
 f = m-link_servers;
 if (!f)
 f = m-system_servers;
+else
+restart = false;
 }
 
 if (!f)
@@ -909,6 +912,25 @@ int manager_connect(Manager *m) {
 return 0;
 }
 
+if (restart  !m-exhausted_servers  
m-poll_interval_usec) {
+log_debug(Waiting after exhausting servers.);
+r = sd_event_add_time(m-event, 
m-event_retry, clock_boottime_or_monotonic(), 
now(clock_boottime_or_monotonic()) + m-poll_interval_usec, 0, 
manager_retry_connect, m);
+if (r  0) {
+log_error(Failed to create retry 
timer: %s, strerror(-r));
+return r;
+}
+
+m-exhausted_servers = true;
+
+/* Increase the polling interval */
+if (m-poll_interval_usec  
NTP_POLL_INTERVAL_MAX_SEC * USEC_PER_SEC)
+m-poll_interval_usec *= 2;
+
+return 0;
+}
+
+m-exhausted_servers = false;
+
 manager_set_server_name(m, f);
 }
 
@@ -1049,7 +1071,7 @@ static int manager_network_event_handler(sd_event_source 
*s, int fd, uint32_t re
 online = network_is_online();
 
 /* check if the client is currently connected */
-connected = m-server_socket = 0 || m-resolve_query;
+connected = m-server_socket = 0 || m-resolve_query || 
m-exhausted_servers;
 
 if (connected  !online) {
 log_info(No network connectivity, watching for changes.);
diff --git a/src/timesync/timesyncd-manager.h b/src/timesync/timesyncd-manager.h
index 1d3cc62..c7efdc5 100644
--- a/src/timesync/timesyncd-manager.h
+++ b/src/timesync/timesyncd-manager.h
@@ -41,6 +41,7 @@ struct Manager {
 LIST_HEAD(ServerName, fallback_servers);
 
 RateLimit ratelimit;
+bool exhausted_servers;
 
 /* network */
 sd_event_source *network_event_source;
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 4/8] timesyncd: check root distance

2014-08-27 Thread Miroslav Lichvar
NTPv4 servers don't reply with unsynchronized status when they lost
synchronization, they only keep increasing the root dispersion and it's
up to the client to decide at which point they no longer consider it
synchronized.

Ignore replies with root distance over 5 seconds.
---
 src/timesync/timesyncd-manager.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index 2b0580c..9b8b7d3 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -89,6 +89,9 @@
 #define NTP_FIELD_MODE(f)   ((f)  7)
 #define NTP_FIELD(l, v, m)  (((l)  6) | ((v)  3) | (m))
 
+/* Maximum acceptable root distance in seconds. */
+#define NTP_MAX_ROOT_DISTANCE   5.0
+
 /*
  * NTP timestamps are represented as a 64-bit unsigned fixed-point number,
  * in seconds relative to 0h on 1 January 1900.
@@ -128,6 +131,10 @@ struct ntp_msg {
 static int manager_arm_timer(Manager *m, usec_t next);
 static int manager_clock_watch_setup(Manager *m);
 
+static double ntp_ts_short_to_d(const struct ntp_ts_short *ts) {
+return be16toh(ts-sec) + (be16toh(ts-frac) / 65536.0);
+}
+
 static double ntp_ts_to_d(const struct ntp_ts *ts) {
 return be32toh(ts-sec) + ((double)be32toh(ts-frac) / UINT_MAX);
 }
@@ -500,6 +507,7 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 ssize_t len;
 double origin, receive, trans, dest;
 double delay, offset;
+double root_distance;
 bool spike;
 int leap_sec;
 int r;
@@ -585,6 +593,12 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 return manager_connect(m);
 }
 
+root_distance = ntp_ts_short_to_d(ntpmsg.root_delay) / 2 + 
ntp_ts_short_to_d(ntpmsg.root_dispersion);
+if (root_distance  NTP_MAX_ROOT_DISTANCE) {
+log_debug(Server has too large root distance. 
Disconnecting.);
+return manager_connect(m);
+}
+
 /* valid packet */
 m-pending = false;
 m-retry_interval = 0;
@@ -626,6 +640,7 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 mode : %u\n
 stratum  : %u\n
 precision: %.6f sec (%d)\n
+root distance: %.6f sec\n
 reference: %.4s\n
 origin   : %.3f\n
 receive  : %.3f\n
@@ -641,6 +656,7 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
   NTP_FIELD_MODE(ntpmsg.field),
   ntpmsg.stratum,
   exp2(ntpmsg.precision), ntpmsg.precision,
+  root_distance,
   ntpmsg.stratum == 1 ? ntpmsg.refid : n/a,
   origin - OFFSET_1900_1970,
   receive - OFFSET_1900_1970,
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 6/8] timesyncd: allow two missed replies before reselecting server

2014-08-27 Thread Miroslav Lichvar
After receiving a reply from the server, allow two missed replies before
switching to another server to avoid unnecessary clock hopping when
packets are getting lost in the network.
---
 src/timesync/timesyncd-manager.c | 27 ++-
 src/timesync/timesyncd-manager.h |  1 +
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index bd6bc39..9f12149 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -92,6 +92,9 @@
 /* Maximum acceptable root distance in seconds. */
 #define NTP_MAX_ROOT_DISTANCE   5.0
 
+/* Maximum number of missed replies before selecting another source. */
+#define NTP_MAX_MISSED_REPLIES  2
+
 /*
  * NTP timestamps are represented as a 64-bit unsigned fixed-point number,
  * in seconds relative to 0h on 1 January 1900.
@@ -219,15 +222,18 @@ static int manager_send_request(Manager *m) {
 return r;
 }
 
-r = sd_event_add_time(
-m-event,
-m-event_timeout,
-clock_boottime_or_monotonic(),
-now(clock_boottime_or_monotonic()) + TIMEOUT_USEC, 0,
-manager_timeout, m);
-if (r  0) {
-log_error(Failed to arm timeout timer: %s, strerror(-r));
-return r;
+m-missed_replies++;
+if (m-missed_replies  NTP_MAX_MISSED_REPLIES) {
+r = sd_event_add_time(
+m-event,
+m-event_timeout,
+clock_boottime_or_monotonic(),
+now(clock_boottime_or_monotonic()) + 
TIMEOUT_USEC, 0,
+manager_timeout, m);
+if (r  0) {
+log_error(Failed to arm timeout timer: %s, 
strerror(-r));
+return r;
+}
 }
 
 return 0;
@@ -562,6 +568,8 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 return 0;
 }
 
+m-missed_replies = 0;
+
 /* check our time cookie (we just stored nanoseconds in the fraction 
field) */
 if (be32toh(ntpmsg.origin_time.sec) != m-trans_time.tv_sec + 
OFFSET_1900_1970 ||
 be32toh(ntpmsg.origin_time.frac) != m-trans_time.tv_nsec) {
@@ -726,6 +734,7 @@ static int manager_begin(Manager *m) {
 assert_return(m-current_server_name, -EHOSTUNREACH);
 assert_return(m-current_server_address, -EHOSTUNREACH);
 
+m-missed_replies = NTP_MAX_MISSED_REPLIES;
 m-poll_interval_usec = NTP_POLL_INTERVAL_MIN_SEC * USEC_PER_SEC;
 
 server_address_pretty(m-current_server_address, pretty);
diff --git a/src/timesync/timesyncd-manager.h b/src/timesync/timesyncd-manager.h
index 2345bf8..1d3cc62 100644
--- a/src/timesync/timesyncd-manager.h
+++ b/src/timesync/timesyncd-manager.h
@@ -52,6 +52,7 @@ struct Manager {
 ServerName *current_server_name;
 ServerAddress *current_server_address;
 int server_socket;
+int missed_replies;
 uint64_t packet_count;
 sd_event_source *event_timeout;
 
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 3/8] timesyncd: get kernel timestamp in nanoseconds

2014-08-27 Thread Miroslav Lichvar
---
 src/timesync/timesyncd-manager.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index 3339606..2b0580c 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -136,10 +136,6 @@ static double ts_to_d(const struct timespec *ts) {
 return ts-tv_sec + (1.0e-9 * ts-tv_nsec);
 }
 
-static double tv_to_d(const struct timeval *tv) {
-return tv-tv_sec + (1.0e-6 * tv-tv_usec);
-}
-
 static double square(double d) {
 return d * d;
 }
@@ -500,7 +496,7 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 .msg_namelen = sizeof(server_addr),
 };
 struct cmsghdr *cmsg;
-struct timeval *recv_time;
+struct timespec *recv_time;
 ssize_t len;
 double origin, receive, trans, dest;
 double delay, offset;
@@ -543,8 +539,8 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 continue;
 
 switch (cmsg-cmsg_type) {
-case SCM_TIMESTAMP:
-recv_time = (struct timeval *) CMSG_DATA(cmsg);
+case SCM_TIMESTAMPNS:
+recv_time = (struct timespec *) CMSG_DATA(cmsg);
 break;
 }
 }
@@ -615,7 +611,7 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 origin = ts_to_d(m-trans_time) + OFFSET_1900_1970;
 receive = ntp_ts_to_d(ntpmsg.recv_time);
 trans = ntp_ts_to_d(ntpmsg.trans_time);
-dest = tv_to_d(recv_time) + OFFSET_1900_1970;
+dest = ts_to_d(recv_time) + OFFSET_1900_1970;
 
 offset = ((receive - origin) + (trans - dest)) / 2;
 delay = (dest - origin) - (trans - receive);
@@ -697,7 +693,7 @@ static int manager_listen_setup(Manager *m) {
 if (r  0)
 return -errno;
 
-r = setsockopt(m-server_socket, SOL_SOCKET, SO_TIMESTAMP, on, 
sizeof(on));
+r = setsockopt(m-server_socket, SOL_SOCKET, SO_TIMESTAMPNS, on, 
sizeof(on));
 if (r  0)
 return -errno;
 
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH 2/8] timesyncd: fix calculation of transmit time

2014-08-27 Thread Miroslav Lichvar
The kernel timestamp (recv_time) is made earlier than current time
(now_ts), use the timestamp captured before sending packet directly.
---
 src/timesync/timesyncd-manager.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index 60f39c6..3339606 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -500,7 +500,6 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
 .msg_namelen = sizeof(server_addr),
 };
 struct cmsghdr *cmsg;
-struct timespec now_ts;
 struct timeval *recv_time;
 ssize_t len;
 double origin, receive, trans, dest;
@@ -613,8 +612,7 @@ static int manager_receive_response(sd_event_source 
*source, int fd, uint32_t re
  *  The round-trip delay, d, and system clock offset, t, are defined 
as:
  *  d = (T4 - T1) - (T3 - T2) t = ((T2 - T1) + (T3 - T4)) / 2
  */
-assert_se(clock_gettime(clock_boottime_or_monotonic(), now_ts) = 0);
-origin = tv_to_d(recv_time) - (ts_to_d(now_ts) - 
ts_to_d(m-trans_time_mon)) + OFFSET_1900_1970;
+origin = ts_to_d(m-trans_time) + OFFSET_1900_1970;
 receive = ntp_ts_to_d(ntpmsg.recv_time);
 trans = ntp_ts_to_d(ntpmsg.trans_time);
 dest = tv_to_d(recv_time) + OFFSET_1900_1970;
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] timedated: add configure option to set name of controlled NTP service

2014-08-26 Thread Miroslav Lichvar
On Tue, Aug 26, 2014 at 03:27:10AM +0200, Lennart Poettering wrote:
 On Thu, 21.08.14 12:49, Miroslav Lichvar (mlich...@redhat.com) wrote:
 
  This is useful for installations where some other service than
  systemd-timesyncd is used to synchronize the system clock.
 
 What's the rationale here?

To have timedated/timedatectl managing the right NTP service on
distributions like Fedora.

 We recently removed support for configuring arbitrary NTP servers from
 timedated, see b72ddf0f4f552dd53d6404b6ddbc9f17d02b8e12. YOu patch would
 undo this change, but in a very limited way...

Yes, it's meant to be a cheap replacement for the removed functionality.
Perhaps I could prepare something better if I knew what exactly was
wrong with the old code.

 We decided to make timedated only manage timesyncd and nothing else, and
 treat all other NTP servers as real servers that when installed and
 enabled replace timesyncd. Hence: every other NTP server should really
 take precedence over timesyncd, but only timesyncd is managable via
 timedated.

The problem is timedated doesn't see or control the other NTP service.
It may report the NTP enabled status incorrectly and set-ntp false
may not actually disable NTP. Enabling NTP starts timesyncd, but after
reboot there is the other NTP service running again. This is
confusing.

 This sounds like the right thing to do, after all timesyncd
 is really the simplest option to run for clients, and hence really is
 what should be managed by GNOME.

I think GNOME should be managing the NTP service which is normally
used on the system, i.e. chronyd on Fedora.

If your suggestion is to use timesyncd by default on all systems
where systemd is included, that might not work. Even if you don't care
much about accuracy, stability or monotonicity of the system clock,
there is a problem with reliability. As timesyncd currently implements
only SNTP and not full NTP client, it doesn't compare times from
multiple servers, so it can't know when a server is broken and another
should be selected instead.

When the client is configured to use just one trusted NTP server,
that's ok. However, when public servers are used, a full NTP client is
preferred to avoid tracking a server whose clock is completely off or
is too slow/fast and is being periodically reset.

BTW, is there an agreement with Google on using their NTP servers?
From what I could find they don't seem to offer it as a public service
and there is really not much value in knowing how well are the clients
keeping time (unlike knowing what sites people visit from their DNS
requests), so I'd not be very surprised if they started to limit the
access in the future.

You may want to consider getting a vendor zone for systemd at
pool.ntp.org and use it in the default list instead (after making sure
there are no problems with frequent polling, etc.).

http://www.pool.ntp.org/en/vendors.html

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] timedated: add configure option to set name of controlled NTP service

2014-08-26 Thread Miroslav Lichvar
On Tue, Aug 26, 2014 at 02:21:10PM +0200, Lennart Poettering wrote:
 On Tue, 26.08.14 11:41, Miroslav Lichvar (mlich...@redhat.com) wrote:
  To have timedated/timedatectl managing the right NTP service on
  distributions like Fedora.
 
 I don't really think that timedated should manage an NTP server like
 ntpd/chrony. 

I'm talking about NTP clients, not servers. Writing a good NTP client
is the difficult part, making server from a client is just a matter of
binding a socket to port 123 and replying to the requests with current
time. Anyway, chronyd doesn't reply to NTP requests by default, so
think of it as an NTP client in this context.

 timedated's primary job is to be a service to GNOME and
 other DEs. But if an admin wants to upgrade to a full NTP server, then
 he should really enable/disable that with systemctl or a similar
 command.

Distributions usually have a full NTP client installed by default, why
should GNOME try to enable/disable an SNTP client that happens to be
distributed with systemd?

 The code wasn't wrong for what it was supposed to do. We just didn't
 think the complexity of it would be appropriate.

Ok, this patch to make it configurable at compilation time is
extremely simple and there is zero runtime cost, so there should be no
problem in including it, or not?

  If your suggestion is to use timesyncd by default on all systems
  where systemd is included, that might not work. Even if you don't care
  much about accuracy, stability or monotonicity of the system clock,
 
 I don't see why timesyncd would be any different regarding monotonicity
 that chrony would...

After start chronyd uses only slewing to correct the clock, but timesyncd
seems to step the clock when the offset is larger than 0.4 second and
is not a spike. With an intermittent or congested internet connection
suffering from bufferbloat it's pretty easy to get offsets larger than
that.

  there is a problem with reliability. As timesyncd currently implements
  only SNTP and not full NTP client, it doesn't compare times from
  multiple servers, so it can't know when a server is broken and another
  should be selected instead.
 
 Well, if we start comparing features, then there's also some stuff that
 timesyncd can do that chrony can't (such as getting NTP server info from
 DHCP networkd, or support for offline clock monotonicity of RTC-less
 systems). 

Ok, chronyd is usually used with NetworkManager and it relies on a
dispatcher script to get the servers from DHCP. Adding support to get
them from networkd doesn't sound like a very difficult task if it's
needed. Or could be networkd modified to write the list of the servers
to a file so any NTP client could use it?

As for systems without RTC, chronyd now has an option to restore the
time similarly to the fake-hwclock script from Debian.

  When the client is configured to use just one trusted NTP server,
  that's ok. However, when public servers are used, a full NTP client is
  preferred to avoid tracking a server whose clock is completely off or
  is too slow/fast and is being periodically reset.
 
 Well, I don't see why a Linux client should implement a full NTP server
 if no other OS does that...

Vendors of other systems (at least Apple and Microsoft) can afford to
run their own NTP servers. SNTP is probably good enough for them.

 This is really about being a *client*, and
 not trying to outsmart servers.

Outsmart servers how exactly?

  You may want to consider getting a vendor zone for systemd at
  pool.ntp.org and use it in the default list instead (after making sure
  there are no problems with frequent polling, etc.).
 
 we are not a vendor, as we don't really do products. Vendors may use
 systemd to build a product, but in that case they should use
 --with-ntp-servers= and set their own NTP servers of choice...

Did you ask them? I think they would be ok with systemd having its own
vendor zone, systemd is not that far to be considered a Linux
distribution. :)

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] timedated: add configure option to set name of controlled NTP service

2014-08-26 Thread Miroslav Lichvar
On Tue, Aug 26, 2014 at 07:50:23AM -0700, Marcel Holtmann wrote:
 and writing a good DHCP client was supposedly also really hard.
 Guess what we have done that twice now and both of our clients are
 better than everything else out there. And guess what, we did the
 same for NTP.

Are you saying timesyncd is already better than chronyd or ntpd? In
what criteria? To me it looks like the only advantage currently is the
integration with networkd. I see some trivial bugs in the code, I'll
send patches. 

 Desktop systems using a high accuracy NTP client instead of just
 using SNTP is a bad idea. Especially using NTP clients that are not
 aware of network states and/or states like Hotspot portals. Or think
 about systems using 3G/LTE connections.

An NTP client can be aware of network states too, I'm not sure why
should be SNTP preferred over NTP. On mobile networks I'd probably
want to use a client that can work well with intermittent connections,
not something using the kernel PLL as it's unstable when undersampled.

 The adjtime system call is limited by the kernel and how much it can
 adjust. I do not remember that actual details, but at some point you
 are required to skip time. Trying to emulate that in userspace is a
 bad idea. Either the kernel can do it for you or you just skip to
 the next synchronized time you have.

The adjtime() offset isn't limited, but adjtimex() in PLL mode limits
the offset to 0.5 second. chronyd controls the kernel frequency
directly and if an update is late, the next correction is compensated
for that. From what I can tell it works very well.

 Even inside ConnMan, we decided to include a SNTP client directly
 into ConnMan. In earlier days, we had ConnMan controlling ntpd's
 client. Ugly and complicated and also full of race conditions. We
 looked into Chrony and it was no better at all. We tried to write
 our own SNTP client that we control and can feed DHCP information
 into it. Worked beautifully and is still in use today and actual
 consumer products.

Ok, how exactly are you feeding the NTP client? Perhaps it's something
we could use in chronyd too.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] timesyncd: Frequent polling when no server could be reached

2014-08-22 Thread Miroslav Lichvar
On Thu, Aug 21, 2014 at 08:15:36PM +0200, Florian Lindner wrote:
 Aug 21 09:45:00 asaru systemd-timesyncd[317]: Timed out waiting for reply 
 from 216.239.32.15:123 (time1.google.com).
 Aug 21 09:45:00 asaru systemd-timesyncd[317]: Using NTP server 
 [2001:4860:4802:32::f]:123 (time1.google.com).
 Aug 21 09:45:00 asaru systemd-timesyncd[317]: Using NTP server 
 216.239.34.15:123 (time2.google.com).
 Aug 21 09:45:10 asaru systemd-timesyncd[317]: Timed out waiting for reply 
 from 216.239.34.15:123 (time2.google.com).
 Aug 21 09:45:10 asaru systemd-timesyncd[317]: Using NTP server 
 [2001:4860:4802:34::f]:123 (time2.google.com).
 Aug 21 09:45:10 asaru systemd-timesyncd[317]: Using NTP server 
 216.239.36.15:123 (time3.google.com).
 
 Polluting my log this way. Is is possible to inhibit that behavior? Maybe 
 trying a couple of times, then giving up until next network status change.

Hm, a well behaved client reduces its polling rate exponentially when
it doesn't receive a reply to avoid overloading the servers and
network congestion.

After running some tests, it seems there is an even bigger problem.
When timesyncd receives a reply saying that the server isn't
synchronized or that the client should reduce its polling rate (KOD
RATE), it selects the next server and sends a new request immediately.
When all servers are unsynchronized, this creates a burst of 10
packets several times per minute.

This really needs to be fixed. An easy solution could be to add an
exponentially increasing delay (with maximum at 2048 seconds) when
all servers were tried and switching back to the first server in the
list.

Clients increasing their polling rate when not receiving reply or
receiving a reply they don't like is the biggest problem the
pool.ntp.org operators have to deal with.

BTW, I was getting segfaults with current git in sd_resolve_getaddrinfo()
in manager_connect() when doing the tests, removing the
server_name_flush_addresses() call seems to fix it.

 Another question I have is about the NTP status output of timedatectl.
 
 Right now (with ntpd running) it says:
 
 NTP enabled: yes
 NTP synchronized: no
  
 I suppose it need some more uptime than the 11 minutes I have currently?

Possibly, ntpd needs to clear the STA_UNSYNC flag in adjtimex to mark
the clock as synchronized.

 When I had timesyncd active it said NTP no, though I had a ntp client 
 runnung, albeit definitly unsynchronized.
 
 Version is 215

With 215, you need to remove all files in /usr/lib/systemd/ntp-units.d/
except the one that lists the timesyncd service to select it for
timedated. In 216 the ntp-units.d directory is ignored and timedated
always controls timesyncd. I think it would be nice if this was
configurable at least at compile time and I sent a patch for that
yesterday.

-- 
Miroslav Lichvar
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] timedated: add configure option to set name of controlled NTP service

2014-08-21 Thread Miroslav Lichvar
This is useful for installations where some other service than
systemd-timesyncd is used to synchronize the system clock.
---
 configure.ac |  9 +
 src/timedate/timedated.c | 10 +-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index 18b7198..d9f95d9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -948,6 +948,15 @@ if test x$enable_timedated != xno; then
 fi
 AM_CONDITIONAL(ENABLE_TIMEDATED, [test $have_timedated = yes])
 
+AC_ARG_WITH(ntp-service,
+AS_HELP_STRING([--with-ntp-service=NTPSERVICE],
+[NTP service controlled by timedated]),
+[NTP_SERVICE=$withval],
+[NTP_SERVICE=systemd-timesyncd.service])
+
+AC_DEFINE_UNQUOTED(NTP_SERVICE, [$NTP_SERVICE], [NTP service controlled by 
timedated])
+AC_SUBST(NTP_SERVICE)
+
 # 
--
 have_timesyncd=no
 AC_ARG_ENABLE(timesyncd, AS_HELP_STRING([--disable-timesyncd], [disable 
timesync daemon]))
diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index fa3f947..cdd16e6 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -198,7 +198,7 @@ static int context_read_ntp(Context *c, sd_bus *bus) {
 error,
 reply,
 s,
-systemd-timesyncd.service);
+NTP_SERVICE);
 
 if (r  0) {
 if (sd_bus_error_has_name(error, SD_BUS_ERROR_FILE_NOT_FOUND) 
||
@@ -236,7 +236,7 @@ static int context_start_ntp(Context *c, sd_bus *bus, 
sd_bus_error *error) {
 error,
 NULL,
 ss,
-systemd-timesyncd.service,
+NTP_SERVICE,
 replace);
 else
 r = sd_bus_call_method(
@@ -248,7 +248,7 @@ static int context_start_ntp(Context *c, sd_bus *bus, 
sd_bus_error *error) {
 error,
 NULL,
 ss,
-systemd-timesyncd.service,
+NTP_SERVICE,
 replace);
 
 if (r  0) {
@@ -282,7 +282,7 @@ static int context_enable_ntp(Context*c, sd_bus *bus, 
sd_bus_error *error) {
 error,
 NULL,
 asbb, 1,
-systemd-timesyncd.service,
+NTP_SERVICE,
 false, true);
 else
 r = sd_bus_call_method(
@@ -294,7 +294,7 @@ static int context_enable_ntp(Context*c, sd_bus *bus, 
sd_bus_error *error) {
 error,
 NULL,
 asb, 1,
-systemd-timesyncd.service,
+NTP_SERVICE,
 false);
 
 if (r  0) {
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel