[PATCH 2/2] NTPD: Allow stratum 1 behavior

[Denys Vlasenko]

On Sunday 03 October 2010 18:24, Jean-Christophe DUBOIS wrote:
> Allow busybox to act as a stratum 1 server if no peer is
> specified on the command line.
> 
> This allow busybox to act as a time reference in an isolated
> network where it is not possible to reach a real stratum
> server through the IP network.
> 
> For this feature to work correctly the busybox system needs
> to be synchronized to a real time source through other means
> like for example a GPS device.

Why do you want this to be a config option? It can be just
a permanently enabled feature, right?

> --- a/networking/ntpd.c
> +++ b/networking/ntpd.c
> @@ -1763,6 +1763,11 @@ recv_and_process_client_pkt(void /*int fd*/)
>  	msg.m_stratum = G.stratum;
>  	msg.m_ppoll = G.poll_exp;
>  	msg.m_precision_exp = G_precision_exp;
> +#ifdef CONFIG_FEATURE_NTPD_STRATUM_1
> +	if (G.stratum == 1) {
> +		G.reftime = gettime1900d(); /* sets G.cur_time too */
> +	}
> +#endif
>       /* this time was obtained between poll() and recv() */
>       msg.m_rectime = d_to_lfp(G.cur_time);
>       msg.m_xmttime = d_to_lfp(gettime1900d()); /* this instant */

Well, calling gettime1900d() twice is wasteful.

Wait...

First gettime1900d() is not needed! Check the only one existing callsite
of recv_and_process_client_pkt(): at that point, current time is
already in G.cur_time. So you can simply do:

	if (G.peer_cnt == 0) {
		/* we have no peers: "stratum 1 server" mode */
		G.reftime = G.cur_time;
	}


>  	ntp_init(argv);
>
> -	/* it is useless to continue if no peer is defined */
> -	/* if in server mode, we will never sync */
> -	/* if in client mode, we will never sync */
>  	if (G.peer_cnt == 0) {
> +#ifdef CONFIG_FEATURE_NTPD_STRATUM_1
> +		/*
> +		 * No peer is defined on the command line. So this is
> +		 * an isolated system and we will pretend being a
> +		 * stratum 1 assuming our local clock is correctly
> +		 * synchronized through another mean (like GPS).
> +		 */
> +		G.stratum = 1;
> +#else
> +		/*
> +		 * it is useless to continue if no peer is defined
> +		 * if in server mode, we will never sync
> +		 * if in client mode, we will never sync
> +		 */
>  		bb_error_msg("no peer defined");
>  		return -EINVAL;
> +#endif

Why the check is not in ntp_init()? Why return -EINVAL? Usual error code is 1,
not -something. Can use bb_error_msg_and_die().

Here is the commit which implements this:

http://git.busybox.net/busybox/commit/?id=d678257c26e0993efc48ac4433d153e1e9dfc954

Thanks!

-- 
vda