Merges dev branch into master (#90)

* Don't assume server supports all test types

The NDT client assumes that the server supports all test types and the
official NDT server respects this assumption.

However, the [neubot/botticelli](https://github.com/neubot/botticelli)
server only implements TEST_META, TEST_S2C, and TEST_S2C.

Therefore, when using the NDT client with a botticelli server, the
client crahes in processing the results of the TEST_MID that however
has not been executed.

Fix by making sure that, when processing results, we use a bitmak where
only bits corresponding to tests that run are actually set.

(There is also another reason why NDT client crashes when testing with
botticelli, addressed by neubot/botticelli#18.)

Problems between NDT client and botticelli reported by @nkinkade, thanks!

* web100clt.c: make results parsing more robust

1) Do not assume that the first line we receive contains a space

2) Do not assume that the first line we receive contains an integer

3) Be robust to the case where input is an empty string

4) Do not assume that after the first token delimited by space we
   will find a second token delimited by newline

Tested under the following conditions:

a) web100clt -n ndt.iupui.mlab1.trn01.measurement-lab.org that sends
   back all variables and checked via printf() that the variables that
   are parsed by the new code are the ones received in resultstr

b) web100clt -n neubot.mlab.mlab1.mil01.measurement-lab.org that at
   the moment is running botticelli v0.0.5 (which is buggy and doesn't
   send any MSG_RESULTS messages) and make sure it does not crash

c) web100clt -n neubot.mlab.mlab1.trn01.measurement-lab.org that at
   the moment is running botticelli v0.0.6 (which sends a single
   dummy variable not considered by NDT) and make sure it doesn't crash

Note that a) and c) did not changed after this patch. What this patch
changes is the behavior in case b).

xref: neubot/botticelli#18.

* Add travis config to build with measurementlab/ndt-builder

* Fetch docker image from mlab-pub

* Add log_first_n, cleanup

* Add missing #include and make some functions static.

* remove confusing I2util discovery

* remove web100 dependent test from TESTS

* remove unused tcp_stat_setbuf decl that breaks strict lint

* Add log_first_n, cleanup

* Add missing #include and make some functions static.

* Add travis config to build with measurementlab/ndt-builder

* Fetch docker image from mlab-pub

* remove confusing I2util discovery

* remove web100 dependent test from TESTS

* remove unused tcp_stat_setbuf decl that breaks strict lint

* Temp workaround for builder

