tests: Avoid race conditions, by letting the kernel choose ports to bind.

An occasionally occurring problem with "make check", especially when
parallel tests are enabled, is that multiple tests try to bind the same
TCP port and, of course, fail.  This happens because the code to select
a TCP port to bind just generates random numbers until it finds a port that
is not currently in use and uses the first one, which is of course prone
to races.

This commit changes the tests to let the kernel directly choose an
available port, which should avoid this type of failure.

Also, some of the tests that generated a random free TCP port actually
used the port number to bind a UDP socket, which of course doesn't work
well.  This commit fixes that problem too as a side effect.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/tests/automake.mk b/tests/automake.mk
index b9dbf3b..4442eb5 100644 (file)
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -301,8 +301,6 @@ noinst_PROGRAMS += tests/test-byte-order
 tests_test_byte_order_SOURCES = tests/test-byte-order.c
 tests_test_byte_order_LDADD = lib/libopenvswitch.a
 
-EXTRA_DIST += tests/choose-port.pl
-
 # Python tests.
 CHECK_PYFILES = \
        tests/appctl.py \

diff --git a/tests/choose-port.pl b/tests/choose-port.pl
deleted file mode 100644 (file)
index 46c8db5..0000000
--- a/tests/choose-port.pl
+++ /dev/null
@@ -1,26 +0,0 @@
-# -*- perl -*-
-
-# Picks a random TCP port and attempts to bind it, retrying a few
-# times if the chosen port is in use.  This is better than just
-# picking a random number without checking whether it is in use (but
-# of course a race window still exists).
-#
-# On success, prints a port number on stdout and exits with status 0.
-# On failure, prints an error on stderr and exits with a nonzero status.
-
-use warnings;
-use strict;
-use Socket;
-
-socket(SOCK, PF_INET, SOCK_STREAM, 0) || die "socket: $!\n";
-for (my ($i) = 0; ; $i++) {
-    my ($port) = int(rand(16383)) + 49152;
-    if (bind(SOCK, sockaddr_in($port, INADDR_ANY))) {
-        print "$port\n";
-        exit 0;
-    } elsif ($i < 10 && $!{EADDRINUSE}) {
-        # Address already in use.  Try again.
-    } else {
-        die "bind: $!\n";
-    }
-}

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index b1d2f26..0de941e 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1208,10 +1208,12 @@ AT_CLEANUP
 
 dnl Test that sFlow samples packets correctly.
 AT_SETUP([ofproto-dpif - sFlow packet sampling])
-AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-SFLOW_PORT=`cat stdout`
 OVS_VSWITCHD_START([set Bridge br0 fail-mode=standalone])
 
