It is possible that named NTP peers may cause XNTP (aka the reference
implementation) to languish forever in a resolver "jail" that prevents
it from ever successfully terminating or synchronizing time. In this
state, the reference implementation can be found timed out in its
ntp_intres handler, attempting to resolve a peer by name
(e.g. pool.ntp.org), leaving two NTP processes running:
154 ? S 0:00 /usr/sbin/ntpd -g -q -c /tmp/connman.ntp.conf_6W
158 ? S 0:00 /usr/sbin/ntpd -g -q -c /tmp/connman.ntp.conf_6W
In this state, the next time connman attempts time synchronization, it
will fails due to the above pended processes.
Consequently, with this patch, we go through the extra effort of
pre-resolving named NTP peers in the plug-in, greatly increasing the
likelihood that the NTP daemon will synchronize time quickly and
successfully.
For resolutions against names that are pools of time servers, we
only use the first resolved result.
We also resolve on-the-fly for every synchronization request to
achieve an effective round-robin load-balancing against those pools
that are dynamic and resolve to a varying set of time servers on each
resolution request.
This patch also ensures that all dynamically-allocated resources are
properly cleaned up.
---
plugins/ntpd.c | 356 ++++++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 284 insertions(+), 72 deletions(-)
diff --git a/plugins/ntpd.c b/plugins/ntpd.c
index 3ee2ad8..8255cfb 100644
--- a/plugins/ntpd.c
+++ b/plugins/ntpd.c
@@ -19,6 +19,29 @@
*
*/
+/*
+ * Long-term architectural plans involve the Connection Manager having
+ * a lightweight, internal SNTP client just as it has with DHCP.
+ *
+ * However, until that plan is reached, we run a compatible NTP daemon
+ * in one-shot mode.
+ *
+ * It is possible that named NTP peers cause XNTP (aka the reference
+ * implementation) to languish forever in a resolver "jail" that
+ * prevents it from ever successfully terminating or synchronizing
+ * time. Consequently, we go through the extra effort of pre-resolving
+ * named NTP peers here, greatly increasing the likelihood that the
+ * daemon will synchronize time quickly and successfully.
+ *
+ * For resolutions against names that are pools of time servers, we
+ * only use the first resolved result.
+ *
+ * We also resolve on-the-fly for every synchronization request to
+ * achieve an effective round-robin load-balancing against those pools
+ * that are dynamic and resolve to a varying set of time servers on each
+ * resolution request.
+ */
+
#ifdef HAVE_CONFIG_H
#include <config.h>
#endif
@@ -36,13 +59,7 @@
#include <connman/timeserver.h>
#include <connman/driver.h>
#include <connman/log.h>
-
-/*
- * The peers list are the peers currently added to a running ntpd,
- * while pending_peers are the one appended but not used by ntpd yet.
- */
-static GList *peers = NULL;
-static GList *pending_peers = NULL;
+#include <gweb/gresolv.h>
#define NTPD_PORT 123
@@ -51,12 +68,29 @@ struct ntpd_peer {
gint refcount;
};
-struct ntpdate_task {
- struct connman_task *task;
- gint conf_fd;
- char *conf_path;
+struct ntpd_data {
+ int error;
+ struct connman_task *task;
+ struct {
+ gint descriptor;
+ char *path;
+ } configuration;
+ /*
+ * The current peers list are the peers currently added to a
+ * running ntpd, while pending are the one appended but not
+ * used by ntpd yet.
+ */
+ struct {
+ GList *current;
+ GList *pending;
+ GResolv *resolver;
+ guint unresolved;
+ guint resolved;
+ } peers;
};
+static struct ntpd_data *ntpd_data = NULL;
+
static struct ntpd_peer *find_peer(GList *peer_list, const char* server)
{
GList *list;
@@ -82,7 +116,26 @@ static void remove_peer(GList *peer_list, struct ntpd_peer
*peer)
peer_list = g_list_remove(peer_list, peer);
}
-static connman_bool_t ntpd_running(void)
+static void remove_peers(GList *peer_list)
+{
+ GList *list;
+ struct ntpd_peer *peer;
+
+ for (list = peer_list; list; list = list->next) {
+ peer = list->data;
+
+ remove_peer(peer_list, peer);
+ }
+}
+
+static connman_bool_t ntpd_has_peers(const struct ntpd_data *data)
+{
+ return (data != NULL) &&
+ ((g_list_length(data->peers.current) != 0) ||
+ (g_list_length(data->peers.pending) != 0));
+}
+
+static connman_bool_t ntpd_running(const struct ntpd_data *data)
{
int sock;
connman_bool_t ret;
@@ -109,98 +162,181 @@ static connman_bool_t ntpd_running(void)
return ret;
}
+static void ntpd_task_free(struct ntpd_data *data)
+{
+ if (data == NULL)
+ return;
+
+ unlink(data->configuration.path);
+ g_free(data->configuration.path);
+ data->configuration.path = NULL;
+
+ connman_task_destroy(data->task);
+ data->task = NULL;
+}
+
static void ntpdate_died(struct connman_task *task,
int exit_code, void *user_data)
{
- struct ntpdate_task *ntpdate = user_data;
+ struct ntpd_data *ntpd = (struct ntpd_data *)user_data;
- DBG("");
+ DBG("exit_code %d user_data %p", exit_code, user_data);
- unlink(ntpdate->conf_path);
- g_free(ntpdate->conf_path);
- connman_task_destroy(ntpdate->task);
+ ntpd_task_free(ntpd);
}
-static void ntpdate_add_peer(struct ntpdate_task *ntpdate, char *peer)
+static void peer_result(GResolvResultStatus status,
+ char **results, gpointer user_data)
{
+ struct ntpd_data *ntpd = (struct ntpd_data *)user_data;
FILE *conf_file;
+ int err;
- DBG("%s", peer);
+ DBG("status %d", status);
- conf_file = fdopen(ntpdate->conf_fd, "a+");
- if (conf_file == NULL) {
- connman_error("fdopen failed");
- return;
+ ntpd->peers.unresolved--;
+
+ if (status == G_RESOLV_RESULT_STATUS_SUCCESS) {
+ if (results == NULL || g_strv_length(results) == 0)
+ return;
+
+ conf_file = fdopen(ntpd->configuration.descriptor, "a+");
+ if (conf_file == NULL) {
+ connman_error("fdopen failed");
+ return;
+ }
+
+ if (*results != NULL) {
+ DBG("resolved %s", *results);
+ fprintf(conf_file, "server %s iburst\n", *results);
+ ntpd->peers.resolved++;
+ }
+
+ fclose(conf_file);
+
+ close(ntpd->configuration.descriptor);
}
- fprintf(conf_file, "server %s iburst\n", peer);
+ if (ntpd->peers.unresolved == 0) {
+ if (ntpd->peers.resolved > 0) {
+ err = connman_task_run(ntpd->task,
+ ntpdate_died,
+ ntpd_data,
+ NULL,
+ NULL,
+ NULL);
+
+ if (err < 0) {
+ connman_error("Failed to run %s: %d: %s",
+ NTPD, -err, strerror(-err));
+ }
+ } else {
+ ntpd_task_free(ntpd);
+ }
+ }
+}
+
+static void ntpdate_add_peer(struct ntpd_data *data, const char *peer)
+{
+ connman_info("Resolving NTP peer %s", peer);
- fclose(conf_file);
+ g_resolv_lookup_hostname(data->peers.resolver,
+ peer,
+ peer_result,
+ data);
}
-static int ntpdate(void)
+static void ntpd_stop(struct ntpd_data *data)
{
- int err;
+ if (data == NULL)
+ return;
+
+ if (data->task == NULL)
+ return;
+
+ connman_task_stop(data->task);
+
+ ntpd_task_free(data);
+}
+
+static gboolean ntpd_launch(gpointer user_data)
+{
+ struct ntpd_data *ntpd = (struct ntpd_data *)user_data;
GError *g_err;
GList *list;
struct ntpd_peer *peer;
- struct ntpdate_task *ntpdate;
+ guint current, pending;
DBG("");
- ntpdate = g_try_new0(struct ntpdate_task, 1);
- if (ntpdate == NULL)
- return -ENOMEM;
-
- /* ntpdate is deprecated, we use ntpd -q instead */
- ntpdate->task = connman_task_create(NTPD);
- if (ntpdate->task == NULL) {
- err = -ENOMEM;
+ /* ntpdate is deprecated, we use ntpd -g -q instead */
+ ntpd->task = connman_task_create(NTPD);
+ if (ntpd->task == NULL) {
+ ntpd->error = -ENOMEM;
goto error_task;
}
- connman_task_add_argument(ntpdate->task, "-g", NULL);
- connman_task_add_argument(ntpdate->task, "-q", NULL);
+ connman_task_add_argument(ntpd->task, "-g", NULL);
+ connman_task_add_argument(ntpd->task, "-q", NULL);
/* The servers are added through a temp configuration file */
- ntpdate->conf_fd = g_file_open_tmp("connman.ntp.conf_XXXXXX",
- &ntpdate->conf_path, &g_err);
- if (ntpdate->conf_fd == -1) {
- err = g_err->code;
+ ntpd->configuration.descriptor =
+ g_file_open_tmp("connman.ntp.conf_XXXXXX",
+ &ntpd->configuration.path, &g_err);
+ if (ntpd->configuration.descriptor == -1) {
+ ntpd->error = g_err->code;
g_free(g_err);
goto error_open;
}
- connman_task_add_argument(ntpdate->task, "-c", ntpdate->conf_path);
+ connman_task_add_argument(ntpd->task, "-c", ntpd->configuration.path);
- DBG("conf path %s", ntpdate->conf_path);
+ DBG("conf path %s", ntpd->configuration.path);
- for (list = pending_peers; list; list = list->next) {
+ pending = g_list_length(ntpd->peers.pending);
+ current = g_list_length(ntpd->peers.current);
+
+ ntpd->peers.unresolved = pending + current;
+ ntpd->peers.resolved = 0;
+
+ for (list = ntpd->peers.pending; list; list = list->next) {
peer = list->data;
- ntpdate_add_peer(ntpdate, peer->server);
+ ntpdate_add_peer(ntpd, peer->server);
}
- for (list = peers; list; list = list->next) {
+ for (list = ntpd->peers.current; list; list = list->next) {
peer = list->data;
- ntpdate_add_peer(ntpdate, peer->server);
+ ntpdate_add_peer(ntpd, peer->server);
}
- close(ntpdate->conf_fd);
+ return FALSE;
- return connman_task_run(ntpdate->task, ntpdate_died, ntpdate,
- NULL, NULL, NULL);
error_open:
- connman_task_destroy(ntpdate->task);
+ connman_task_destroy(ntpd->task);
+ ntpd->task = NULL;
error_task:
- g_free(ntpdate);
+ return FALSE;
+}
+
+static int ntpdate(struct ntpd_data *data)
+{
+ DBG("");
- return err;
+ ntpd_launch(data);
+
+ if (data->error < 0) {
+ connman_error("Failed to launch %s: %d: %s",
+ NTPD, -data->error, strerror(-data->error));
+ return data->error;
+ }
+
+ return 0;
}
-static int ntpd_add_peer(char *peer)
+static int ntpd_add_peer(const char *peer)
{
DBG("%s", peer);
@@ -214,18 +350,17 @@ static void ntpd_sync(void)
DBG("");
- if (g_list_length(pending_peers) == 0 &&
- g_list_length(peers) == 0)
+ if (!ntpd_has_peers(ntpd_data))
return;
- if (!ntpd_running()) {
- ntpdate();
+ if (!ntpd_running(ntpd_data)) {
+ ntpdate(ntpd_data);
return;
}
/* TODO Grab ntp keys path */
- list = g_list_first(pending_peers);
+ list = g_list_first(ntpd_data->peers.pending);
while(list) {
struct ntpd_peer *peer = list->data;
@@ -233,11 +368,13 @@ static void ntpd_sync(void)
if (err)
continue;
- peers = g_list_prepend(peers, peer);
+ ntpd_data->peers.current =
+ g_list_prepend(ntpd_data->peers.current, peer);
list = g_list_next(list);
- pending_peers = g_list_remove(pending_peers, peer);
+ ntpd_data->peers.pending =
+ g_list_remove(ntpd_data->peers.pending, peer);
};
}
@@ -245,13 +382,16 @@ static int ntpd_append(const char *server)
{
struct ntpd_peer *peer;
- DBG("");
+ connman_info("Adding NTP server %s", server);
+
+ if (ntpd_data == NULL)
+ return -ENODEV;
if (server == NULL)
return 0;
- if ((peer = find_peer(pending_peers, server)) ||
- (peer = find_peer(peers, server))) {
+ if ((peer = find_peer(ntpd_data->peers.pending, server)) ||
+ (peer = find_peer(ntpd_data->peers.current, server))) {
g_atomic_int_inc(&peer->refcount);
return 0;
}
@@ -268,7 +408,8 @@ static int ntpd_append(const char *server)
peer->refcount = 1;
- pending_peers = g_list_prepend(pending_peers, peer);
+ ntpd_data->peers.pending = g_list_prepend(ntpd_data->peers.pending,
+ peer);
return 0;
}
@@ -277,25 +418,28 @@ static int ntpd_remove(const char *server)
{
struct ntpd_peer *peer;
- DBG("");
+ connman_info("Removing NTP server %s", server);
+
+ if (ntpd_data == NULL)
+ return -ENODEV;
if (server == NULL)
return 0;
- peer = find_peer(peers, server);
+ peer = find_peer(ntpd_data->peers.current, server);
if (peer == NULL)
goto remove;
- remove_peer(peers, peer);
+ remove_peer(ntpd_data->peers.current, peer);
remove:
/* TODO: send ntpd remove command */
- peer = find_peer(pending_peers, server);
+ peer = find_peer(ntpd_data->peers.pending, server);
if (peer == NULL)
return 0;
- remove_peer(pending_peers, peer);
+ remove_peer(ntpd_data->peers.pending, peer);
return 0;
}
@@ -308,13 +452,81 @@ static struct connman_timeserver_driver ntpd_driver = {
.sync = ntpd_sync,
};
+static struct ntpd_data *ntpd_alloc(void)
+{
+ struct ntpd_data *temp;
+
+ temp = g_try_new0(struct ntpd_data, 1);
+ if (temp == NULL)
+ goto error;
+
+ temp->peers.resolver = g_resolv_new(0);
+ if (temp->peers.resolver == NULL)
+ goto error_free;
+
+ return temp;
+
+ error_free:
+ g_free(temp);
+
+ error:
+ return NULL;
+}
+
+static void ntpd_destroy(struct ntpd_data *data)
+{
+ if (data == NULL)
+ return;
+
+ ntpd_stop(data);
+
+ if (data->peers.resolver != NULL) {
+ g_resolv_unref(data->peers.resolver);
+ data->peers.resolver = NULL;
+ }
+
+ if (data->peers.current != NULL) {
+ remove_peers(data->peers.current);
+ data->peers.current = NULL;
+ }
+
+ if (data->peers.current != NULL) {
+ remove_peers(data->peers.pending);
+ data->peers.pending = NULL;
+ }
+
+ g_free(data);
+}
+
static int ntpd_init(void)
{
- return connman_timeserver_driver_register(&ntpd_driver);
+ struct ntpd_data *temp;
+ int err;
+
+ DBG("");
+
+ temp = ntpd_alloc();
+ if (temp == NULL)
+ return -ENOMEM;
+
+ err = connman_timeserver_driver_register(&ntpd_driver);
+ if (err < 0)
+ return err;
+
+ ntpd_data = temp;
+
+ return 0;
}
static void ntpd_exit(void)
{
+ DBG("ntpd_data %p", ntpd_data);
+
+ if (ntpd_data != NULL) {
+ ntpd_destroy(ntpd_data);
+ ntpd_data = NULL;
+ }
+
connman_timeserver_driver_unregister(&ntpd_driver);
}
--
1.7.3.5
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