* Remove logfile after test (#88)

* Made the unit tests create the logfile in /tmp

* Clarified the test code

.travis.yml

@@ -0,0 +1,22 @@
+# Build NDT and run unit tests.
+# * Uses docker build-tools container
+# * Executes unit tests within travis after build.
+
+dist: trusty
+language: ruby
+services:
+- docker
+
+script:
+
+# Temporarily use the git repo dev branch, instead of the google container registry.
+# TODO - update this once the GCR builder is up to date.
+- docker build -t build https://github.com/m-lab/builder.git#dev
+- docker run build /root/ndt_build_and_test.sh dev
+
+#- docker pull gcr.io/mlab-pub/github-m-lab-builder:latest
+# Run basic unit tests that don't require web100
+#- docker run gcr.io/mlab-pub/github-m-lab-builder /root/ndt_build_and_test.sh
+
+# TODO - collect coverage stats and export to coveralls.
+

configure.ac

@@ -191,77 +191,32 @@ else
 fi
 
 
+#
+# configure I2util
+#
 AC_ARG_WITH(I2util,
 		AC_HELP_STRING([--with-I2util=<dir>],
 				[defaults to building I2util under NDT if exists]),
 		with_I2util=$withval, with_I2util=yes)		
 
-#
-# find I2util
-#
 I2UTILLDFLAGS=""
 I2UTILLIBS=""
 I2UTILLIBDEPS=""
 I2UTILLIBMAKE=""
 I2UTILINCS=""
 
-if test -n "$with_I2util" && test "$with_I2util" != "yes"; then
-	I2util_dir=${with_I2util}
-	case $I2util_dir in
-		/*) ;; # already an absolute path
-		*) I2util_dir="`pwd`/$I2util_dir" ;;
-	esac
-	I2util_include_dir=`dirname $with_I2util`/include
-	I2UTILINCS="-I$I2util_include_dir $I2UTILINCS"
-	I2UTILLDFLAGS="-L$I2util_dir $I2UTILLDFLAGS"
-	I2UTILLIBDEPS="$I2util_dir/libI2util.a"
-fi
-
-# Save the old CFLAGS and LDFLAGS
-OLD_LDFLAGS="$LDFLAGS"
-OLD_CFLAGS="$CFLAGS"
-
-CFLAGS="$CFLAGS $I2UTILINCS"
-LDFLAGS="$CFLAGS $I2UTILLDFLAGS"
-
-AC_SEARCH_LIBS([I2AddrByNode],I2util, HAVE_I2UTIL_LIBS=1,HAVE_I2UTIL_LIBS=0)
-AC_CHECK_HEADERS([I2util/util.h I2util/conf.h], HAVE_I2UTIL_HEADERS=1,HAVE_I2UTIL_HEADERS=0)
-
-LDFLAGS="$OLD_LDFLAGS"
-CFLAGS="$OLD_CFLAGS"
-
-if test "$HAVE_I2UTIL_HEADERS" != "1" || test "$HAVE_I2UTIL_LIBS" != "1"; then
-	# now, check for sub-build/sub-configure
-	if test -d I2util/I2util; then
-		AC_CONFIG_SUBDIRS(I2util)
-		TOP_BUILD_DIRS="I2util $TOP_BUILD_DIRS"
-		I2util_dir='${top_srcdir}/I2util'
-		I2UTILINCS="-I$I2util_dir $I2UTILINCS"
-		I2UTILLDFLAGS="-L$I2util_dir/I2util $I2UTILLDFLAGS"
-		I2UTILLIBDEPS="$I2util_dir/I2util/libI2util.a"
-		I2UTILLIBMAKE="cd $I2util_dir; make"
-	else
-		AC_MSG_FAILURE([I2util required to build NDT])
-	fi
-
-        # Save the old CFLAGS and LDFLAGS
-	OLD_LDFLAGS="$LDFLAGS"
-	OLD_CFLAGS="$CFLAGS"
-
-        CFLAGS="$CFLAGS $I2UTILINCS"
-        LDFLAGS="$CFLAGS $I2UTILLDFLAGS"
-
-        AC_SEARCH_LIBS([I2AddrByNode],I2util, ,AC_MSG_ERROR([Couldn't find I2util library]))
-        AC_CHECK_HEADERS([I2util/util.h I2util/conf.h], ,AC_MSG_ERROR([Couldn't find I2util header files]), [AC_INCLUDES_DEFAULT])
-
-	LDFLAGS="$OLD_LDFLAGS"
-	CFLAGS="$OLD_CFLAGS"
-
-	I2UTILLIBS="$I2UTILLDFLAGS -lI2util"
+if test -d I2util/I2util; then
+	AC_CONFIG_SUBDIRS(I2util)
+	TOP_BUILD_DIRS="I2util $TOP_BUILD_DIRS"
+	I2util_dir='${top_srcdir}/I2util'
+	I2UTILINCS="-I$I2util_dir $I2UTILINCS"
+	I2UTILLDFLAGS="-L$I2util_dir/I2util $I2UTILLDFLAGS"
+	I2UTILLIBDEPS="$I2util_dir/I2util/libI2util.a"
+	I2UTILLIBMAKE="cd $I2util_dir; make"
+else
+	AC_MSG_FAILURE([I2util required to build NDT])
 fi
 
-I2UTILLIBS="$I2UTILLDFLAGS -lI2util"
-
 AC_SUBST(I2UTILLDFLAGS)
 AC_SUBST(I2UTILLIBS)
 AC_SUBST(I2UTILLIBDEPS)

src/Makefile.am

@@ -28,7 +28,7 @@ endif
 
 bin_PROGRAMS =
 sbin_PROGRAMS = $(ADD_FAKEWWW)
-noinst_PROGRAMS = 
+noinst_PROGRAMS =
 TESTS =
 
 if HAVE_WEB100

src/genplot.c

@@ -27,6 +27,7 @@
 #include <sys/errno.h>
 #include <getopt.h>
 #include <sys/types.h>
+#include <web100.h>
 
 #include "web100srv.h"
 #include "usage.h"

src/logging.h

@@ -32,14 +32,22 @@ char* get_logfile();
 
 I2ErrHandle get_errhandle();
 
-// TODO: audit all the calls to log_print and log_println to maxmise ease of
+// TODO: audit all the calls to log_print and log_println to maximise ease of
 //       grepping through logs and eliminate the possibility of loglines that
 //       don't start with the timestamp/pid/level/file/line prefix.
 #define log_print log_println
 void log_println_impl(int lvl, const char* file, int line, const char* format, ...);
 #define log_println(lvl, ...) \
     log_println_impl((lvl), __FILE__, __LINE__, __VA_ARGS__)
 
+#define log_first_n(lvl, n, ...) { \
+    static int LOG_COUNT&&__LINE__ = n; \
+    if (LOG_COUNT&&__LINE__ > 0) { \
+      LOG_COUNT&&__LINE__--; \
+      log_println_impl((lvl), __FILE__, __LINE__, __VA_ARGS__); \
+    }}
+
+
 void log_free(void);
 void set_timestamp();
 time_t get_timestamp();

src/test_s2c_srv.c

@@ -11,6 +11,8 @@
 #include <pthread.h>
 #include <sys/times.h>
 #include <ctype.h>
+#include <web100.h>
+
 #include "tests_srv.h"
 #include "strlutils.h"
 #include "ndtptestconstants.h"

src/test_sfw_srv.c

@@ -8,6 +8,7 @@
 
 #include <assert.h>
 #include <pthread.h>
+#include <web100.h>
 
 #include "test_sfw.h"
 #include "logging.h"

src/testoptions.c

@@ -9,6 +9,7 @@
 #include <string.h>
 // #include <ctype.h>
 #include <pthread.h>
+#include <web100.h>
 
 #include "testoptions.h"
 #include "network.h"

src/web100-util.c

@@ -9,6 +9,7 @@
 #include <ctype.h>
 #include <time.h>
 #include <assert.h>
+#include <web100.h>
 
 #include "logging.h"
 #include "network.h"
@@ -821,7 +822,7 @@ int tcp_stat_get_data(tcp_stat_snap** snap, Connection* testsock, int streamsNum
  *
  *
  */
-int web100_rtt(int sock, web100_agent* agent, web100_connection* cn) {
+static int web100_rtt(int sock, web100_agent* agent, web100_connection* cn) {
   web100_var* var;
   char buf[32];
   web100_group* group;
@@ -938,8 +939,8 @@ int tcp_stat_autotune(int sock, tcp_stat_agent* agent, tcp_stat_connection cn) {
  * 					35 - cannot read value of RcvWinScale web100 variable.
  *
  */
-int tcp_stat_setbuff(int sock, tcp_stat_agent* agent, tcp_stat_connection cn,
-                     int autotune) {
+static int tcp_stat_setbuff(int sock, tcp_stat_agent* agent, tcp_stat_connection cn,
+                            int autotune) {
   web100_var* var;
   char buf[32];
   web100_group* group;
@@ -1108,7 +1109,7 @@ void tcp_stat_logvars_to_file(char* webVarsValuesLog, int connNum, struct tcp_va
   fclose(file);
 }
 
-tcp_stat_var agg_vars_sum(int connNum, int varId, struct tcp_vars* vars) {
+static tcp_stat_var agg_vars_sum(int connNum, int varId, struct tcp_vars* vars) {
   int i;
   tcp_stat_var varValue = *&((tcp_stat_var *)&vars[0])[varId];
   for (i = 1; i < connNum; ++i) {
@@ -1117,7 +1118,7 @@ tcp_stat_var agg_vars_sum(int connNum, int varId, struct tcp_vars* vars) {
   return varValue;
 }
 
-tcp_stat_var agg_vars_max(int connNum, int varId, struct tcp_vars* vars) {
+static tcp_stat_var agg_vars_max(int connNum, int varId, struct tcp_vars* vars) {
   int i;
   tcp_stat_var varValue = *&((tcp_stat_var *)&vars[0])[varId];
   for (i = 1; i < connNum; ++i) {
@@ -1128,7 +1129,7 @@ tcp_stat_var agg_vars_max(int connNum, int varId, struct tcp_vars* vars) {
   return varValue;
 }
 
-tcp_stat_var agg_vars_min(int connNum, int varId, struct tcp_vars* vars) {
+static tcp_stat_var agg_vars_min(int connNum, int varId, struct tcp_vars* vars) {
   int i;
   tcp_stat_var varValue = *&((tcp_stat_var *)&vars[0])[varId];
   for (i = 1; i < connNum; ++i) {
@@ -1139,7 +1140,7 @@ tcp_stat_var agg_vars_min(int connNum, int varId, struct tcp_vars* vars) {
   return varValue;
 }
 
-tcp_stat_var agg_vars_avg(int connNum, int varId, struct tcp_vars* vars) {
+static tcp_stat_var agg_vars_avg(int connNum, int varId, struct tcp_vars* vars) {
   int i;
   tcp_stat_var varValue = *&((tcp_stat_var *)&vars[0])[varId];
   for (i = 1; i < connNum; ++i) {
@@ -1326,7 +1327,7 @@ int CwndDecrease(char* logname, u_int32_t *dec_cnt,
  * @param nwords integer length (in bytes) of the header
  * @return unsigned short checksum
  */
-unsigned short csum(unsigned short *buff, int nwords) {
+static unsigned short csum(unsigned short *buff, int nwords) {
   /* generate a TCP/IP checksum for our packet */
 
   register int sum = 0;

src/web100clt.c

@@ -149,15 +149,12 @@ void testResults(char tests, char *testresult_str, char* host) {
   }
 
   sysvar = strtok(testresult_str, " ");
-  sysval = strtok(NULL, "\n");
-  i = atoi(sysval);
-  save_int_values(sysvar, i);
-
   for (;;) {
-    sysvar = strtok(NULL, " ");
     if (sysvar == NULL)
       break;
     sysval = strtok(NULL, "\n");
+    if (sysval == NULL)
+      break;
     if (strchr(sysval, '.') == NULL) {
       i = atoi(sysval);
       save_int_values(sysvar, i);
@@ -167,6 +164,7 @@ void testResults(char tests, char *testresult_str, char* host) {
       save_dbl_values(sysvar, &j);
       log_println(7, "Stored %0.2f (%s) in %s", j, sysval, sysvar);
     }
+    sysvar = strtok(NULL, " ");
   }
 
   // CountRTT = 615596;
@@ -924,6 +922,12 @@ int main(int argc, char *argv[]) {
   } else
     ptr = strtok_r(buff, " ", &strtokbuf);
 
+  /*
+   * Server may support less test types that we support. Hence, collect
+   * into this var _only_ the test types that were run successfully.
+   */
+  unsigned char run_tests = 0;
+
   // Run all tests requested, based on the ID.
   while (ptr) {
     if (check_int(ptr, &testId)) {
@@ -935,39 +939,45 @@ int main(int argc, char *argv[]) {
         if (test_mid_clt(ctlSocket, tests, host, conn_options, buf_size,
                          mid_resultstr, jsonSupport)) {
           log_println(0, "Middlebox test FAILED!");
-          tests &= (~TEST_MID);
+        } else {
+          run_tests |= TEST_MID;
         }
         break;
       case TEST_C2S:
         if (test_c2s_clt(ctlSocket, tests, host, conn_options, buf_size, &c2s_ThroughputSnapshots, jsonSupport, 0)) {
           log_println(0, "C2S throughput test FAILED!");
-          tests &= (~TEST_C2S);
+        } else {
+          run_tests |= TEST_C2S;
         }
         break;
       case TEST_C2S_EXT:
         if (test_c2s_clt(ctlSocket, tests, host, conn_options, buf_size, &c2s_ThroughputSnapshots, jsonSupport, 1)) {
           log_println(0, "Extended S2C throughput test FAILED!");
-          tests &= (~TEST_C2S_EXT);
+        } else {
+          run_tests |= TEST_C2S_EXT;
         }
         break;
       case TEST_S2C:
         if (test_s2c_clt(ctlSocket, tests, host, conn_options, buf_size,
                          resultstr, &s2c_ThroughputSnapshots, jsonSupport, 0)) {
           log_println(0, "S2C throughput test FAILED!");
-          tests &= (~TEST_S2C);
+        } else {
+          run_tests |= TEST_S2C;
         }
         break;
       case TEST_S2C_EXT:
         if (test_s2c_clt(ctlSocket, tests, host, conn_options, buf_size,
                          resultstr, &s2c_ThroughputSnapshots, jsonSupport, 1)) {
           log_println(0, "Extended S2C throughput test FAILED!");
-          tests &= (~TEST_S2C_EXT);
+        } else {
+          run_tests |= TEST_S2C_EXT;
         }
         break;
       case TEST_SFW:
         if (test_sfw_clt(ctlSocket, tests, host, conn_options, jsonSupport)) {
           log_println(0, "Simple firewall test FAILED!");
-          tests &= (~TEST_SFW);
+        } else {
+          run_tests |= TEST_SFW;
         }
         break;
       case TEST_META:
@@ -977,7 +987,8 @@ int main(int argc, char *argv[]) {
 
         if (test_meta_clt(ctlSocket, tests, host, conn_options, client_app_id, jsonSupport)) {
           log_println(0, "META test FAILED!");
-          tests &= (~TEST_META);
+        } else {
+          run_tests |= TEST_META;
         }
         break;
       default:
@@ -987,6 +998,9 @@ int main(int argc, char *argv[]) {
     ptr = strtok_r(NULL, " ", &strtokbuf);
   }
 
+  /* Make sure we only process tests that were run */
+  tests = run_tests;
+
   /* get the final results from server.
    *
    * The results are encapsulated by the MSG_RESULTS messages. The last

src/web100srv.h

@@ -323,8 +323,6 @@ int tcp_stat_autotune(int sock, tcp_stat_agent* agent, tcp_stat_connection cn);
 int tcp_stat_init(char *VarFileName);
 void tcp_stat_middlebox(int sock, tcp_stat_agent* agent, tcp_stat_connection cn, char *results_keys,
                         size_t results_keys_strlen, char *results_values, size_t results_strlen);
-int tcp_stat_setbuff(int sock, tcp_stat_agent* agent, tcp_stat_connection cn,
-                   int autotune);/* Not used so no web10g version */
 void tcp_stat_get_data_recv(int sock, tcp_stat_agent* agent,
                             tcp_stat_connection cn, int count_vars);