Code

Fix bugs and flaws in best offset server selection of check_ntp_time and (deprecated...
authorThomas Guyot-Sionnest <dermoth@users.sourceforge.net>
Tue, 29 Jan 2008 08:54:48 +0000 (08:54 +0000)
committerThomas Guyot-Sionnest <dermoth@users.sourceforge.net>
Tue, 29 Jan 2008 08:54:48 +0000 (08:54 +0000)
git-svn-id: https://nagiosplug.svn.sourceforge.net/svnroot/nagiosplug/nagiosplug/trunk@1909 f882894a-f735-0410-b71e-b25c423dba1c

NEWS
plugins/check_ntp.c
plugins/check_ntp_time.c

diff --git a/NEWS b/NEWS
index 8e5a4946780eb22161eda8b53f6d0e9fa9eab4fe..628210da99d4192da39a420c3df8893830878b4f 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -4,10 +4,11 @@ This file documents the major additions and syntax changes between releases.
        Added ./check_nt -v INSTANCES to count number of instances (Alessandro Ren)
        New check_icmp -s option to specify the source IP address
        check_dns now sorts addresses for testing results for more than one returned IP (Matthias Urlichs)
-       Fix segfault in check_ntp_time and (deprecated) check_ntp. (Bug #1862300)
+       Fix segfault in check_ntp_time and (deprecated) check_ntp (Bug #1862300)
        check_disk should now work with large file systems (2TB+) on all archs that supports it
        Fixed check_disk disk usage calculation when using --group=NAME (related to bug #1348746)
        Fix help text of check_ntp_* (Bug #1880095)
+       Fix bugs and flaws in best offset server selection of check_ntp_time and (deprecated) check_ntp
 
 1.4.11 13th December 2007
        Fixed check_http regression in 1.4.10 where following redirects to
index b474d9a81c5a6313624f6ca4b35237ab521418ae..2b3cc91d7e2528c6f0c590990c7db573357791fd 100644 (file)
@@ -302,50 +302,52 @@ void setup_request(ntp_message *p){
  * this is done by filtering servers based on stratum, dispersion, and
  * finally round-trip delay. */
 int best_offset_server(const ntp_server_results *slist, int nservers){
-       int i=0, j=0, cserver=0, candidates[5], csize=0;
+       int i=0, cserver=0, best_server=-1;
 
        /* for each server */
        for(cserver=0; cserver<nservers; cserver++){
-               /* sort out servers with error flags */
-               if ( LI(slist[cserver].flags) != LI_NOWARNING ){
-                       if (verbose) printf("discarding peer id %d: flags=%d\n", cserver, LI(slist[cserver].flags));
-                       break;
+               /* We don't want any servers that fails these tests */
+               /* Sort out servers that didn't respond or responede with a 0 stratum;
+                * stratum 0 is for reference clocks so no NTP server should ever report
+                * a stratum 0 */
+               if ( slist[cserver].stratum == 0){
+                       if (verbose) printf("discarding peer %d: stratum=%d\n", cserver, slist[cserver].stratum);
+                       continue;
+               }
+               /* Sort out servers with error flags */
+               if ( LI(slist[cserver].flags) == LI_ALARM ){
+                       if (verbose) printf("discarding peer %d: flags=%d\n", cserver, LI(slist[cserver].flags));
+                       continue;
                }
 
-               /* compare it to each of the servers already in the candidate list */
-               for(i=0; i<csize; i++){
-                       /* does it have an equal or better stratum? */
-                       if(slist[cserver].stratum <= slist[i].stratum){
-                               /* does it have an equal or better dispersion? */
-                               if(slist[cserver].rtdisp <= slist[i].rtdisp){
-                                       /* does it have a better rtdelay? */
-                                       if(slist[cserver].rtdelay < slist[i].rtdelay){
-                                               break;
-                                       }
-                               }
-                       }
+               /* If we don't have a server yet, use the first one */
+               if (best_server == -1) {
+                       best_server = cserver;
+                       DBG(printf("using peer %d as our first candidate\n", best_server));
+                       continue;
                }
 
-               /* if we haven't reached the current list's end, move everyone
-                * over one to the right, and insert the new candidate */
-               if(i<csize){
-                       for(j=4; j>i; j--){
-                               candidates[j]=candidates[j-1];
+               /* compare the server to the best one we've seen so far */
+               /* does it have an equal or better stratum? */
+               DBG(printf("comparing peer %d with peer %d\n", cserver, best_server));
+               if(slist[cserver].stratum <= slist[best_server].stratum){
+                       DBG(printf("stratum for peer %d <= peer %d\n", cserver, best_server));
+                       /* does it have an equal or better dispersion? */
+                       if(slist[cserver].rtdisp <= slist[best_server].rtdisp){
+                               DBG(printf("dispersion for peer %d <= peer %d\n", cserver, best_server));
+                               /* does it have a better rtdelay? */
+                               if(slist[cserver].rtdelay < slist[best_server].rtdelay){
+                                       DBG(printf("rtdelay for peer %d < peer %d\n", cserver, best_server));
+                                       best_server = cserver;
+                                       DBG(printf("peer %d is now our best candidate\n", best_server));
+                               }
                        }
                }
-               /* regardless, if they should be on the list... */
-               if(i<5) {
-                       candidates[i]=cserver;
-                       if(csize<5) csize++;
-               /* otherwise discard the server */
-               } else {
-                       DBG(printf("discarding peer id %d\n", cserver));
-               }
        }
 
-       if(csize>0) {
-               DBG(printf("best server selected: peer %d\n", candidates[0]));
-               return candidates[0];
+       if(best_server >= 0) {
+               DBG(printf("best server selected: peer %d\n", best_server));
+               return best_server;
        } else {
                DBG(printf("no peers meeting synchronization criteria :(\n"));
                return -1;
index 16e4c3c7663b6257a737d68a546b22c6b12a4fb9..11a0eced801c083c976b4c90e0977cf224378141 100644 (file)
@@ -247,50 +247,52 @@ void setup_request(ntp_message *p){
  * this is done by filtering servers based on stratum, dispersion, and
  * finally round-trip delay. */
 int best_offset_server(const ntp_server_results *slist, int nservers){
-       int i=0, j=0, cserver=0, candidates[5], csize=0;
+       int i=0, cserver=0, best_server=-1;
 
        /* for each server */
        for(cserver=0; cserver<nservers; cserver++){
-               /* sort out servers with error flags */
-               if ( LI(slist[cserver].flags) != LI_NOWARNING ){
-                       if (verbose) printf("discarding peer id %d: flags=%d\n", cserver, LI(slist[cserver].flags));
-                       break;
+               /* We don't want any servers that fails these tests */
+               /* Sort out servers that didn't respond or responede with a 0 stratum;
+                * stratum 0 is for reference clocks so no NTP server should ever report
+                * a stratum 0 */
+               if ( slist[cserver].stratum == 0){
+                       if (verbose) printf("discarding peer %d: stratum=%d\n", cserver, slist[cserver].stratum);
+                       continue;
+               }
+               /* Sort out servers with error flags */
+               if ( LI(slist[cserver].flags) == LI_ALARM ){
+                       if (verbose) printf("discarding peer %d: flags=%d\n", cserver, LI(slist[cserver].flags));
+                       continue;
                }
 
-               /* compare it to each of the servers already in the candidate list */
-               for(i=0; i<csize; i++){
-                       /* does it have an equal or better stratum? */
-                       if(slist[cserver].stratum <= slist[i].stratum){
-                               /* does it have an equal or better dispersion? */
-                               if(slist[cserver].rtdisp <= slist[i].rtdisp){
-                                       /* does it have a better rtdelay? */
-                                       if(slist[cserver].rtdelay < slist[i].rtdelay){
-                                               break;
-                                       }
-                               }
-                       }
+               /* If we don't have a server yet, use the first one */
+               if (best_server == -1) {
+                       best_server = cserver;
+                       DBG(printf("using peer %d as our first candidate\n", best_server));
+                       continue;
                }
 
-               /* if we haven't reached the current list's end, move everyone
-                * over one to the right, and insert the new candidate */
-               if(i<csize){
-                       for(j=4; j>i; j--){
-                               candidates[j]=candidates[j-1];
+               /* compare the server to the best one we've seen so far */
+               /* does it have an equal or better stratum? */
+               DBG(printf("comparing peer %d with peer %d\n", cserver, best_server));
+               if(slist[cserver].stratum <= slist[best_server].stratum){
+                       DBG(printf("stratum for peer %d <= peer %d\n", cserver, best_server));
+                       /* does it have an equal or better dispersion? */
+                       if(slist[cserver].rtdisp <= slist[best_server].rtdisp){
+                               DBG(printf("dispersion for peer %d <= peer %d\n", cserver, best_server));
+                               /* does it have a better rtdelay? */
+                               if(slist[cserver].rtdelay < slist[best_server].rtdelay){
+                                       DBG(printf("rtdelay for peer %d < peer %d\n", cserver, best_server));
+                                       best_server = cserver;
+                                       DBG(printf("peer %d is now our best candidate\n", best_server));
+                               }
                        }
                }
-               /* regardless, if they should be on the list... */
-               if(i<5) {
-                       candidates[i]=cserver;
-                       if(csize<5) csize++;
-               /* otherwise discard the server */
-               } else {
-                       DBG(printf("discarding peer id %d\n", cserver));
-               }
        }
 
-       if(csize>0) {
-               DBG(printf("best server selected: peer %d\n", candidates[0]));
-               return candidates[0];
+       if(best_server >= 0) {
+               DBG(printf("best server selected: peer %d\n", best_server));
+               return best_server;
        } else {
                DBG(printf("no peers meeting synchronization criteria :(\n"));
                return -1;