Send connman mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. Fix NTP behaviour in Connman  (Zolt?n B?sz?rm?nyi)
   2. Fix NTP behaviour in Connman (Zolt?n B?sz?rm?nyi)
   3. Fix NTP behaviour in Connman (Zolt?n B?sz?rm?nyi)
   4.  (Zolt?n B?sz?rm?nyi)
   5.  (Zolt?n B?sz?rm?nyi)
   6. [PATCH 1/2] ntp: Add option TimeUpdateThreshold
      (Zolt?n B?sz?rm?nyi)
   7. [PATCH 2/2] ntp: Don't use the gateway address as a
      timeserver (Zolt?n B?sz?rm?nyi)
   8. Re: [PATCH 1/2] ntp: Add option TimeUpdateThreshold (Patrik Flykt)
   9. Re: [PATCH 0/2] Extend NTP for exotic use cases (Patrik Flykt)


----------------------------------------------------------------------

Message: 1
Date: Tue,  8 Mar 2016 11:53:41 +0100
From: Zolt?n B?sz?rm?nyi <[email protected]>
To: [email protected]
Subject: Fix NTP behaviour in Connman 
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8


Hi,

we are using Connman with a Yocto-derived custom OS.
However, we also found some problems caused by hardwired parameters
in the NTP functionality.

These problems are:

1. The 0.128 seconds threshold for using settimeofday() instead of
   adjtime() is too small. We found that some machines have bad
   RTCs that shift time more than the hardcoded value under the NTP
   polling period.

   Let's make it configurable because a single hardcoded value won't
   satisfy everyone.

2. Although the appliances we ship are configured to use our own company
   NTP server but the gateway address is also added unconditionally
   to the list of timeservers by Connman. Since at clients' site the
   DHCP servers are not under our control and in most cases the DHCP
   server is a router which also serves as an NTP server, this
   behaviour is not good. Either the NTP servers are manually configured
   or they are advertised by the DHCP. No need to add the gateway
   IP address as a timeserver, especially in expert cases when the
   gateway is not the DHCP server and is not an NTP server, either.

Please, review and consider including these patches into 1.32.

Best regards,
Zolt?n B?sz?rm?nyi



------------------------------

Message: 2
Date: Tue,  8 Mar 2016 11:53:42 +0100
From: Zolt?n B?sz?rm?nyi <[email protected]>
To: [email protected]
Subject: Fix NTP behaviour in Connman
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8

GIT: 
GIT:  
GIT: [PATCH 1/2] ntp: Add option TimeUpdateThreshold
GIT: [PATCH 2/2] ntp: Don't use the gateway address as a timeserver

Hi,

we are using Connman with a Yocto-derived custom OS.
However, we also found some problems caused by hardwired parameters
in the NTP functionality.

These problems are:

1. The 0.128 seconds threshold for using settimeofday() instead of
   adjtime() is too small. We found that some machines have bad
   RTCs that shift time more than the hardcoded value under the NTP
   polling period.

   Let's make it configurable because a single hardcoded value won't
   satisfy everyone.

2. Although the appliances we ship are configured to use our own company
   NTP server but the gateway address is also added unconditionally
   to the list of timeservers by Connman. Since at clients' site the
   DHCP servers are not under our control and in most cases the DHCP
   server is a router which also serves as an NTP server, this
   behaviour is not good. Either the NTP servers are manually configured
   or they are advertised by the DHCP. No need to add the gateway
   IP address as a timeserver, especially in expert cases when the
   gateway is not the DHCP server and is not an NTP server, either.

Please, review and consider including these patches into 1.32.

Best regards,
Zolt?n B?sz?rm?nyi



------------------------------

Message: 3
Date: Tue,  8 Mar 2016 11:53:43 +0100
From: Zolt?n B?sz?rm?nyi <[email protected]>
To: [email protected]
Subject: Fix NTP behaviour in Connman
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8


Hi,

we are using Connman with a Yocto-derived custom OS.
However, we also found some problems caused by hardwired parameters
in the NTP functionality.

These problems are:

