tests: Reduce risk of port collision and remove bash dependency.

A few tests need a random TCP port on which to listen for connections.
Until now, the tests have used the $RANDOM bash extension to do this, but
this runs the risk of occasionally colliding with an in-use port.  This
commit removes the bash dependency by switching to using a small Perl
program to pick random ports and reduces the risk of collision by
attempting to bind the port that it chooses.

Reported-by: Timothy Chen <tchen@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index 87bb7212bcd9f36859f932f79d2743c2c12bd89c..c19bbde5c2027d83ef5592c9f47facb3d6b48bec 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -127,6 +127,7 @@ Srini Seetharaman       seethara@stanford.edu
 Stephen Hemminger       shemminger@vyatta.com
 Takayuki HAMA           t-hama@cb.jp.nec.com
 Teemu Koponen           koponen@nicira.com
+Timothy Chen            tchen@nicira.com
 Vishal Swarankar        vishal.swarnkar@gmail.com
 Voravit T.              voravit@kth.se
 YAMAMOTO Takashi        yamamoto@valinux.co.jp

diff --git a/tests/automake.mk b/tests/automake.mk
index 37925823c3bccc9d92c7d995437845ee3ce91ef1..28b2749a3711f2c664ed8bc7fbcf9c0a80aaee83 100644 (file)
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -334,6 +334,8 @@ 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.
 EXTRA_DIST += \
        tests/test-daemon.py \

diff --git a/tests/choose-port.pl b/tests/choose-port.pl
new file mode 100644 (file)
index 0000000..46c8db5
--- /dev/null
+++ b/tests/choose-port.pl
@@ -0,0 +1,26 @@
+# -*- 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 bb00714e2b51ae28508e3988f4f514b299300988..3860b01dc2cc04917ef8919a62e91b4a0b73efe1 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -915,8 +915,8 @@ 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_SKIP_IF([test "x$RANDOM" = x])
-NETFLOW_PORT=`expr 32767 + \( $RANDOM % 32767 \)`
+AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
+NETFLOW_PORT=`cat stdout`
 
 OVS_VSWITCHD_START(
   [set Bridge br0 fail-mode=standalone -- \
@@ -957,8 +957,8 @@ AT_CLEANUP
 dnl Test that basic NetFlow reports active expirations correctly.
 AT_SETUP([ofproto-dpif - NetFlow active expiration])
 
-AT_SKIP_IF([test "x$RANDOM" = x])
-NETFLOW_PORT=`expr 32767 + \( $RANDOM % 32767 \)`
+AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
+NETFLOW_PORT=`cat stdout`
 
 OVS_VSWITCHD_START(
   [set Bridge br0 fail-mode=standalone -- \

diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 7d79d220d3e024a981fd91896c77b96dac65de38..ce558866571c16cb95353e9ac9a6df63c2768c07 100644 (file)
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -199,8 +199,6 @@ AT_CLEANUP
 AT_SETUP([SSL db: implementation])
 AT_KEYWORDS([ovsdb server positive ssl $5])
 AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
-AT_SKIP_IF([test "x$RANDOM" = x])
-SSL_PORT=`expr 32767 + \( $RANDOM % 32767 \)`
 PKIDIR=$abs_top_builddir/tests
 AT_SKIP_IF([expr "$PKIDIR" : ".*[      '\"
 \r\\]"])
@@ -223,6 +221,8 @@ 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`
 AT_CHECK(
   [ovsdb-server --detach --pidfile=$PWD/pid \
         --private-key=db:SSL,private_key \
@@ -390,10 +390,10 @@ m4_define([OVSDB_CHECK_EXECUTION],
   [AT_SETUP([$1])
    AT_KEYWORDS([ovsdb server positive ssl $5])
    AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
-   AT_SKIP_IF([test "x$RANDOM" = x])
    AT_DATA([schema], [$2
 ])
-   SSL_PORT=`expr 32767 + \( $RANDOM % 32767 \)`
+   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 --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])