[ovs-dev] [PATCH] tests: Reduce risk of port collision and remove bash dependency.
Ben Pfaff
blp at nicira.com
Fri Feb 3 00:37:42 UTC 2012
Good idea. I made that change and pushed this to master.
On Wed, Feb 01, 2012 at 05:35:38PM -0800, Ethan Jackson wrote:
> Doesn't matter at all, but seems like we'd want to use the range
> 49152–65535 as that's the IANA recommendation for this sort of thing.
>
> Looks good,
> Ethan
>
> On Mon, Jan 30, 2012 at 14:00, Ben Pfaff <blp at nicira.com> wrote:
> > 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 at nicira.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > AUTHORS | 1 +
> > tests/automake.mk | 2 ++
> > tests/choose-port.pl | 26 ++++++++++++++++++++++++++
> > tests/ofproto-dpif.at | 8 ++++----
> > tests/ovsdb-server.at | 8 ++++----
> > 5 files changed, 37 insertions(+), 8 deletions(-)
> > create mode 100644 tests/choose-port.pl
> >
> > diff --git a/AUTHORS b/AUTHORS
> > index 87bb721..c19bbde 100644
> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -127,6 +127,7 @@ Srini Seetharaman seethara at stanford.edu
> > Stephen Hemminger shemminger at vyatta.com
> > Takayuki HAMA t-hama at cb.jp.nec.com
> > Teemu Koponen koponen at nicira.com
> > +Timothy Chen tchen at nicira.com
> > Vishal Swarankar vishal.swarnkar at gmail.com
> > Voravit T. voravit at kth.se
> > YAMAMOTO Takashi yamamoto at valinux.co.jp
> > diff --git a/tests/automake.mk b/tests/automake.mk
> > index 9ae4c5c..49c0b92 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -325,6 +325,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
> > index 0000000..a9dd09f
> > --- /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(32767)) + 32768;
> > + 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 bb00714..3860b01 100644
> > --- 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 7d79d22..ce55886 100644
> > --- 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" : ".*[ '\"
> >
> > \\]"])
> > @@ -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])
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list