[PATCH 2/2] NTPD: Allow stratum 1 behavior
Jean-Christophe Dubois
jcd at tribudubois.net
Mon Oct 4 06:25:52 UTC 2010
le lundi 4 octobre 2010 Denys Vlasenko a écrit
> 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?
Well I was not sure everybody would:
- agree with the feature (is it right to claim by default you are a stratum
1?)
- need the feature
- happy with the small code it is adding
So making it optional was allowing people to add the feature only if they
needed it.
> > --- 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.
True.
> 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.
True, my bad.
> So you can simply do:
>
> if (G.peer_cnt == 0) {
> /* we have no peers: "stratum 1 server" mode */
> G.reftime = G.cur_time;
> }
OK
>
> > 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().
OK. I am not used to these BB specific functions yet. Anyway your patch doesn't
use it as the stratum 1 mode is now default if no peer argument on the command
line.
> Here is the commit which implements this:
>
> http://git.busybox.net/busybox/commit/?id=d678257c26e0993efc48ac4433d153e1e
> 9dfc954
>
> Thanks!
More information about the busybox
mailing list