1. The 0.128 seconds threshold for using settimeofday() instead of
   adjtime() is too small. We found that some machines have bad
   RTCs that shift time more than the hardcoded value under the NTP
   polling period.

   Let's make it configurable because a single hardcoded value won't
   satisfy everyone.

2. Although the appliances we ship are configured to use our own company
   NTP server but the gateway address is also added unconditionally
   to the list of timeservers by Connman. Since at clients' site the
   DHCP servers are not under our control and in most cases the DHCP
   server is a router which also serves as an NTP server, this
   behaviour is not good. Either the NTP servers are manually configured
   or they are advertised by the DHCP. No need to add the gateway
   IP address as a timeserver, especially in expert cases when the
   gateway is not the DHCP server and is not an NTP server, either.

Please, review and consider including these patches into 1.32.

Best regards,
Zolt?n B?sz?rm?nyi



------------------------------

Message: 4
Date: Tue,  8 Mar 2016 11:53:44 +0100
From: Zolt?n B?sz?rm?nyi <[email protected]>
To: [email protected]
Message-ID: <[email protected]>

GIT: [PATCH 1/2] ntp: Add option TimeUpdateThreshold
GIT: [PATCH] ntp: Don't use the gateway address as a timeserver


------------------------------

Message: 5
Date: Tue,  8 Mar 2016 11:53:45 +0100
From: Zolt?n B?sz?rm?nyi <[email protected]>
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8



------------------------------

Message: 6
Date: Tue,  8 Mar 2016 11:53:46 +0100
From: Zolt?n B?sz?rm?nyi <[email protected]>
To: [email protected]
Subject: [PATCH 1/2] ntp: Add option TimeUpdateThreshold
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8

From: B?sz?rm?nyi Zolt?n <[email protected]>

Add option TimeUpdateThreshold to make the threshold for using
settimeofday() vs adjtime() configurable. The RTC in some machines
is so bad that with the current hardcoded 0.128 seconds, connman
occasionally decides to use settimeofday() instead of adjtime().

v2:

The configuration value is in /etc/connman/main.conf instead of
in /var/lib/connman/settings.

Signed-off-by: Zolt?n B?sz?rm?nyi <[email protected]>

