SF.net SVN: nagiosplug: [1909] nagiosplug/trunk

[email protected]
Newsgroups gmane.network.nagios.plugins.cvs
Message-ID <[email protected]>
Revision: 1909
          http://nagiosplug.svn.sourceforge.net/nagiosplug/?rev=1909&view=rev
Author:   dermoth
Date:     2008-01-29 00:54:48 -0800 (Tue, 29 Jan 2008)

Log Message:
-----------
Fix bugs and flaws in best offset server selection of check_ntp_time and (deprecated) check_ntp

Modified Paths:
--------------
    nagiosplug/trunk/NEWS
    nagiosplug/trunk/plugins/check_ntp.c
    nagiosplug/trunk/plugins/check_ntp_time.c

Modified: nagiosplug/trunk/NEWS
===================================================================
--- nagiosplug/trunk/NEWS	2008-01-26 15:55:03 UTC (rev 1908)
+++ nagiosplug/trunk/NEWS	2008-01-29 08:54:48 UTC (rev 1909)
@@ -4,10 +4,11 @@
 	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

Modified: nagiosplug/trunk/plugins/check_ntp.c
===================================================================
--- nagiosplug/trunk/plugins/check_ntp.c	2008-01-26 15:55:03 UTC (rev 1908)
+++ nagiosplug/trunk/plugins/check_ntp.c	2008-01-29 08:54:48 UTC (rev 1909)
@@ -302,50 +302,52 @@
  * 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;

Modified: nagiosplug/trunk/plugins/check_ntp_time.c
===================================================================
--- nagiosplug/trunk/plugins/check_ntp_time.c	2008-01-26 15:55:03 UTC (rev 1908)
+++ nagiosplug/trunk/plugins/check_ntp_time.c	2008-01-29 08:54:48 UTC (rev 1909)
@@ -247,50 +247,52 @@
  * 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;


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.