This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Added unicast support to SNTP client


Andrew,

Thanks for the code review.  I agree with all your
tweaks, and thanks for filling in the missing pieces.

In terms of (re)setting the NTP server list, I
considered the "immediate wakeup" approach but decided
that it didn't really buy us too much.  The user
application has to be generally tolerant of a "slow"
initial time update, since a lost UDP packet or a busy
NTP server can cause normal delays here anyway.

Once our time is set, it doesn't seem necessary to
immediately update after an NTP server change either. 
The presumption here is that all NTP servers are
reasonably synchronized to the "correct" time. 
Waiting up to 30 minutes to resynchronize is probably
not a big deal.

So, abstractly, I agree it would be nice to trigger an
immediate NTP update whenever the NTP list changes,
but  practically speaking I don't know that it matters
that much.  Did you have any other scenarios where you
could imagine this being more critical?

-- Dan

--- Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Oct 15, 2003 at 10:39:29AM -0700, Dan
> Jakubiec wrote:
> > I've attached an SNTP patch that addresses Andrew
> > Lunn's comments about my previous SNTP unicast
> > enhancements.  This patch includes:
> > 
> > 1) SNTP documentation updates
> > 
> > 2) SNTP test updates to include unicast mode
> testing
> > 
> > 3) Support for extracting NTP server information
> from
> > DHCP
> > 
> > 4) Removed NTP_UDP_PORT constant from sntp.h
> 
> Thanks for the patch. I made a few tweaks.
> 
> There was a problem that the DHCP code would modify
> the list of
> servers while sntp could be using it. I got around
> this by
> deregistering the existing servers before building
> the new list.
> 
> I added code where the list of options are printed
> so that it know
> knows about the TAG_NTP_SERVER option.
> 
> I also made a few tweaks to the documentation.
> 
> What would be nice is if the call the
> cyg_sntp_set_servers would break
> the main thread out of select and so imeadiately use
> the new list of
> servers. Something to add to the TODO list.
> 
> Attached i what i committed.
> 
>          Andrew
> > Index: net/sntp/current/ChangeLog
>
===================================================================
> RCS file:
> /cvs/ecos/ecos-opt/net/net/sntp/current/ChangeLog,v
> retrieving revision 1.7
> diff -u -r1.7 ChangeLog
> --- net/sntp/current/ChangeLog	2 Oct 2003 22:43:41
> -0000	1.7
> +++ net/sntp/current/ChangeLog	21 Oct 2003 17:12:21
> -0000
> @@ -1,3 +1,12 @@
> +2003-10-15  Dan Jakubiec  <djakubiec@yahoo.com>
> +            Andrew Lunn   <andrew.lunn@ascom.ch>
> +
> +	* src/sntp.c: Added DHCP support for SNTP unicast
> mode.
> +	* src/sntp.cdl: Added DHCP support for SNTP
> unicast mode.
> +	* src/sntp.h: Removed UDP port constant.
> +	* src/sntp1.c: Added test code for SNTP unicast
> mode.
> +	* src/sntp.sgml: Added documentation for SNTP
> unicast mode.
> +
>  2003-09-29  Dan Jakubiec 
> <firstname.lastname@systech.com>
>  
>  	* src/sntp.c: Added support for SNTP unicast mode.
> Index: net/sntp/current/cdl/sntp.cdl
>
===================================================================
> RCS file:
>
/cvs/ecos/ecos-opt/net/net/sntp/current/cdl/sntp.cdl,v
> retrieving revision 1.3
> diff -u -r1.3 sntp.cdl
> --- net/sntp/current/cdl/sntp.cdl	2 Oct 2003
> 22:43:41 -0000	1.3
> +++ net/sntp/current/cdl/sntp.cdl	21 Oct 2003
> 17:12:22 -0000
> @@ -62,21 +62,34 @@
>      requires      CYGSEM_LIBC_TIME_SETTIME_WORKING
>      compile       sntp.c
>      
> +    cdl_component CYGPKG_NET_SNTP_UNICAST {
> +        display "Enable SNTP client unicast
> support"
> +        flavor  bool
> +        default_value 0
> +        description   "
> +            This option enables SNTP unicast mode
> in
> +            for the SNTP client.  This mode will
> send
> +            SNTP requests to NTP/SNTP servers in
> +            addition to listening for SNTP
> broadcasts."
> +
> +        cdl_option CYGNUM_NET_SNTP_UNICAST_MAXDHCP
> {
> +            display "Maximum number of NTP servers
> to use from DHCP"
> +            flavor  booldata
> +            requires CYGPKG_NET_DHCP
> +            legal_values 1 to 8
> +            default_value 2
> +            description   "
> +                This option specifies the maximum
> number of
> +                NTP servers to get from DHCP. 
> These servers
> +                are used to configure the unicast
> SNTP client.
> +                Disabling this option disables DHCP
> usage."
> +        }
> +    }
> +
>      cdl_component CYGPKG_NET_SNTP_OPTIONS {
>          display "SNTP support build options"
>          flavor  none
>          no_define
> -
> -        cdl_option CYGPKG_NET_SNTP_UNICAST {
> -            display "Enable SNTP client unicast
> support"
> -            flavor  bool
> -            default_value 0
> -            description   "
> -                This option enables SNTP unicast
> mode in
> -                for the SNTP client.  This mode
> will send
> -                SNTP requests to NTP/SNTP servers
> in
> -                addition to listening for SNTP
> broadcasts."
> -        }
>  
>          cdl_option CYGPKG_NET_SNTP_CFLAGS_ADD {
>              display "Additional compiler flags"
> Index: net/sntp/current/doc/sntp.sgml
>
===================================================================
> RCS file:
>
/cvs/ecos/ecos-opt/net/net/sntp/current/doc/sntp.sgml,v
> retrieving revision 1.3
> diff -u -r1.3 sntp.sgml
> --- net/sntp/current/doc/sntp.sgml	21 May 2003
> 14:03:36 -0000	1.3
> +++ net/sntp/current/doc/sntp.sgml	21 Oct 2003
> 17:12:22 -0000
> @@ -36,7 +36,8 @@
>  The SNTP package provides implementation of a
> client for RFC 2030, the
>  Simple Network Time Protocol (SNTP). The client
> listens for broadcasts
>  or IPv6 multicasts from an NTP server and uses the
> information received to
> -set the system clock.
> +set the system clock.  It can also be configured to
> send SNTP time
> +requests to specific NTP servers using SNTP's
> unicast mode.
>  </PARA>
>  </PARTINTRO>
>  <CHAPTER id="net-sntp">
> @@ -45,16 +46,20 @@
>  <TITLE>Starting the SNTP client</TITLE>
>  <para>
>  The sntp client is implemented as a thread which
> listens for NTP
> -broadcasts and IPv6 multicasts. This thread is not
> automatically start by the
> -system. Instead it must be started by the user
> application. The header
> -file <filename>cyg/sntp/sntp.h</filename> declares
> the function to be
> +broadcasts and IPv6 multicasts, and optionally
> sends SNTP unicast
> +requests to specific NTP servers. This thread may
> be automatically
> +started by the system if it receives a list of
> (S)NTP servers from the
> +DHCP server and unicast mode is enabled. Otherwise
> it must be started
> +by the user application. The header file
> +<filename>cyg/sntp/sntp.h</filename> declares the
> function to be
>  called.  The thread is then started by calling the
> function:
>  </para>
>  <programlisting>
>  void cyg_sntp_start(void);
>  </programlisting>
>  <para>
> -Once started, the thread will run forever.
> +It is safe to call this function multiple times.
> Once started, the
> +thread will run forever.
>  </para>
>  </sect1>
>  
> @@ -62,8 +67,8 @@
>  <title>What it does</title>
>  <para>
>  The SNTP client listens for NTP IPv4 broadcasts
> from any NTP servers,
> -or IPv6 multicasts using the address fe0x:0X::101,
> where X can be 1
> -(Node Local), 2 (Link Local), 5 (Site-Local) or 0xe
> (Global). Such
> +or IPv6 multicasts using the address fe0x:0X::101,
> where X can be
> +2 (Link Local), 5 (Site-Local) or 0xe (Global).
> Such
>  packets contain a timestamp indicating the current
> time. The packet
>  also contains information about where the server is
> in the hierarchy
>  of time servers. A server at the root of the time
> server tree normally
> @@ -73,8 +78,19 @@
>  servers using version 3 or 4 of the protocol. When
> receiving packets
>  from multiple servers, it will use the packets from
> the server with
>  the lowest stratum. However, if there are no
> packets from this server
> -for 10 minute and another server is sending
> packets, the client will
> -change server.
> +for 10 minutes and another server is sending
> packets, the client will
> +change servers.
> +</para>
> +<para>
> +If SNTP unicast mode is enabled via the
> CYGPKG_NET_SNTP_UNICAST
> +option, the SNTP client can additionally be
> configured with a list
> +of specific NTP servers to query.  The general
> algorithm is as follows: if
> +the system clock has not yet been set via an NTP
> time 
=== message truncated ===


__________________________________
Do you Yahoo!?
The New Yahoo! Shopping - with improved product search
http://shopping.yahoo.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]