ntpd: improve postponed hostname resolution
Run the namelookup from the main loop so a misspelled first ntp server
name does not block everything forever.
This fixes the following situation which would block forever:
$ sudo ./busybox ntpd -dn -p foobar -p pool.ntp.org
ntpd: bad address 'foobar'
ntpd: bad address 'foobar'
ntpd: bad address 'foobar'
...
New behavior:
ntpd: bad address 'foobar'
ntpd: sending query to 137.190.2.4
ntpd: reply from 137.190.2.4: offset:-1.009775 delay:0.175550 status:0x24 strat:1 refid:0x00535047 rootdelay:0.000000 reach:0x01
ntpd: sending query to 137.190.2.4
ntpd: reply from 137.190.2.4: offset:-1.009605 delay:0.175461 status:0x24 strat:1 refid:0x00535047 rootdelay:0.000000 reach:0x03
ntpd: sending query to 137.190.2.4
ntpd: reply from 137.190.2.4: offset:-1.005327 delay:0.167027 status:0x24 strat:1 refid:0x00535047 rootdelay:0.000000 reach:0x07
ntpd: sending query to 137.190.2.4
ntpd: bad address 'foobar'
ntpd: reply from 137.190.2.4: offset:-1.046349 delay:0.248705 status:0x24 strat:1 refid:0x00535047 rootdelay:0.000000 reach:0x0f
This patch is based on Kaarle Ritvanens work.
http://lists.busybox.net/pipermail/busybox/2016-May/084197.html
function old new delta
ntpd_main 1061 1079 +18
ntp_init 556 560 +4
resolve_peer_hostname 81 75 -6
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/1 up/down: 22/-6) Total: 16 bytes
Signed-off-by: Kaarle Ritvanen <kaarle.ritvanen@datakunkku.fi>
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
diff --git a/networking/ntpd.c b/networking/ntpd.c
index b7fa5dc..bfd5705 100644
--- a/networking/ntpd.c
+++ b/networking/ntpd.c
@@ -155,6 +155,7 @@
#define RETRY_INTERVAL 32 /* on send/recv error, retry in N secs (need to be power of 2) */
#define NOREPLY_INTERVAL 512 /* sent, but got no reply: cap next query by this many seconds */
#define RESPONSE_INTERVAL 16 /* wait for reply up to N secs */
+#define HOSTNAME_INTERVAL 5 /* hostname lookup failed. Wait N secs for next try */
/* Step threshold (sec). std ntpd uses 0.128.
*/
@@ -790,28 +791,20 @@
VERB6 bb_error_msg("%s->lastpkt_recv_time=%f", p->p_dotted, p->lastpkt_recv_time);
}
-static void
-resolve_peer_hostname(peer_t *p, int loop_on_fail)
+static len_and_sockaddr*
+resolve_peer_hostname(peer_t *p)
{
- len_and_sockaddr *lsa;
-
- again:
- lsa = host2sockaddr(p->p_hostname, 123);
- if (!lsa) {
- /* error message already emitted by host2sockaddr() */
- if (!loop_on_fail)
- return;
-//FIXME: do this to avoid infinite looping on typo in a hostname?
-//well... in which case, what is a good value for loop_on_fail?
- //if (--loop_on_fail == 0)
- // xfunc_die();
- sleep(5);
- goto again;
+ len_and_sockaddr *lsa = host2sockaddr(p->p_hostname, 123);
+ if (lsa) {
+ free(p->p_lsa);
+ free(p->p_dotted);
+ p->p_lsa = lsa;
+ p->p_dotted = xmalloc_sockaddr2dotted_noport(&lsa->u.sa);
+ } else {
+ /* error message is emitted by host2sockaddr() */
+ set_next(p, HOSTNAME_INTERVAL);
}
- free(p->p_lsa);
- free(p->p_dotted);
- p->p_lsa = lsa;
- p->p_dotted = xmalloc_sockaddr2dotted_noport(&lsa->u.sa);
+ return lsa;
}
static void
@@ -822,28 +815,28 @@
p = xzalloc(sizeof(*p) + strlen(s));
strcpy(p->p_hostname, s);
- resolve_peer_hostname(p, /*loop_on_fail=*/ 1);
+ p->p_fd = -1;
+ p->p_xmt_msg.m_status = MODE_CLIENT | (NTP_VERSION << 3);
+ p->next_action_time = G.cur_time; /* = set_next(p, 0); */
+ reset_peer_stats(p, STEP_THRESHOLD);
/* Names like N.<country2chars>.pool.ntp.org are randomly resolved
* to a pool of machines. Sometimes different N's resolve to the same IP.
* It is not useful to have two peers with same IP. We skip duplicates.
*/
- for (item = G.ntp_peers; item != NULL; item = item->link) {
- peer_t *pp = (peer_t *) item->data;
- if (strcmp(p->p_dotted, pp->p_dotted) == 0) {
- bb_error_msg("duplicate peer %s (%s)", s, p->p_dotted);
- free(p->p_lsa);
- free(p->p_dotted);
- free(p);
- return;
+ if (resolve_peer_hostname(p)) {
+ for (item = G.ntp_peers; item != NULL; item = item->link) {
+ peer_t *pp = (peer_t *) item->data;
+ if (pp->p_dotted && strcmp(p->p_dotted, pp->p_dotted) == 0) {
+ bb_error_msg("duplicate peer %s (%s)", s, p->p_dotted);
+ free(p->p_lsa);
+ free(p->p_dotted);
+ free(p);
+ return;
+ }
}
}
- p->p_fd = -1;
- p->p_xmt_msg.m_status = MODE_CLIENT | (NTP_VERSION << 3);
- p->next_action_time = G.cur_time; /* = set_next(p, 0); */
- reset_peer_stats(p, STEP_THRESHOLD);
-
llist_add_to(&G.ntp_peers, p);
G.peer_cnt++;
}
@@ -871,6 +864,11 @@
static void
send_query_to_peer(peer_t *p)
{
+ if (!p->p_lsa) {
+ if (!resolve_peer_hostname(p))
+ return;
+ }
+
/* Why do we need to bind()?
* See what happens when we don't bind:
*
@@ -2238,7 +2236,7 @@
IF_FEATURE_NTPD_SERVER("I:") /* compat */
"d" /* compat */
"46aAbgL", /* compat, ignored */
- &peers,&G.script_name,
+ &peers, &G.script_name,
#if ENABLE_FEATURE_NTPD_SERVER
&G.if_name,
#endif
@@ -2263,9 +2261,6 @@
if (opts & OPT_N)
setpriority(PRIO_PROCESS, 0, -15);
- /* add_peers() calls can retry DNS resolution (possibly forever).
- * Daemonize before them, or else boot can stall forever.
- */
if (!(opts & OPT_n)) {
bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO, argv);
logmode = LOGMODE_NONE;
@@ -2400,7 +2395,7 @@
/* What if don't see it because it changed its IP? */
if (p->reachable_bits == 0)
- resolve_peer_hostname(p, /*loop_on_fail=*/ 0);
+ resolve_peer_hostname(p);
set_next(p, timeout);
}