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' ... This patch is based on Kaarle Ritvanens work. http://lists.busybox.net/pipermail/busybox/2016-May/084197.html function old new delta static.set_next - 23 +23 step_time 426 422 -4 recv_and_process_peer_pkt 2342 2338 -4 ntpd_main 1178 1171 -7 add_peers 229 217 -12 resolve_peer_hostname 102 88 -14 .rodata 125081 125065 -16 ------------------------------------------------------------------------------ (add/remove: 1/0 grow/shrink: 0/6 up/down: 23/-57) Total: -34 bytes text data bss dec hex filename 802129 15459 2192 819780 c8244 busybox_old 802095 15459 2192 819746 c8222 busybox_unstripped Signed-off-by: Kaarle Ritvanen <[email protected]> Signed-off-by: Natanael Copa <[email protected]> --- networking/ntpd.c | 71 ++++++++++++++++++++++++++----------------------------- 1 file changed, 33 insertions(+), 38 deletions(-) diff --git a/networking/ntpd.c b/networking/ntpd.c index b7fa5dce9..97e3919fb 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 4 /* hostname lookup failed. Wait N secs for next try */ /* Step threshold (sec). std ntpd uses 0.128. */ @@ -293,6 +294,7 @@ typedef struct { typedef struct { len_and_sockaddr *p_lsa; + char *p_hostname; char *p_dotted; int p_fd; int datapoint_idx; @@ -318,7 +320,6 @@ typedef struct { datapoint_t filter_datapoint[NUM_DATAPOINTS]; /* last sent packet: */ msg_t p_xmt_msg; - char p_hostname[1]; } peer_t; @@ -791,27 +792,17 @@ reset_peer_stats(peer_t *p, double offset) } static void -resolve_peer_hostname(peer_t *p, int loop_on_fail) -{ - 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; +resolve_peer_hostname(peer_t *p) { + len_and_sockaddr *lsa = host2sockaddr(p->p_hostname, 123); + if (lsa) { + if (p->p_lsa) { + free(p->p_lsa); + free(p->p_dotted); + } + p->p_lsa = lsa; + p->p_dotted = xmalloc_sockaddr2dotted_noport(&lsa->u.sa); } - free(p->p_lsa); - free(p->p_dotted); - p->p_lsa = lsa; - p->p_dotted = xmalloc_sockaddr2dotted_noport(&lsa->u.sa); + set_next(p, lsa ? 0 : HOSTNAME_INTERVAL); } static void @@ -820,28 +811,29 @@ add_peers(const char *s) llist_t *item; peer_t *p; - p = xzalloc(sizeof(*p) + strlen(s)); - strcpy(p->p_hostname, s); - resolve_peer_hostname(p, /*loop_on_fail=*/ 1); + p = xzalloc(sizeof(*p)); + p->p_hostname = xstrdup(s); + resolve_peer_hostname(p); /* 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 (p->p_lsa) + for (item = G.ntp_peers; item != NULL; item = item->link) { + peer_t *pp = (peer_t *) item->data; + if (pp->p_lsa && strcmp(p->p_dotted, pp->p_dotted) == 0) { + bb_error_msg("duplicate peer %s (%s)", s, p->p_dotted); + free(p->p_hostname); + 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); @@ -2263,8 +2255,8 @@ static NOINLINE void ntp_init(char **argv) 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. + /* add_peers() calls can retry DNS resolution. + * Daemonize before them, or else boot can stall. */ if (!(opts & OPT_n)) { bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO, argv); @@ -2378,7 +2370,10 @@ int ntpd_main(int argc UNUSED_PARAM, char **argv) for (item = G.ntp_peers; item != NULL; item = item->link) { peer_t *p = (peer_t *) item->data; - if (p->next_action_time <= G.cur_time) { + if (!p->p_lsa) { + resolve_peer_hostname(p); + + } else if (p->next_action_time <= G.cur_time) { if (p->p_fd == -1) { /* Time to send new req */ if (--cnt == 0) { @@ -2400,7 +2395,7 @@ int ntpd_main(int argc UNUSED_PARAM, char **argv) /* 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); } -- 2.11.0 _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
