[ovs-dev] [PATCH 3/3] tests: Avoid race conditions, by letting the kernel choose ports to bind.
Neil Mckee
neil.mckee at inmon.com
Thu Apr 4 00:01:40 UTC 2013
"parse-listening-port" seems to return the port that the server was told to listen on(?) But that bears no relation to the UDP port that sFlow or NetFlow might be sent out to. The way you have it now will probably work, but only by accident: because you can usually open UDP port X at the same time as someone else has opened TCP port X.
Or did I miss something?
Neil
On Apr 3, 2013, at 2:23 PM, Ben Pfaff wrote:
> I didn't notice that, but this commit actually removes choose-port.pl
> entirely so it presumably fixes that problem?
>
> I'll add a note to the commit message to mention that it fixes that
> problem too.
>
> On Wed, Apr 03, 2013 at 01:56:03PM -0700, Neil Mckee wrote:
>> You probably noticed this, but choose-port.pl is sometimes being used
>> to return a free TCP port.... which is promptly used for UDP. Perhaps
>> choose-port.pl should take an argument?
>>
>> Neil
>>
>> On Apr 3, 2013, at 12:02 PM, Ben Pfaff wrote:
>>
>>> 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.
>>>
>>> Signed-off-by: Ben Pfaff <blp at nicira.com>
>>> ---
>>> 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 275ff53..aad8692 100644
>>> --- a/tests/automake.mk
>>> +++ b/tests/automake.mk
>>> @@ -300,8 +300,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 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 06ebf23..857e6ab 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 9a6d5ab..cad0080 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 3c32e2f..21a22db 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 62eae38..9cabab2 100644
>>> --- a/tests/ovsdb-server.at
>>> +++ b/tests/ovsdb-server.at
>>> @@ -254,15 +254,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 \
>>> @@ -437,12 +437,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`])
>>> @@ -460,10 +460,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
>>> @@ -487,12 +487,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`])
>>> --
>>> 1.7.10.4
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev at openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
>>
More information about the dev
mailing list