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>
This commit is contained in:
		
				
					committed by
					
						
						Denys Vlasenko
					
				
			
			
				
	
			
			
			
						parent
						
							e6add210b2
						
					
				
				
					commit
					b62ea34afe
				
			@@ -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 @@ reset_peer_stats(peer_t *p, double offset)
 | 
			
		||||
	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 @@ add_peers(const char *s)
 | 
			
		||||
 | 
			
		||||
	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 @@ do_sendto(int fd,
 | 
			
		||||
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 @@ static NOINLINE void ntp_init(char **argv)
 | 
			
		||||
			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 @@ 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.
 | 
			
		||||
	 */
 | 
			
		||||
	if (!(opts & OPT_n)) {
 | 
			
		||||
		bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO, argv);
 | 
			
		||||
		logmode = LOGMODE_NONE;
 | 
			
		||||
@@ -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);
 | 
			
		||||
				}
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user