patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9581, Add "server...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9581: Add "server reachability" register to SNTP

Submitter:  Tom Ferrin <tferrin>
Submitted:  Wed 07 Mar 2018 11:35:22 PM UTC
   
 
Category:  apps Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None

Tue 03 Apr 2018 07:58:42 PM UTC, comment #5: 

Pushed slightly changed:
- moved the reachability into the server array
- added a define to guard against increased size to sntp_opts.h

Thanks for the patch.

Simon Goldschmidt <goldsimon>
Group administrator
Sat 31 Mar 2018 06:56:04 PM UTC, comment #4: 

Ok. I believe I have addressed all the concerns:

1) I've made the reachability variable into an array sized by SNTP_MAX_SERVERS.

2) I set the appropriate entry to zero in sntp_stop() instead of sntp_init().

3) I've submitted the changes as patch files.

4) I've tested the code with SNTP_MAX_SERVERS equal to both 1 and 3, and using both reliable and unreliable time servers. In doing #4 I have discovered a bug in the existing code and I will submit a fix for that as a separate patch request.


(file #43746, file #43747)

Tom Ferrin <tferrin>
Tue 13 Mar 2018 08:58:38 PM UTC, comment #3: 

Ok. If sntp_stop is your concern, you should set the variable to zero in that function.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 13 Mar 2018 08:56:43 PM UTC, comment #2: 

Thank you for responding, Simon.

a) I set the reachability variable to zero in the init function because one possible scenario is that the application calls the stop function and then at some future time calls init again (e.g., after shutting down a WiFi connection and then restarting it). In this case I thought it best to re-initialize the variable.

b) I will resubmit my changes as a patch.

c) You raise a good point if more than one server is in use. From my reading of the RFC and knowing how this variable is implemented on Linux systems, there should be a separate variable for each server. I will update the code prior to submitting the patch.

Tom Ferrin <tferrin>
Tue 13 Mar 2018 08:05:19 PM UTC, comment #1: 

Thre things:
a) the reachability variable does not have to be set to zero in the init function, C startup code does this for you (sntp_pcb is not set to zero, either)
b) a patch instead of files is far better. When changes are made to sntp before your patch gets applied, it can get difficult to see what you actually changed
c) the 'reachability' variable is global but we have more than one server. Is this intended by the RFC you cited? (I'm too lazy to search for it right now)

Simon Goldschmidt <goldsimon>
Group administrator
Wed 07 Mar 2018 11:35:22 PM UTC, original submission:  

Currently there is no way for an application to determine if a time server is reliably reachable. Since NTP is a UDP-based protocol, network congestion or an overloaded server could easily mean that NTP packets are only occasionally being received, but the application has no way of knowing this.

The additions in this patch implement a 8-bit “p.reach” server reachability shift register, as described in section 9.2 of RFC 5905, and the associated sntp_getreachability() function returns the value of this register to the application layer. This can then be used by the application to determine whether the server is reachable and, if not, this fact can be reported and another server can be chosen.

The algorithm from RFC 5905 is simple and the implementation in this patch requires very few lines of code. The reachability register is initialized to 0 and then shifted left by one bit when a request packet is sent. If a valid response packet arrives, the rightmost bit is set to one. If the register contains any nonzero bits, the server is considered reachable. The number of one bits versus zero bits can also be used to determine how reliable the reception of data packets is.

Tom Ferrin <tferrin>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #43746:  sntp.h.patch added by tferrin (440B - application/octet-stream)
file #43747:  sntp.c.patch added by tferrin (2KiB - application/octet-stream)
file #43484:  sntp.h added by tferrin (3KiB - application/octet-stream)
file #43485:  sntp.c added by tferrin (26KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by tferrin (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-04-03 goldsimon StatusNone Done
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2018-03-31 tferrin Attached File- Added sntp.h.patch, #43746
        Attached File- Added sntp.c.patch, #43747
    2018-03-07 tferrin Attached File- Added sntp.h, #43484
        Attached File- Added sntp.c, #43485

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code