---
 include/setting.h |  1 +
 src/main.c        | 20 ++++++++++++++++++++
 src/main.conf     |  7 +++++++
 src/ntp.c         |  6 +++---
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/include/setting.h b/include/setting.h
index a882021..df87d05 100644
--- a/include/setting.h
+++ b/include/setting.h
@@ -29,6 +29,7 @@ extern "C" {
 #endif
 
 bool connman_setting_get_bool(const char *key);
+double connman_setting_get_double(const char *key);
 char **connman_setting_get_string_list(const char *key);
 unsigned int *connman_setting_get_uint_list(const char *key);
 
diff --git a/src/main.c b/src/main.c
index e46fa7b..2cfccc0 100644
--- a/src/main.c
+++ b/src/main.c
@@ -74,6 +74,7 @@ static struct {
        char **tethering_technologies;
        bool persistent_tethering_mode;
        bool enable_6to4;
+       double time_update_threshold;
 } connman_settings  = {
        .bg_scan = true,
        .pref_timeservers = NULL,
@@ -88,6 +89,7 @@ static struct {
        .tethering_technologies = NULL,
        .persistent_tethering_mode = false,
        .enable_6to4 = false,
+       .time_update_threshold = 0.128,
 };
 
 #define CONF_BG_SCAN                    "BackgroundScanning"
@@ -103,6 +105,7 @@ static struct {
 #define CONF_TETHERING_TECHNOLOGIES      "TetheringTechnologies"
 #define CONF_PERSISTENT_TETHERING_MODE  "PersistentTetheringMode"
 #define CONF_ENABLE_6TO4                "Enable6to4"
+#define CONF_TIME_UPDATE_THRESHOLD     "TimeUpdateThreshold"
 
 static const char *supported_options[] = {
        CONF_BG_SCAN,
@@ -118,6 +121,7 @@ static const char *supported_options[] = {
        CONF_TETHERING_TECHNOLOGIES,
        CONF_PERSISTENT_TETHERING_MODE,
        CONF_ENABLE_6TO4,
+       CONF_TIME_UPDATE_THRESHOLD,
        NULL
 };
 
@@ -236,6 +240,7 @@ static void parse_config(GKeyFile *config)
 {
        GError *error = NULL;
        bool boolean;
+       double doubleval;
        char **timeservers;
        char **interfaces;
        char **str_list;
@@ -365,6 +370,13 @@ static void parse_config(GKeyFile *config)
                connman_settings.enable_6to4 = boolean;
 
        g_clear_error(&error);
+
+       doubleval = g_key_file_get_double(config, "General",
+                                       CONF_TIME_UPDATE_THRESHOLD, &error);
+       if (!error)
+               connman_settings.time_update_threshold = doubleval;
+
+       g_clear_error(&error);
 }
 
 static int config_init(const char *file)
@@ -545,6 +557,14 @@ bool connman_setting_get_bool(const char *key)
        return false;
 }
 
+double connman_setting_get_double(const char *key)
+{
+       if (g_str_equal(key, CONF_TIME_UPDATE_THRESHOLD))
+               return connman_settings.time_update_threshold;
+
+       return 0.0;
+}
+
 char **connman_setting_get_string_list(const char *key)
 {
        if (g_str_equal(key, CONF_PREF_TIMESERVERS))
diff --git a/src/main.conf b/src/main.conf
index eb352fb..a67a702 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -101,3 +101,10 @@
 # quality. See RFC6343. Default value is false (as recommended by RFC6343
 # section 4.1).
 # Enable6to4 = false
+
+# The NTP implementation in Connman has the ability to set the time
+# immediately, as opposed to the traditional ntpdate/ntpd split.
+# This value determines the threshold (in seconds) over which to set
+# the time immediately instead of making the clock slower or faster.
+# Default value is 0.128 s
+# TimeUpdateThreshold = 0.128
diff --git a/src/ntp.c b/src/ntp.c
index dd246eb..0d456a3 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -66,8 +66,6 @@ struct ntp_msg {
 
 #define OFFSET_1900_1970  2208988800UL /* 1970 - 1900 in seconds */
 
-#define STEPTIME_MIN_OFFSET  0.128
-
 #define LOGTOD(a)  ((a) < 0 ? 1. / (1L << -(a)) : 1L << (int)(a))
 
 #define NTP_SEND_TIMEOUT       2
@@ -246,6 +244,7 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv,
        struct ntp_msg *msg = base;
        double m_delta, org, rec, xmt, dst;
        double delay, offset;
+       double threshold;
        static guint transmit_delay;
 
        if (len < sizeof(*msg)) {
@@ -338,7 +337,8 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv,
 
        connman_info("ntp: time slew %+.6f s", offset);
 
-       if (offset < STEPTIME_MIN_OFFSET && offset > -STEPTIME_MIN_OFFSET) {
+       threshold = connman_setting_get_double("TimeUpdateThreshold");
+       if (offset < threshold && offset > -threshold) {
                struct timeval adj;
 
                adj.tv_sec = (long) offset;
-- 
2.5.0



------------------------------

Message: 7
Date: Tue,  8 Mar 2016 11:53:47 +0100
From: Zolt?n B?sz?rm?nyi <[email protected]>
To: [email protected]
Subject: [PATCH 2/2] ntp: Don't use the gateway address as a
        timeserver
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8

From: B?sz?rm?nyi Zolt?n <[email protected]>

Don't use the gateway IP address as a timeserver. It is either
already pulled in from the list of NTP timeservers advertised
through DHCP or the NTP server is configured manually.
There is no sane use case to add the gateway address as NTP.

Signed-off-by: Zolt?n B?sz?rm?nyi <[email protected]>

---
 src/timeserver.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/src/timeserver.c b/src/timeserver.c
index f0d33e5..72e6ea9 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -204,17 +204,6 @@ GSList *__connman_timeserver_get_all(struct 
connman_service *service)
        for (i = 0; service_ts && service_ts[i]; i++)
                list = __connman_timeserver_add_list(list, service_ts[i]);
 
-       network = __connman_service_get_network(service);
-       if (network) {
-               index = connman_network_get_index(network);
-               service_gw = __connman_ipconfig_get_gateway_from_index(index,
-                       CONNMAN_IPCONFIG_TYPE_ALL);
-
-               /* Then add Service Gateway to the list */
-               if (service_gw)
-                       list = __connman_timeserver_add_list(list, service_gw);
-       }
-
        /* Then add Global Timeservers to the list */
        timeservers = load_timeservers();
 
-- 
2.5.0



------------------------------

Message: 8
Date: Tue, 08 Mar 2016 13:36:15 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 1/2] ntp: Add option TimeUpdateThreshold
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"

On Tue, 2016-03-08 at 10:11 +0100, [email protected] wrote:
> Hi,
> 
> > I don't think such a short interval was the intention here. I myself was
> > in the belief that it was 2 seconds. Need to have a look at related
> > patches to see if it has broken at some point.
> > 
> > I'm not a very big fan of adding more configuration variables if finding
> > a reasonable value fixes the case.
> 
> I don't think you can find a value that satisfies anyone.

What is the delta you're seeing here? In the log you can find something
like 'ntp: time slew +0.007760 s', and running with '-d src/ntp.c' gives
even more details.

You said RTC in your question, but you actually meant system time? If
the system clock jumps that much, there is a pretty good chance
something else has been going wrong as well. If the round trip time in
the network is large, then the time computed will fall out of this range
and the system clock be set to its computed value. Notice also that
ConnMan only sets system time, it does not touch any hw RTCs.

Adjusting the clock does not make much sense if the delta starts to be a
few seconds. With the clock for example 4 seconds off and a max
adjustment of 0.5 ms/s, the whole adjustment procedure will take a bit
more than 2 hours. This again starts to be about the maximum that can be
"reasonably" tolerated on a system. For example systemd-networkd uses
+/- 0.4 seconds, ntpd uses also 128 ms when deciding whether to adjust
the rate of the clock or set it.

> I would personally set it to 30 seconds, but e.g. one of my bosses
> demanded to set it to a year to NEVER use settimeofday, to mimic
> the ntpdate/ntpd split behaviour.

See the TimeUpdates property of the Clock API, doc/clock-api.txt to
disable this. With that it can be left as some other application's
problem to deal with the system clock.

Cheers,

        Patrik



------------------------------

Message: 9
Date: Tue, 08 Mar 2016 13:39:40 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 0/2] Extend NTP for exotic use cases
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"


        Hi,

On Tue, 2016-03-08 at 09:59 +0100, [email protected] wrote:

> > Hmm. Why can't you just set Timeservers.Configuration for the service in
> > question? Or, if it is provisioned, Timeservers in the .config file?
> 
> Timeservers = ... is set in wired.config and despite that, the DHCP server
> address (which is also the gateway) still occurs in the list of timeservers.
> 
> Apparently, this occurs because the network gateway address is added
> unconditionally to the list of timeservers.
> 
> This patch would solve this part:
> 
> diff --git a/src/timeserver.c b/src/timeserver.c
> index f0d33e5..72e6ea9 100644
> --- a/src/timeserver.c
> +++ b/src/timeserver.c
> @@ -204,17 +204,6 @@ GSList *__connman_timeserver_get_all(struct 
> connman_service *service)
>  for (i = 0; service_ts && service_ts[i]; i++)
>  list = __connman_timeserver_add_list(list, service_ts[i]);
>  
> - network = __connman_service_get_network(service);
> - if (network) {
> - index = connman_network_get_index(network);
> - service_gw = __connman_ipconfig_get_gateway_from_index(index,
> - CONNMAN_IPCONFIG_TYPE_ALL);
> -
> - /* Then add Service Gateway to the list */
> - if (service_gw)
> - list = __connman_timeserver_add_list(list, service_gw);
> - }
> -
>  /* Then add Global Timeservers to the list */
>  timeservers = load_timeservers();
>  
> 
> Why is this done in the first place?

As a backup, should a badly implemented/configured home router decide to
run a NTP server but forget to tell it to its DHCP clients. So this one
needs one more condition, add the gateway to the list if and only if it
is not in the set of timeservers given already by DHCP.

Cheers,

        Patrik



------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 5, Issue 10
**************************************

Reply via email to