+AT_CHECK([test-sflow --log-file --detach --no-chdir --pidfile 0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CAPTURE_FILE([sflow.log])
+SFLOW_PORT=`parse_listening_port < test-sflow.log`
+
 ovs-appctl time/stop
 
 ADD_OF_PORTS([br0], 1, 2)
@@ -1223,8 +1225,6 @@ ovs-vsctl \
    --id=@sf create sflow targets=\"127.0.0.1:$SFLOW_PORT\" \
      header=128 sampling=1 polling=1
 ON_EXIT([kill `cat test-sflow.pid`])
-AT_CHECK([test-sflow --detach --no-chdir --pidfile $SFLOW_PORT:127.0.0.1 > sflow.log])
-AT_CAPTURE_FILE([sflow.log])
 
 dnl open with ARP packets to seed the bridge-learning.  The output
 dnl ifIndex numbers should be reported predictably after that.
@@ -1504,20 +1504,19 @@ dnl - Flow actions changing (in this case, due to MAC learning)
 dnl   cause a record to be sent.
 AT_SETUP([ofproto-dpif - NetFlow flow expiration])
 
-AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-NETFLOW_PORT=`cat stdout`
-
 OVS_VSWITCHD_START([set Bridge br0 fail-mode=standalone])
 ADD_OF_PORTS([br0], 1, 2)
+
+ON_EXIT([kill `cat test-netflow.pid`])
+AT_CHECK([test-netflow --log-file --detach --no-chdir --pidfile 0:127.0.0.1 > netflow.log], [0], [], [ignore])
+AT_CAPTURE_FILE([netflow.log])
+NETFLOW_PORT=`parse_listening_port < test-netflow.log`
+
 ovs-vsctl \
    set Bridge br0 netflow=@nf -- \
    --id=@nf create NetFlow targets=\"127.0.0.1:$NETFLOW_PORT\" \
      engine_id=1 engine_type=2 active_timeout=30 add-id-to-interface=false
 
-ON_EXIT([kill `cat test-netflow.pid`])
-AT_CHECK([test-netflow --detach --no-chdir --pidfile $NETFLOW_PORT:127.0.0.1 > netflow.log])
-AT_CAPTURE_FILE([netflow.log])
-
 for delay in 1000 30000; do
     ovs-appctl netdev-dummy/receive p1 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'
     ovs-appctl netdev-dummy/receive p2 'in_port(1),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=0,code=0)'
@@ -1546,19 +1545,19 @@ AT_CLEANUP
 dnl Test that basic NetFlow reports active expirations correctly.
 AT_SETUP([ofproto-dpif - NetFlow active expiration])
 
-AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-NETFLOW_PORT=`cat stdout`
-
 OVS_VSWITCHD_START([set Bridge br0 fail-mode=standalone])
 ADD_OF_PORTS([br0], 1, 2)
+
+ON_EXIT([kill `cat test-netflow.pid`])
+AT_CHECK([test-netflow --log-file --detach --no-chdir --pidfile 0:127.0.0.1 > netflow.log], [0], [], [ignore])
+AT_CAPTURE_FILE([netflow.log])
+NETFLOW_PORT=`parse_listening_port < test-netflow.log`
+
 ovs-vsctl \
    set Bridge br0 netflow=@nf -- \
    --id=@nf create NetFlow targets=\"127.0.0.1:$NETFLOW_PORT\" \
      engine_id=1 engine_type=2 active_timeout=10 add-id-to-interface=false
 
-ON_EXIT([kill `cat test-netflow.pid`])
-AT_CHECK([test-netflow --detach --no-chdir --pidfile $NETFLOW_PORT:127.0.0.1 > netflow.log])AT_CAPTURE_FILE([netflow.log])
-
 AT_CHECK([ovs-appctl time/stop])
 n=1
 while test $n -le 60; do

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 9a6d5ab..cad0080 100644 (file)
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -13,6 +13,23 @@ s/ n_bytes=0,//
 s/ idle_age=[0-9]*,//
 s/ hard_age=[0-9]*,//
 '
+}
+
+# parse_listening_port [SERVER]
+#
+# Parses the TCP or SSL port on which a server is listening from the log,
+# given that the server was told to listen on a kernel-chosen port,
+# file provided on stdin, and prints the port number on stdout.
+#
+# Here's an example of how to use this with ovsdb-server:
+#
+#    OVS_LOGDIR=`pwd`; export OVS_LOGDIR
+#    ovsdb-server --log-file --remote=ptcp:0:127.0.0.1 ...
+#    TCP_PORT=`parse_listening_port < ovsdb-server.log`
+#
+# (Also works with pssl: in place of ptcp:.)
+parse_listening_port () {
+    sed -n 's/.*0:127\.0\.0\.1: listening on port \([0-9]*\)$/\1/p'
 }]
 m4_divert_pop([PREPARE_TESTS])
 

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 3c32e2f..21a22db 100644 (file)
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -57,11 +57,12 @@ m4_define([OVSDB_CHECK_IDL_TCP_PY],
    AT_SKIP_IF([test $HAVE_PYTHON = no])
    AT_KEYWORDS([ovsdb server idl positive Python with tcp socket $5])
    OVS_RUNDIR=`pwd`; export OVS_RUNDIR
+   OVS_LOGDIR=`pwd`; export OVS_LOGDIR
    AT_CHECK([ovsdb-tool create db $abs_srcdir/idltest.ovsschema],
                   [0], [stdout], [ignore])
-   AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-   TCP_PORT=`cat stdout`
-   AT_CHECK([ovsdb-server '-vPATTERN:console:ovsdb-server|%c|%m' --detach --no-chdir --pidfile="`pwd`"/pid --remote=ptcp:$TCP_PORT:127.0.0.1 --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
+   AT_CHECK([ovsdb-server --log-file '-vPATTERN:console:ovsdb-server|%c|%m' --detach --no-chdir --pidfile="`pwd`"/pid --remote=punix:socket --remote=ptcp:0:127.0.0.1 --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
+   TCP_PORT=`parse_listening_port < ovsdb-server.log`
+
    m4_if([$2], [], [],
      [AT_CHECK([ovsdb-client transact tcp:127.0.0.1:$TCP_PORT $2], [0], [ignore], [ignore], [kill `cat pid`])])
    AT_CHECK([$PYTHON $srcdir/test-ovsdb.py  -t10 idl $srcdir/idltest.ovsschema tcp:127.0.0.1:$TCP_PORT $3],

diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 50f95bd..1f0deca 100644 (file)
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -296,15 +296,15 @@ AT_CHECK(
                 "certificate": "'"$PKIDIR/testpki-cert2.pem"'",
                 "ca_cert": "'"$PKIDIR/testpki-cacert.pem"'"}}]']],
   [0], [ignore], [ignore])
-AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-SSL_PORT=`cat stdout`
+OVS_LOGDIR=`pwd`; export OVS_LOGDIR
 AT_CHECK(
-  [ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid \
+  [ovsdb-server --log-file --detach --no-chdir --pidfile="`pwd`"/pid \
         --private-key=db:SSL,private_key \
         --certificate=db:SSL,certificate \
         --ca-cert=db:SSL,ca_cert \
-         --remote=pssl:$SSL_PORT:127.0.0.1 --unixctl="`pwd`"/unixctl db],
+        --remote=pssl:0:127.0.0.1 --unixctl="`pwd`"/unixctl db],
   [0], [ignore], [ignore])
+SSL_PORT=`parse_listening_port < ovsdb-server.log`
 AT_CHECK(
   [[ovsdb-client \
         --private-key=$PKIDIR/testpki-privkey.pem \
@@ -479,12 +479,12 @@ m4_define([OVSDB_CHECK_EXECUTION],
    AT_KEYWORDS([ovsdb server positive ssl $5])
    AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
    OVS_RUNDIR=`pwd`; export OVS_RUNDIR
+   OVS_LOGDIR=`pwd`; export OVS_LOGDIR
    $2 > schema
-   AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-   SSL_PORT=`cat stdout`
    PKIDIR=$abs_top_builddir/tests
    AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
-   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid --private-key=$PKIDIR/testpki-privkey2.pem --certificate=$PKIDIR/testpki-cert2.pem --ca-cert=$PKIDIR/testpki-cacert.pem --remote=pssl:$SSL_PORT:127.0.0.1 --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
+   AT_CHECK([ovsdb-server --log-file --detach --no-chdir --pidfile="`pwd`"/pid --private-key=$PKIDIR/testpki-privkey2.pem --certificate=$PKIDIR/testpki-cert2.pem --ca-cert=$PKIDIR/testpki-cacert.pem --remote=pssl:0:127.0.0.1 --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
+   SSL_PORT=`parse_listening_port < ovsdb-server.log`
    m4_foreach([txn], [$3], 
      [AT_CHECK([ovsdb-client --private-key=$PKIDIR/testpki-privkey.pem --certificate=$PKIDIR/testpki-cert.pem --ca-cert=$PKIDIR/testpki-cacert.pem transact ssl:127.0.0.1:$SSL_PORT 'txn'], [0], [stdout], [ignore],
      [test ! -e pid || kill `cat pid`])
@@ -502,10 +502,10 @@ AT_BANNER([OVSDB -- ovsdb-server transactions (TCP sockets)])
 AT_SETUP([ovsdb-client get-schema-version - tcp socket])
 AT_KEYWORDS([ovsdb server positive tcp])
 ordinal_schema > schema
-AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-TCP_PORT=`cat stdout`
 AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])
-AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid --unixctl="`pwd`"/unixctl --remote=ptcp:$TCP_PORT:127.0.0.1 db], [0], [ignore], [ignore])
+OVS_LOGDIR=`pwd`; export OVS_LOGDIR
+AT_CHECK([ovsdb-server --log-file --detach --no-chdir --pidfile="`pwd`"/pid --unixctl="`pwd`"/unixctl --remote=ptcp:0:127.0.0.1 db], [0], [ignore], [ignore])
+TCP_PORT=`parse_listening_port < ovsdb-server.log`
 AT_CHECK([ovsdb-client get-schema-version tcp:127.0.0.1:$TCP_PORT ordinals], [0], [5.1.3
 ])
 OVSDB_SERVER_SHUTDOWN
@@ -529,12 +529,12 @@ m4_define([OVSDB_CHECK_EXECUTION],
   [AT_SETUP([$1])
    AT_KEYWORDS([ovsdb server positive tcp $5])
    OVS_RUNDIR=`pwd`; export OVS_RUNDIR
+   OVS_LOGDIR=`pwd`; export OVS_LOGDIR
    $2 > schema
-   AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
-   TCP_PORT=`cat stdout`
    PKIDIR=$abs_top_builddir/tests
    AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
-   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid --remote=ptcp:$TCP_PORT:127.0.0.1 --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
+   AT_CHECK([ovsdb-server --log-file --detach --no-chdir --pidfile="`pwd`"/pid --remote=ptcp:0:127.0.0.1 --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
+   TCP_PORT=`parse_listening_port < ovsdb-server.log`
    m4_foreach([txn], [$3],
      [AT_CHECK([ovsdb-client transact tcp:127.0.0.1:$TCP_PORT 'txn'], [0], [stdout], [ignore],
      [test ! -e pid || kill `cat pid`])