From 1e04fcc879ea09bb8c7512cda6de91f7725627f6 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 3 Apr 2013 13:30:18 -0500 Subject: [PATCH] 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 --- tests/automake.mk | 2 -- tests/choose-port.pl | 26 -------------------------- tests/ofproto-dpif.at | 33 ++++++++++++++++----------------- tests/ofproto-macros.at | 17 +++++++++++++++++ tests/ovsdb-idl.at | 7 ++++--- tests/ovsdb-server.at | 26 +++++++++++++------------- 6 files changed, 50 insertions(+), 61 deletions(-) delete mode 100644 tests/choose-port.pl diff --git a/tests/automake.mk b/tests/automake.mk index b9dbf3bd7..4442eb509 100644 --- 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 index 46c8db5ed..000000000 --- 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 b1d2f2600..0de941ea2 100644 --- 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 9a6d5ab21..cad00804a 100644 --- 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 3c32e2f56..21a22dbb8 100644 --- 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 50f95bda2..1f0deca63 100644 --- 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`]) -- 2.43.0