[ovs-dev] [PATCHv2] ovs-test: Enhancements to the ovs-test tool

Ansis Atteka aatteka at nicira.com
Mon Apr 16 12:32:11 PDT 2012


On Fri, Apr 13, 2012 at 8:00 PM, Reid Price <reid at nicira.com> wrote:

>
>
> On Fri, Apr 13, 2012 at 7:26 PM, Ansis Atteka <aatteka at nicira.com> wrote:
>
>>
>>
>> On Wed, Apr 11, 2012 at 3:10 PM, Reid Price <reid at nicira.com> wrote:
>>
>>> Sure thing, a few notes inline.  Looks good overall.
>>>
>>> On Tue, Apr 10, 2012 at 6:02 PM, Ansis Atteka <aatteka at nicira.com>wrote:
>>>
>>>> -Implemented support for ovs-test client, so that it could automatically
>>>> spawn an ovs-test server process from itself. This reduces the number of
>>>> commands the user have to type to get tests running.
>>>> -Automated creation of OVS bridges and ports (for VLAN and GRE tests),
>>>> so that
>>>> user would not need to invoke ovs-vsctl manually to switch from direct,
>>>> 802.1Q
>>>> and GRE tests.
>>>> -Fixed some pylint reported warnings.
>>>> -Fixed ethtool invocation so that we always try to query the physical
>>>> interface
>>>> to get the driver name and version.
>>>> -and some others enhancements.
>>>>
>>>> The new usage:
>>>> Node1:ovs-test -s 15531
>>>> Node2:ovs-test -c 127.0.0.1,1.1.1.1 192.168.122.151,1.1.1.2 -d -l 125
>>>> -t gre
>>>>
>>>> Signed-off-by: Ansis Atteka <aatteka at nicira.com>
>>>> ---
>>>>  NEWS                        |    4 +
>>>>  python/automake.mk          |    3 +-
>>>>  python/ovstest/args.py      |  156 +++++++++++++++++++------
>>>>  python/ovstest/rpcserver.py |  156 ++++++++++++++++++++++++-
>>>>  python/ovstest/tcp.py       |   23 +----
>>>>  python/ovstest/udp.py       |   10 +--
>>>>  python/ovstest/util.py      |   82 +++++++++++--
>>>>  python/ovstest/vswitch.py   |   97 ++++++++++++++++
>>>>  utilities/ovs-test.8.in     |  152 ++++++++++++++-----------
>>>>  utilities/ovs-test.in       |  262
>>>> ++++++++++++++++++++++++++++++++++++++-----
>>>>  10 files changed, 763 insertions(+), 182 deletions(-)
>>>>  create mode 100644 python/ovstest/vswitch.py
>>>>
>>>> diff --git a/NEWS b/NEWS
>>>> index ed3fc88..ac00335 100644
>>>> --- a/NEWS
>>>> +++ b/NEWS
>>>> @@ -8,6 +8,10 @@ post-v1.6.0
>>>>       Internetwork Control (0xc0).
>>>>     - Added the granular link health statistics, 'cfm_health', to an
>>>>       interface.
>>>> +    - ovs-test:
>>>> +        - Added support for spawning ovs-test server from the client.
>>>> +        - Now ovs-test is able to automatically create test bridges
>>>> and ports.
>>>> +
>>>>
>>>>
>>>>  v1.6.0 - xx xxx xxxx
>>>> diff --git a/python/automake.mk b/python/automake.mk
>>>> index b60e26b..59db000 100644
>>>> --- a/python/automake.mk
>>>> +++ b/python/automake.mk
>>>> @@ -6,7 +6,8 @@ ovstest_pyfiles = \
>>>>        python/ovstest/rpcserver.py \
>>>>        python/ovstest/tcp.py \
>>>>        python/ovstest/udp.py \
>>>> -       python/ovstest/util.py
>>>> +       python/ovstest/util.py \
>>>> +       python/ovstest/vswitch.py
>>>>
>>>>  ovs_pyfiles = \
>>>>        python/ovs/__init__.py \
>>>> diff --git a/python/ovstest/args.py b/python/ovstest/args.py
>>>> index d6b4756..39a32b3 100644
>>>> --- a/python/ovstest/args.py
>>>> +++ b/python/ovstest/args.py
>>>> @@ -1,4 +1,4 @@
>>>> -# Copyright (c) 2011 Nicira Networks
>>>> +# Copyright (c) 2011, 2012 Nicira Networks
>>>>  #
>>>>  # Licensed under the Apache License, Version 2.0 (the "License");
>>>>  # you may not use this file except in compliance with the License.
>>>> @@ -19,9 +19,10 @@ ovsargs provide argument parsing for ovs-test utility
>>>>  import argparse
>>>>  import socket
>>>>  import re
>>>> +import sys
>>>>
>>> Google style likes these alphabetized (one line higher)
>>>
>>>
>>>>
>>>>
>>>> -def ip(string):
>>>> +def ip_address(string):
>>>>     """Verifies if string is a valid IP address"""
>>>>     try:
>>>>         socket.inet_aton(string)
>>>> @@ -30,8 +31,28 @@ def ip(string):
>>>>     return string
>>>>
>>>>
>>>> +def ip_optional_mask(string):
>>>> +    """
>>>> +    Verifies if string contains a valid IP address and an optional
>>>> mask in
>>>> +    CIDR notation.
>>>> +    """
>>>> +    token = string.split("/")
>>>> +    if len(token) > 2:
>>>> +        raise argparse.ArgumentTypeError("IP address and netmask must
>>>> be "
>>>> +                                         "separated by a single slash")
>>>> +    elif len(token) == 2:
>>>> +        try:
>>>> +            mask = int(token[1])
>>>> +        except ValueError:
>>>> +            raise argparse.ArgumentTypeError("Netmask is not a valid
>>>> integer")
>>>> +        if mask < 0 or mask > 31:
>>>> +            raise argparse.ArgumentTypeError("Netmask must be in range
>>>> 0..31")
>>>> +    ip_address(token[0])
>>>> +    return string
>>>> +
>>>> +
>>>>  def port(string):
>>>> -    """Convert a string into a Port (integer)"""
>>>> +    """Convert a string into a TCP/UDP Port (integer)"""
>>>>     try:
>>>>         port_number = int(string)
>>>>         if port_number < 1 or port_number > 65535:
>>>> @@ -41,39 +62,76 @@ def port(string):
>>>>     return port_number
>>>>
>>>>
>>>> -def ip_optional_port(string, default_port):
>>>> +def ip_optional_port(string, default_port, ip_callback):
>>>>     """Convert a string into IP and Port pair. If port was absent then
>>>> use
>>>> -    default_port as the port"""
>>>> +    default_port as the port. The third argument is a callback that
>>>> verifies
>>>> +    whether IP address is given in correct format."""
>>>>     value = string.split(':')
>>>>     if len(value) == 1:
>>>> -        return (ip(value[0]), default_port)
>>>> +        return (ip_callback(value[0]), default_port)
>>>>     elif len(value) == 2:
>>>> -        return (ip(value[0]), port(value[1]))
>>>> +        return (ip_callback(value[0]), port(value[1]))
>>>>     else:
>>>>         raise argparse.ArgumentTypeError("IP address from the optional
>>>> Port "
>>>>                                          "must be colon-separated")
>>>>
>>>>
>>>> +def vlan_tag(string):
>>>> +    """
>>>> +    This function verifies whether given string is a correct VLAN tag.
>>>> +    """
>>>> +    try:
>>>> +        value = int(string)
>>>> +        if value < 1 or value > 4094:
>>>> +            raise argparse.ArgumentTypeError("Not a valid VLAN tag. "
>>>> +                                             "VLAN tag should be in
>>>> the "
>>>> +                                             "range 1..4094.")
>>>> +        return string
>>>> +    except ValueError:
>>>> +        raise argparse.ArgumentTypeError("VLAN tag is not a valid
>>>> integer")
>>>>
>>> Nit, would shrink the try/except to cover just the integer casting
>>>
>>>
>>>> +
>>>>
>>>>  def server_endpoint(string):
>>>> -    """Converts a string in
>>>> ControlIP[:ControlPort][,TestIP[:TestPort]] format
>>>> +    """Converts a string OuterIP[:OuterPort],InnerIP[/Mask][:InnerPort]
>>>>     into a 4-tuple, where:
>>>> -    1. First element is ControlIP
>>>> -    2. Second element is ControlPort (if omitted will use default
>>>> value 15531)
>>>> -    3  Third element is TestIP (if omitted will be the same as
>>>> ControlIP)
>>>> -    4. Fourth element is TestPort (if omitted will use default value
>>>> 15532)"""
>>>> +    1. First element is OuterIP
>>>> +    2. Second element is OuterPort (if omitted will use default value
>>>> 15531)
>>>> +    3  Third element is InnerIP with optional mask
>>>> +    4. Fourth element is InnerPort (if omitted will use default value
>>>> 15532)
>>>> +    """
>>>>     value = string.split(',')
>>>> -    if len(value) == 1: #  TestIP and TestPort are not present
>>>> -        ret = ip_optional_port(value[0], 15531)
>>>> -        return (ret[0], ret[1], ret[0], 15532)
>>>> -    elif len(value) == 2:
>>>> -        ret1 = ip_optional_port(value[0], 15531)
>>>> -        ret2 = ip_optional_port(value[1], 15532)
>>>> +    if len(value) == 2:
>>>> +        ret1 = ip_optional_port(value[0], 15531, ip_address)
>>>> +        ret2 = ip_optional_port(value[1], 15532, ip_optional_mask)
>>>>
>>> Might be worth storing some of these magic values as variables somewhere,
>>> if you ever reference them again.
>>>
>>>          return (ret1[0], ret1[1], ret2[0], ret2[1])
>>>>     else:
>>>> -        raise argparse.ArgumentTypeError("ControlIP:ControlPort and
>>>> TestIP:"
>>>> -                                         "TestPort must be comma "
>>>> -                                         "separated")
>>>> +        raise argparse.ArgumentTypeError("OuterIP:OuterPort and
>>>> InnerIP/Mask:"
>>>> +                                         "InnerPort must be comma
>>>> separated")
>>>> +
>>>> +
>>>> +class UniqueServerAction(argparse.Action):
>>>> +    """
>>>> +    This custom action class will prevent user from entering multiple
>>>> ovs-test
>>>> +    servers with the same OuterIP. If there is an server with
>>>> 127.0.0.1 outer
>>>> +    IP address then it will be inserted in the front of the list.
>>>> +    """
>>>> +    def __call__(self, parser, namespace, values, option_string=None):
>>>> +        outer_ips = set()
>>>> +        endpoints = []
>>>> +        for server in values:
>>>> +            try:
>>>> +                endpoint = server_endpoint(server)
>>>> +            except argparse.ArgumentTypeError:
>>>> +                raise argparse.ArgumentError(self,
>>>> str(sys.exc_info()[1]))
>>>> +            if endpoint[0] in outer_ips:
>>>> +                raise argparse.ArgumentError(self, "Duplicate OuterIPs
>>>> found")
>>>> +            else:
>>>> +                outer_ips.add(endpoint[0])
>>>> +                if endpoint[0] == "127.0.0.1":
>>>> +                    endpoints.insert(0, endpoint)
>>>> +                else:
>>>> +                    endpoints.append(endpoint)
>>>> +        setattr(namespace, self.dest, endpoints)
>>>>
>>>>
>>>>  def bandwidth(string):
>>>> @@ -86,30 +144,54 @@ def bandwidth(string):
>>>>     return long(bwidth) / 8 #  Convert from bits to bytes
>>>>
>>>>
>>>> +def tunnel_types(string):
>>>> +    """
>>>> +    This function converts a string into a list that contains all
>>>> tunnel types
>>>> +    that user intended to test.
>>>> +    """
>>>> +    return string.split(',')
>>>> +
>>>> +
>>>>  def ovs_initialize_args():
>>>> -    """Initialize args for ovstest utility"""
>>>> -    parser = argparse.ArgumentParser(description = 'Test ovs
>>>> connectivity')
>>>> +    """
>>>> +    Initialize argument parsing for ovs-test utility.
>>>> +    """
>>>>
>>>
>>> For all of the kwargs below, the usual style is to omit the spaces on
>>> either
>>> side of key=value pairs.
>>>
>>>
>>>> +    parser = argparse.ArgumentParser(description = 'Test connectivity '
>>>> +                                                'between two Open
>>>> vSwitches.')
>>>> +
>>>>     parser.add_argument('-v', '--version', action = 'version',
>>>>                 version = 'ovs-test (Open vSwitch) @VERSION@')
>>>> +
>>>>     parser.add_argument("-b", "--bandwidth", action = 'store',
>>>>                 dest = "targetBandwidth", default = "1M", type =
>>>> bandwidth,
>>>> -                help = 'target bandwidth for UDP tests in bits/second.
>>>> Use '
>>>> +                help = 'Target bandwidth for UDP tests in bits/second.
>>>> Use '
>>>>                 'postfix M or K to alter unit magnitude.')
>>>> +    parser.add_argument("-i", "--interval", action = 'store',
>>>> +                dest = "testInterval", default = "5", type = int,
>>>>
>>> Nit, 5 doesn't need to be in string format
>>>
>>>
>>>> +                help = 'Interval for how long to run each test in
>>>> seconds.')
>>>> +
>>>> +    parser.add_argument("-t", "--tunnel-modes", action = 'store',
>>>> +                dest = "tunnelModes", default = [], type =
>>>> tunnel_types,
>>>>
>>> Do you ever parse arguments multiple times, or modify this?  If it
>>> doesn't
>>> break anything else, might want to use () instead of [].  I'm mostly just
>>> paranoid, those bugs are tricky to catch if they do happen.
>>>
>>>
>>>> +                help = 'Do L3 tests with the given tunnel modes.')
>>>> +    parser.add_argument("-l", "--vlan-tag", action = 'store',
>>>> +                dest = "vlanTag", default = None, type = vlan_tag,
>>>> +                help = 'Do VLAN tests and use the given VLAN tag.')
>>>> +    parser.add_argument("-d", "--direct", action = 'store_true',
>>>> +                dest = "direct", default = None,
>>>> +                help = 'Do direct tests between both ovs-test
>>>> servers.')
>>>> +
>>>>     group = parser.add_mutually_exclusive_group(required = True)
>>>>     group.add_argument("-s", "--server", action = "store", dest =
>>>> "port",
>>>>                 type = port,
>>>> -                help = 'run in server mode and wait client to connect
>>>> to this '
>>>> -                'port')
>>>> -    group.add_argument('-c', "--client", action = "store", nargs = 2,
>>>> -                dest = "servers", type = server_endpoint,
>>>> +                help = 'Run in server mode and wait for the client to '
>>>> +                'connect to this port.')
>>>> +    group.add_argument('-c', "--client", nargs = 2,
>>>> +                dest = "servers", action = UniqueServerAction,
>>>>                 metavar = ("SERVER1", "SERVER2"),
>>>> -                help = 'run in client mode and do tests between these '
>>>> -                'two servers. Each server must be specified in
>>>> following '
>>>> -                'format - ControlIP[:ControlPort][,TestIP[:TestPort]].
>>>> If '
>>>> -                'TestIP is omitted then ovs-test server will also use
>>>> the '
>>>> -                'ControlIP for testing purposes. ControlPort is TCP
>>>> port '
>>>> -                'where server will listen for incoming XML/RPC control
>>>> '
>>>> -                'connections to schedule tests (by default 15531).
>>>> TestPort '
>>>> -                'is port which will be used by server to send test
>>>> traffic '
>>>> -                '(by default 15532)')
>>>> +                help = 'Run in client mode and do tests between these '
>>>> +                'two ovs-test servers. Each server must be specified
>>>> in '
>>>> +                'following format - OuterIP:OuterPort,InnerIP[/mask] '
>>>> +                ':InnerPort. It is possible to start local instance of
>>>> '
>>>> +                'ovs-test server in the client mode by using 127.0.0.1
>>>> as '
>>>> +                'OuterIP.')
>>>>     return parser.parse_args()
>>>> diff --git a/python/ovstest/rpcserver.py b/python/ovstest/rpcserver.py
>>>> index 41d2569..9271c2b 100644
>>>> --- a/python/ovstest/rpcserver.py
>>>> +++ b/python/ovstest/rpcserver.py
>>>> @@ -1,4 +1,4 @@
>>>> -# Copyright (c) 2011 Nicira Networks
>>>> +# Copyright (c) 2011, 2012 Nicira Networks
>>>>  #
>>>>  # Licensed under the Apache License, Version 2.0 (the "License");
>>>>  # you may not use this file except in compliance with the License.
>>>> @@ -21,13 +21,15 @@ from twisted.web import xmlrpc, server
>>>>  from twisted.internet.error import CannotListenError
>>>>  import udp
>>>>  import tcp
>>>> -import args
>>>>  import util
>>>> -
>>>> +import vswitch
>>>> +import xmlrpclib
>>>> +import sys
>>>> +import exceptions
>>>>
>>> Alphabetize again.  I have a slight personal preference for segregating
>>> builtin modules from local ones with a blank line, think that is in
>>> google
>>> style too, not sure about pep8.
>>>
>>>
>>>>  class TestArena(xmlrpc.XMLRPC):
>>>>     """
>>>> -    This class contains all the functions that ovstest will call
>>>> +    This class contains all the functions that ovs-test client will
>>>> call
>>>>     remotely. The caller is responsible to use designated handleIds
>>>>     for designated methods (e.g. do not mix UDP and TCP handles).
>>>>     """
>>>> @@ -36,6 +38,9 @@ class TestArena(xmlrpc.XMLRPC):
>>>>         xmlrpc.XMLRPC.__init__(self)
>>>>         self.handle_id = 1
>>>>         self.handle_map = {}
>>>> +        self.bridges = set()
>>>> +        self.ports = set()
>>>> +        self.request = None
>>>>
>>>>     def __acquire_handle(self, value):
>>>>         """
>>>> @@ -58,6 +63,40 @@ class TestArena(xmlrpc.XMLRPC):
>>>>         """
>>>>         del self.handle_map[handle]
>>>>
>>>> +    def cleanup(self):
>>>> +        """
>>>> +        Delete all remaining bridges and ports if ovs-test client did
>>>> not had
>>>> +        a chance to remove them. It is necessary to call this function
>>>> if
>>>> +        ovs-test server is abruptly terminated when doing the tests.
>>>> +        """
>>>> +        for port in self.ports:
>>>> +            vswitch.ovs_vsctl_del_port_from_bridge(port)
>>>> +
>>>> +        for bridge in self.bridges:
>>>> +            vswitch.ovs_vsctl_del_bridge(bridge)
>>>> +
>>>> +    def render(self, request):
>>>> +        """
>>>> +        This method overrides the original XMLRPC.render method so
>>>> that it
>>>> +        would be possible to get the XML RPC client IP address from the
>>>> +        request object.
>>>> +        """
>>>> +        self.request = request
>>>> +        return xmlrpc.XMLRPC.render(self, request)
>>>> +
>>>> +    def xmlrpc_get_my_address(self):
>>>> +        """
>>>> +        Returns the RPC client's IP address.
>>>> +        """
>>>> +        return self.request.getClientIP()
>>>>
>>> I assume this is in a fairly structured environment where you aren't
>>> worried
>>> about thread-safety or other trickery.
>>>
>>>
>>>> +
>>>> +    def xmlrpc_get_my_address_from(self, his_ip, his_port):
>>>> +        """
>>>> +        Returns the ovs-test server IP address that the other ovs-test
>>>> server
>>>> +        with the given ip will see.
>>>> +        """
>>>> +        server1 = xmlrpclib.Server("http://%s:%u/" % (his_ip,
>>>> his_port))
>>>> +        return server1.get_my_address()
>>>
>>>
>>>>     def xmlrpc_create_udp_listener(self, port):
>>>>         """
>>>> @@ -171,6 +210,98 @@ class TestArena(xmlrpc.XMLRPC):
>>>>             return -1
>>>>         return 0
>>>>
>>>> +    def xmlrpc_create_test_bridge(self, bridge, iface):
>>>> +        """
>>>> +        This function creates a physical bridge from iface. It moves
>>>> the
>>>> +        IP configuration from the physical interface to the bridge.
>>>> +        """
>>>> +        ret = vswitch.ovs_vsctl_add_bridge(bridge)
>>>> +        if ret == 0:
>>>> +            self.bridges.add(bridge)
>>>> +            util.interface_up(bridge)
>>>> +            (ip_addr, mask) = util.interface_get_ip(iface)
>>>> +            util.interface_assign_ip(bridge, ip_addr, mask)
>>>> +            util.interface_assign_ip(iface, "0.0.0.0",
>>>> "255.255.255.255")
>>>> +            ret = vswitch.ovs_vsctl_add_port_to_bridge(bridge, iface)
>>>> +            if ret == 0:
>>>> +                self.ports.add(iface)
>>>>
>>> Is it the responsibility of this function to set the IP back to the
>>> interface
>>> if the add_port call fails?  I don't see anything in create_bridge to do
>>> so.
>>>
>>>
>>>> +        return ret
>>>> +
>>>> +    def xmlrpc_del_test_bridge(self, bridge, iface):
>>>> +        """
>>>> +        This function deletes the test bridge and moves its IP
>>>> configuration
>>>> +        back to the physical interface.
>>>> +        """
>>>> +        (ip_addr, mask) = util.interface_get_ip(bridge)
>>>> +        util.interface_assign_ip(iface, ip_addr, mask)
>>>> +        ret = vswitch.ovs_vsctl_del_bridge(bridge)
>>>> +        return ret
>>>> +
>>>> +    def xmlrpc_get_iface_from_bridge(self, brname):
>>>> +        """
>>>> +        Tries to figure out physical interface from bridge.
>>>> +        """
>>>> +        return vswitch.ovs_get_physical_interface(brname)
>>>> +
>>>> +    def xmlrpc_create_bridge(self, brname):
>>>> +        """
>>>> +        Creates an OVS bridge.
>>>> +        """
>>>> +        ret = vswitch.ovs_vsctl_add_bridge(brname)
>>>> +        if ret == 0:
>>>> +            self.bridges.add(brname)
>>>> +        return ret
>>>> +
>>>> +    def xmlrpc_del_bridge(self, brname):
>>>> +        """
>>>> +        Deletes an OVS bridge.
>>>> +        """
>>>> +        ret = vswitch.ovs_vsctl_del_bridge(brname)
>>>> +        if ret == 0:
>>>> +            self.bridges.discard(brname)
>>>> +        return ret
>>>> +
>>>> +    def xmlrpc_is_ovs_bridge(self, bridge):
>>>> +        """
>>>> +        This function verifies whether given interface is an ovs
>>>> bridge.
>>>> +        """
>>>> +        return vswitch.ovs_vsctl_is_ovs_bridge(bridge)
>>>> +
>>>> +    def xmlrpc_add_port_to_bridge(self, bridge, port):
>>>> +        """
>>>> +        Adds a port to the OVS bridge.
>>>> +        """
>>>> +        ret = vswitch.ovs_vsctl_add_port_to_bridge(bridge, port)
>>>> +        if ret == 0:
>>>> +            self.ports.add(port)
>>>> +        return ret
>>>> +
>>>> +    def xmlrpc_del_port_from_bridge(self, port):
>>>> +        """
>>>> +        Removes a port from OVS bridge.
>>>> +        """
>>>> +        ret = vswitch.ovs_vsctl_del_port_from_bridge(port)
>>>> +        if ret == 0:
>>>> +            self.ports.discard(port)
>>>> +        return ret
>>>> +
>>>> +    def xmlrpc_ovs_vsctl_set(self, table, record, column, key, value):
>>>> +        """
>>>> +        This function allows to alter OVS database.
>>>> +        """
>>>> +        return vswitch.ovs_vsctl_set(table, record, column, key, value)
>>>> +
>>>> +    def xmlrpc_interface_up(self, iface):
>>>> +        """
>>>> +        This function brings up given interface.
>>>> +        """
>>>> +        return util.interface_up(iface)
>>>> +
>>>> +    def xmlrpc_interface_assign_ip(self, iface, ip_address, mask):
>>>> +        """
>>>> +        This function allows to assing ip address to the given
>>>> interface.
>>>> +        """
>>>> +        return util.interface_assign_ip(iface, ip_address, mask)
>>>>
>>>>     def xmlrpc_get_interface(self, address):
>>>>         """
>>>> @@ -198,6 +329,17 @@ class TestArena(xmlrpc.XMLRPC):
>>>>
>>>>
>>>>  def start_rpc_server(port):
>>>> -    RPC_SERVER = TestArena()
>>>> -    reactor.listenTCP(port, server.Site(RPC_SERVER))
>>>> -    reactor.run()
>>>> +    """
>>>> +    This function creates a RPC server and adds it to the Twisted
>>>> Reactor.
>>>> +    """
>>>> +    rpc_server = TestArena()
>>>> +    reactor.listenTCP(port, server.Site(rpc_server))
>>>> +    try:
>>>> +        print "Starting RPC server\n"
>>>> +        sys.stdout.flush()
>>>> +         # If this server was started from ovs-test client then we
>>>> must flush
>>>> +         # STDOUT so that client would know that server is ready to
>>>> accept
>>>> +         # XML RPC connections.
>>>> +        reactor.run()
>>>> +    finally:
>>>> +        rpc_server.cleanup()
>>>>
>>> May want to add a cleaning up message, don't know if that is obvious in
>>> context.
>>>
>>>
>>>> diff --git a/python/ovstest/tcp.py b/python/ovstest/tcp.py
>>>> index 33dc719..738d06b 100644
>>>> --- a/python/ovstest/tcp.py
>>>> +++ b/python/ovstest/tcp.py
>>>> @@ -1,4 +1,4 @@
>>>> -# Copyright (c) 2011 Nicira Networks
>>>> +# Copyright (c) 2011, 2012 Nicira Networks
>>>>  #
>>>>  # Licensed under the Apache License, Version 2.0 (the "License");
>>>>  # you may not use this file except in compliance with the License.
>>>> @@ -29,14 +29,10 @@ class TcpListenerConnection(Protocol):
>>>>     def __init__(self):
>>>>         self.stats = 0
>>>>
>>>> -    def connectionMade(self):
>>>> -        print "Started TCP Listener connection"
>>>> -
>>>>     def dataReceived(self, data):
>>>>         self.stats += len(data)
>>>>
>>>>     def connectionLost(self, reason):
>>>> -        print "Stopped TCP Listener connection"
>>>
>>>         self.factory.stats += self.stats
>>>>
>>>>
>>>> @@ -50,12 +46,6 @@ class TcpListenerFactory(Factory):
>>>>     def __init__(self):
>>>>         self.stats = 0
>>>>
>>>> -    def startFactory(self):
>>>> -        print "Starting TCP listener factory"
>>>> -
>>>> -    def stopFactory(self):
>>>> -        print "Stopping TCP listener factory"
>>>> -
>>>>     def getResults(self):
>>>>         """ returns the number of bytes received as string"""
>>>>         #XML RPC does not support 64bit int (
>>>> http://bugs.python.org/issue2985)
>>>> @@ -104,18 +94,13 @@ class TcpSenderConnection(Protocol):
>>>>     """
>>>>
>>>>     def connectionMade(self):
>>>> -        print "Started TCP sender connection"
>>>>         producer = Producer(self, self.factory.duration)
>>>>         self.transport.registerProducer(producer, True)
>>>>         producer.resumeProducing()
>>>>
>>>>     def dataReceived(self, data):
>>>> -        print "Sender received data!", data
>>>>         self.transport.loseConnection()
>>>>
>>>> -    def connectionLost(self, reason):
>>>> -        print "Stopped TCP sender connection"
>>>> -
>>>>
>>>>  class TcpSenderFactory(ClientFactory):
>>>>     """
>>>> @@ -128,12 +113,6 @@ class TcpSenderFactory(ClientFactory):
>>>>         self.duration = duration
>>>>         self.stats = 0
>>>>
>>>> -    def startFactory(self):
>>>> -        print "Starting TCP sender factory"
>>>> -
>>>> -    def stopFactory(self):
>>>> -        print "Stopping TCP sender factory"
>>>> -
>>>>     def getResults(self):
>>>>         """Returns amount of bytes sent to the Listener (as a string)"""
>>>>         return str(self.stats)
>>>> diff --git a/python/ovstest/udp.py b/python/ovstest/udp.py
>>>> index e09569d..a4d85b9 100644
>>>> --- a/python/ovstest/udp.py
>>>> +++ b/python/ovstest/udp.py
>>>> @@ -1,4 +1,4 @@
>>>> -# Copyright (c) 2011 Nicira Networks
>>>> +# Copyright (c) 2011, 2012 Nicira Networks
>>>>  #
>>>>  # Licensed under the Apache License, Version 2.0 (the "License");
>>>>  # you may not use this file except in compliance with the License.
>>>> @@ -28,12 +28,6 @@ class UdpListener(DatagramProtocol):
>>>>     def __init__(self):
>>>>         self.stats = []
>>>>
>>>> -    def startProtocol(self):
>>>> -        print "Starting UDP listener"
>>>> -
>>>> -    def stopProtocol(self):
>>>> -        print "Stopping UDP listener"
>>>> -
>>>>     def datagramReceived(self, data, (_1, _2)):
>>>>         """This function is called each time datagram is received"""
>>>>         try:
>>>> @@ -61,13 +55,11 @@ class UdpSender(DatagramProtocol):
>>>>         self.data = array.array('c', 'X' * size)
>>>>
>>>>     def startProtocol(self):
>>>> -        print "Starting UDP sender"
>>>>         self.looper = LoopingCall(self.sendData)
>>>>         period = self.duration / float(self.count)
>>>>         self.looper.start(period , now = False)
>>>>
>>>>     def stopProtocol(self):
>>>> -        print "Stopping UDP sender"
>>>>         if (self.looper is not None):
>>>>             self.looper.stop()
>>>>             self.looper = None
>>>> diff --git a/python/ovstest/util.py b/python/ovstest/util.py
>>>> index 3321e69..8def507 100644
>>>> --- a/python/ovstest/util.py
>>>> +++ b/python/ovstest/util.py
>>>> @@ -1,4 +1,4 @@
>>>> -# Copyright (c) 2011 Nicira Networks
>>>> +# Copyright (c) 2011, 2012 Nicira Networks
>>>>  #
>>>>  # Licensed under the Apache License, Version 2.0 (the "License");
>>>>  # you may not use this file except in compliance with the License.
>>>> @@ -15,13 +15,21 @@
>>>>  """
>>>>  util module contains some helper function
>>>>  """
>>>> -import socket, struct, fcntl, array, os, subprocess, exceptions
>>>> +import socket, struct, fcntl, array, os, subprocess, exceptions, re
>>>>
>>> Style prefers one import per line
>>>
>>>
>>>>
>>>> -def str_ip(ip):
>>>> -    (x1, x2, x3, x4) = struct.unpack("BBBB", ip)
>>>> +
>>>> +def str_ip(ip_address):
>>>> +    """
>>>> +    Converts an IP address from binary format to a string.
>>>> +    """
>>>> +    (x1, x2, x3, x4) = struct.unpack("BBBB", ip_address)
>>>>     return ("%u.%u.%u.%u") % (x1, x2, x3, x4)
>>>>
>>>> +
>>>>  def get_interface_mtu(iface):
>>>> +    """
>>>> +    Returns MTU of the given interface.
>>>> +    """
>>>>     s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
>>>>     indata = iface + ('\0' * (32 - len(iface)))
>>>>     try:
>>>> @@ -32,6 +40,7 @@ def get_interface_mtu(iface):
>>>>
>>>>     return mtu
>>>>
>>>> +
>>>>  def get_interface(address):
>>>>     """
>>>>     Finds first interface that has given address
>>>> @@ -52,23 +61,70 @@ def get_interface(address):
>>>>             return name
>>>>     return "" #  did not find interface we were looking for
>>>>
>>>> +
>>>>  def uname():
>>>>     os_info = os.uname()
>>>>     return os_info[2] #return only the kernel version number
>>>>
>>>> -def get_driver(iface):
>>>> +
>>>> +def start_process(args):
>>>>     try:
>>>> -        p = subprocess.Popen(
>>>> -            ["ethtool", "-i", iface],
>>>> +        p = subprocess.Popen(args,
>>>>             stdin = subprocess.PIPE,
>>>>             stdout = subprocess.PIPE,
>>>>             stderr = subprocess.PIPE)
>>>>         out, err = p.communicate()
>>>> -        if p.returncode == 0:
>>>> -            lines = out.split("\n")
>>>> -            driver = "%s(%s)" % (lines[0], lines[1]) #driver name +
>>>> version
>>>> -        else:
>>>> -            driver = "no support for ethtool"
>>>> +        return (p.returncode, out, err)
>>>>     except exceptions.OSError:
>>>> -        driver = ""
>>>> +        return (-1, None, None)
>>>> +
>>>> +
>>>> +def get_driver(iface):
>>>> +    ret, out, _err = start_process(["ethtool", "-i", iface])
>>>> +    if ret == 0:
>>>> +        lines = out.split("\n")
>>>>
>>> Could use splitlines, doesn't matter in this context
>>>
>>>
>>>> +        driver = "%s(%s)" % (lines[0], lines[1]) #driver name + version
>>>> +    else:
>>>> +        driver = "<unable to get driver information from ethtool>"
>>>>     return driver
>>>> +
>>>> +
>>>> +def interface_up(iface):
>>>> +    """
>>>> +    This function brings given iface up.
>>>> +    """
>>>> +    ret, _out, _err = start_process(["ifconfig", iface, "up"])
>>>> +    return ret
>>>> +
>>>> +
>>>> +def interface_assign_ip(iface, ip_addr, mask):
>>>> +    """
>>>> +    This function allows to assign IP address to an interface. If mask
>>>> is an
>>>> +    empty string then ifconfig will decide what kind of mask to use.
>>>> The
>>>> +    caller can also specify the mask by using CIDR notation in ip
>>>> argument by
>>>> +    leaving the mask argument as an empty string. In case of success
>>>> this
>>>> +    function returns 0.
>>>> +    """
>>>> +    args = ["ifconfig", iface, ip_addr]
>>>> +    if mask != "":
>>>> +        args.append("netmask")
>>>> +        args.append(mask)
>>>> +    ret, _out, _err = start_process(args)
>>>> +    return ret
>>>> +
>>>> +
>>>> +def interface_get_ip(iface):
>>>> +    """
>>>> +    This function returns tuple - ip and mask that was assigned to the
>>>> +    interface.
>>>> +    """
>>>> +    args = ["ifconfig", iface]
>>>> +    ret, out, _err = start_process(args)
>>>> +
>>>> +    if ret == 0:
>>>> +        ip = re.search(r'inet addr:(\S+)', out)
>>>> +        mask = re.search(r'Mask:(\S+)', out)
>>>> +        if ip is not None and mask is not None:
>>>> +            return (ip.group(1), mask.group(1))
>>>> +    else:
>>>> +        return ret
>>>> diff --git a/python/ovstest/vswitch.py b/python/ovstest/vswitch.py
>>>> new file mode 100644
>>>> index 0000000..5bd15d0
>>>> --- /dev/null
>>>> +++ b/python/ovstest/vswitch.py
>>>> @@ -0,0 +1,97 @@
>>>> +# Copyright (c) 2012 Nicira Networks
>>>> +#
>>>> +# Licensed under the Apache License, Version 2.0 (the "License");
>>>> +# you may not use this file except in compliance with the License.
>>>> +# You may obtain a copy of the License at:
>>>> +#
>>>> +#     http://www.apache.org/licenses/LICENSE-2.0
>>>> +#
>>>> +# Unless required by applicable law or agreed to in writing, software
>>>> +# distributed under the License is distributed on an "AS IS" BASIS,
>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>>>> implied.
>>>> +# See the License for the specific language governing permissions and
>>>> +# limitations under the License.
>>>> +
>>>> +"""
>>>> +vswitch module allows its callers to interact with OVS DB.
>>>> +"""
>>>> +import subprocess
>>>> +import exceptions
>>>> +import util
>>>>
>>> alphabetize, maybe split builtins
>>>
>>>
>>>> +
>>>> +
>>>> +def ovs_vsctl_add_bridge(bridge):
>>>> +    """
>>>> +    This function creates an OVS bridge.
>>>> +    """
>>>> +    ret, _out, _err = util.start_process(["ovs-vsctl", "add-br",
>>>> bridge])
>>>> +    return ret
>>>> +
>>>> +
>>>> +def ovs_vsctl_del_bridge(bridge):
>>>> +    """
>>>> +    This function deletes the OVS bridge.
>>>> +    """
>>>> +    ret, _out, _err = util.start_process(["ovs-vsctl", "del-br",
>>>> bridge])
>>>> +    return ret
>>>> +
>>>> +
>>>> +def ovs_vsctl_is_ovs_bridge(bridge):
>>>> +    """
>>>> +    This function verifies whether given port is an OVS bridge. If it
>>>> is an
>>>> +    OVS bridge then it will return True.
>>>> +    """
>>>> +    ret, _out, _err = util.start_process(["ovs-vsctl", "br-exists",
>>>> bridge])
>>>> +    return ret == 0
>>>> +
>>>> +
>>>> +def ovs_vsctl_add_port_to_bridge(bridge, iface):
>>>> +    """
>>>> +    This function adds given interface to the bridge.
>>>> +    """
>>>> +    ret, _out, _err = util.start_process(["ovs-vsctl", "add-port",
>>>> bridge,
>>>> +                                          iface])
>>>> +    return ret
>>>> +
>>>> +
>>>> +def ovs_vsctl_del_port_from_bridge(port):
>>>> +    """
>>>> +    This function removes given port from a OVS bridge.
>>>> +    """
>>>> +    ret, _out, _err = util.start_process(["ovs-vsctl", "del-port",
>>>> port])
>>>> +    return ret
>>>> +
>>>> +
>>>> +def ovs_vsctl_set(table, record, column, key, value):
>>>> +    """
>>>> +    This function allows to alter OVS database. If column is a map,
>>>> then
>>>>
>>> s/alter OVS/alter the OVS/
>>>
>>>
>>>> +    caller should also set the key, otherwise the key should be left
>>>> as an
>>>> +    empty string.
>>>> +    """
>>>> +    if key == "":
>>>>
>>> A few places here you are using "" as the empty value, you may want to
>>> use
>>> None instead (not sure if these are the result of string parsing
>>> elsewhere).
>>> Regardless, you could support both by just doing "if foo:" in these
>>> places.
>>>
>> Actually I decided that it would be better to use "" instead of None,
>> because this value is passed from RPC client to the server (RPC
>> by default do not handle them).
>>
>> Though, I guess I could try to enable None on Twisted by
>> setting allowNone to True. And by enabling allow_none in the
>> xmlrpclib.
>>
>
> Ah, I forgot this was through RPC.  None works fine if you make those two
> changes, but it isn't a big deal.
>
>
>>
>>>
>>>> +        index = column
>>>> +    else:
>>>> +        index = "%s:%s" % (column, key)
>>>> +    index_value = "%s=%s" % (index, value)
>>>> +    ret, _out, _err = util.start_process(["ovs-vsctl", "set", table,
>>>> record,
>>>> +                                          index_value])
>>>> +    return ret
>>>> +
>>>> +
>>>> +def ovs_get_physical_interface(bridge):
>>>> +    """
>>>> +    This function tries to figure out which is the physical interface
>>>> that
>>>> +    belongs to the bridge. If there are multiple physical interfaces
>>>> assigned
>>>> +    to this bridge then it will return the first match.
>>>> +    """
>>>> +    ret, out, _err = util.start_process(["ovs-vsctl", "list-ifaces",
>>>> bridge])
>>>> +
>>>> +    if ret == 0:
>>>> +        ifaces = out.split("\n")
>>>>
>>> splitlines might be slightly more useful here, since it would drop a
>>> trailing newline
>>>
>>>
>>>> +        for iface in ifaces:
>>>> +            ret, out, _err = util.start_process(["ovs-vsctl", "get",
>>>> +                                                 "Interface", iface,
>>>> "type"])
>>>> +            if ret == 0:
>>>> +                if ('""' in out) or ('system' in out):
>>>> +                    return iface #this should be the physical interface
>>>>
>>> Formatting nit, two spaces before '#', one after:  "<code>  # Comment"
>>>
>>> +    return -1
>>>>
>>> May want to use None as the blank value here
>>>
>>>
>>>> diff --git a/utilities/ovs-test.8.in b/utilities/ovs-test.8.in
>>>> index 87c8944..d5f9f20 100644
>>>> --- a/utilities/ovs-test.8.in
>>>> +++ b/utilities/ovs-test.8.in
>>>> @@ -3,41 +3,47 @@
>>>>  .  ns
>>>>  .  IP "\\$1"
>>>>  ..
>>>> -.TH ovs\-test 1 "October 2011" "Open vSwitch" "Open vSwitch Manual"
>>>> +.TH ovs\-test 1 "March 2012" "Open vSwitch" "Open vSwitch Manual"
>>>>
>>> April?
>>>
>>>
>>>>  .
>>>>  .SH NAME
>>>> -\fBovs\-test\fR \- check Linux drivers for performance and vlan
>>>> problems
>>>> +\fBovs\-test\fR \- check Linux drivers for performance, vlan and L3
>>>> tunneling
>>>> +problems
>>>>  .
>>>>  .SH SYNOPSIS
>>>>  \fBovs\-test\fR \fB\-s\fR \fIport\fR
>>>>  .PP
>>>> -\fBovs\-test\fR \fB\-c\fR \fIserver1\fR
>>>> -\fIserver2\fR [\fB\-b\fR \fIbandwidth\fR]
>>>> +\fBovs\-test\fR \fB\-c\fR \fIserver1\fR \fIserver2\fR
>>>> +[\fB\-b\fR \fItargetbandwidth\fR] [\fB\-i\fR \fItestinterval\fR]
>>>> +[\fB\-d\fR]
>>>> +[\fB\-l\fR \fIvlantag\fR]
>>>> +[\fB\-t\fR \fItunnelmodes\fR]
>>>>  .so lib/common-syn.man
>>>>  .
>>>>  .SH DESCRIPTION
>>>>  The \fBovs\-test\fR program may be used to check for problems sending
>>>> -802.1Q traffic that Open vSwitch may uncover. These problems can
>>>> -occur when Open vSwitch is used to send 802.1Q traffic through physical
>>>> -interfaces running certain drivers of certain Linux kernel versions.
>>>> To run a
>>>> -test, configure Open vSwitch to tag traffic originating from
>>>> \fIserver1\fR and
>>>> -forward it to the \fIserver2\fR. On both servers run \fBovs\-test\fR
>>>> -in server mode. Then, on any other host, run the \fBovs\-test\fR in
>>>> client
>>>> -mode. The client will connect to both \fBovs\-test\fR servers and
>>>> schedule
>>>> -tests between them. \fBovs\-test\fR will perform UDP and TCP tests.
>>>> +802.1Q or GRE traffic that Open vSwitch may uncover. These problems,
>>>> +for example, can occur when Open vSwitch is used to send 802.1Q traffic
>>>> +through physical interfaces running certain drivers of certain Linux
>>>> kernel
>>>> +versions. To run a test, configure IP addresses on \fIserver1\fR and
>>>> +\fIserver2\fR for interfaces you intended to test. These interfaces
>>>> could
>>>> +also be already configured OVS bridges that have a physical interface
>>>> attached
>>>> +to them. Then, on one of the nodes, run \fBovs\-test\fR in server mode
>>>> and on
>>>> +the other node run it in client mode. The client will connect to
>>>> +\fBovs\-test\fR server and schedule tests between both of them. The
>>>> +\fBovs\-test\fR client will perform UDP and TCP tests.
>>>>  .PP
>>>> -UDP tests can report packet loss and achieved bandwidth, because UDP
>>>> flow
>>>> -control is done inside \fBovs\-test\fR. It is also possible to specify
>>>> target
>>>> -bandwidth for UDP. By default it is 1Mbit/s.
>>>> +UDP tests can report packet loss and achieved bandwidth for various
>>>> +datagram sizes. By default target bandwidth for UDP tests is 1Mbit/s.
>>>>  .PP
>>>>  TCP tests report only achieved bandwidth, because kernel TCP stack
>>>>  takes care of flow control and packet loss. TCP tests are essential to
>>>> detect
>>>> -potential TSO related VLAN issues.
>>>> +potential TSO related issues.
>>>>  .PP
>>>> -To determine whether Open vSwitch is encountering any 802.1Q related
>>>> problems,
>>>> +To determine whether Open vSwitch is encountering any problems,
>>>>  the user must compare packet loss and achieved bandwidth in a setup
>>>> where
>>>> -traffic is being tagged against one where it is not. If in the tagged
>>>> setup
>>>> -both servers are unable to communicate or the achieved bandwidth is
>>>> lower,
>>>> +traffic is being directly sent and in one where it is not. If in the
>>>> +802.1Q or L3 tunneled tests both \fBovs\-test\fR processes are unable
>>>> to
>>>> +communicate or the achieved bandwidth is much lower compared to direct
>>>> setup,
>>>>  then, most likely, Open vSwitch has encountered a pre-existing kernel
>>>> or
>>>>  driver bug.
>>>>  .PP
>>>> @@ -46,71 +52,87 @@ Some examples of the types of problems that may be
>>>> encountered are:
>>>>  .
>>>>  .SS "Client Mode"
>>>>  An \fBovs\-test\fR client will connect to two \fBovs\-test\fR servers
>>>> and
>>>> -will ask them to exchange traffic.
>>>> +will ask them to exchange test traffic. It is also possible to spawn an
>>>> +\fBovs\-test\fR server automatically from the client.
>>>>  .
>>>>  .SS "Server Mode"
>>>>  To conduct tests, two \fBovs\-test\fR servers must be running on two
>>>> different
>>>> -hosts where client can connect. The actual test traffic is exchanged
>>>> only
>>>> -between both \fBovs\-test\fR server test IP addresses. It is
>>>> recommended that
>>>> -both servers have their test IP addresses in the same subnet,
>>>> otherwise one
>>>> -will need to change routing so that the test traffic actually goes
>>>> through the
>>>> -interface that he originally intended to test.
>>>> +hosts where the client can connect. The actual test traffic is
>>>> exchanged only
>>>> +between both \fBovs\-test\fR servers. It is recommended that both
>>>> servers have
>>>> +their IP addresses in the same subnet, otherwise one would have to
>>>> make sure
>>>> +that routing is set up correctly.
>>>>  .
>>>>  .SH OPTIONS
>>>>  .
>>>>  .IP "\fB\-s \fIport\fR"
>>>>  .IQ "\fB\-\-server\fR \fIport\fR"
>>>> -Run in server mode and wait for a client to establish XML RPC Control
>>>> -Connection on TCP \fIport\fR. It is recommended to have ethtool
>>>> installed on
>>>> -the server so that it could retrieve information about NIC driver.
>>>> +Run in server mode and wait for the client to establish XML RPC Control
>>>> +Connection on this TCP \fIport\fR. It is recommended to have
>>>> \fBethtool\fR(8)
>>>> +installed on the server so that it could retrieve information about NIC
>>>> +driver.
>>>>
>>> i'd go with either "NIC drivers" or "the NIC driver"
>>>
>>>
>>>>  .
>>>>  .IP "\fB\-c \fIserver1\fR \fIserver2\fR"
>>>>  .IQ "\fB\-\-client \fIserver1\fR \fIserver2\fR"
>>>>  Run in client mode and schedule tests between \fIserver1\fR and
>>>> \fIserver2\fR,
>>>> -where each \fIserver\fR must be given in following format -
>>>> -ControlIP[:ControlPort][,TestIP[:TestPort]]. If TestIP is omitted then
>>>> -ovs-test server will use the ControlIP for testing purposes.
>>>> ControlPort is
>>>> -TCP port where server will listen for incoming XML/RPC control
>>>> -connections to schedule tests (by default it is 15531). TestPort
>>>> -is port which will be used by server to listen for test traffic
>>>> -(by default it is 15532).
>>>> -.
>>>> -.IP "\fB\-b \fIbandwidth\fR"
>>>> -.IQ "\fB\-\-bandwidth\fR \fIbandwidth\fR"
>>>> -Target bandwidth for UDP tests. The \fIbandwidth\fR must be given in
>>>> bits per
>>>> -second. It is possible to use postfix M or K to alter the target
>>>> bandwidth
>>>> -magnitude.
>>>> +where each \fIserver\fR must be given in the following format -
>>>> +\fIOuterIP[:OuterPort],InnerIP[/Mask][:InnerPort]\fR. The
>>>> \fIOuterIP\fR must
>>>> +be already assigned to the physical interface which is going to be
>>>> tested.
>>>> +This is the IP address where client will try to establish XML RPC
>>>> connection.
>>>> +If \fIOuterIP\fR is 127.0.0.1 then client will automatically spawn a
>>>> local
>>>> +instance of \fBovs\-test\fR server. \fIOuterPort\fR is TCP port where
>>>> server
>>>> +is listening for incoming XML/RPC control connections to schedule
>>>> tests (by
>>>> +default it is 15531). The \fBovs\-test\fR will automatically assign
>>>> +\fIInnerIP[/Mask]\fR to the interfaces that will be created on the fly
>>>> for
>>>> +testing purposes. It is important that \fIInnerIP[/Mask]\fR does not
>>>> interfere
>>>> +with already existing IP addresses on both \fBovs\-test\fR servers and
>>>> client.
>>>> +\fIInnerPort\fR is port which will be used by server to listen for test
>>>> +traffic that will be encapsulated (by default it is 15532).
>>>> +.
>>>> +.IP "\fB\-b \fItargetbandwidth\fR"
>>>> +.IQ "\fB\-\-bandwidth\fR \fItargetbandwidth\fR"
>>>> +Target bandwidth for UDP tests. The \fItargetbandwidth\fR must be
>>>> given in
>>>> +bits per second. It is possible to use postfix M or K to alter the
>>>> target
>>>> +bandwidth magnitude.
>>>> +.
>>>> +.IP "\fB\-i \fItestinterval\fR"
>>>> +.IQ "\fB\-\-interval\fR \fItestinterval\fR"
>>>> +For how long each test should run. By default 5 seconds.
>>>>
>>> I think s/For how long/How long/
>>>
>>>
>>>>  .
>>>>  .so lib/common.man
>>>> -.SH EXAMPLES
>>>> -.PP
>>>> -Set up a bridge which forwards traffic originating from \fB1.2.3.4\fR
>>>> out
>>>> -\fBeth1\fR with VLAN tag 10.
>>>> -.IP
>>>> -.B ovs\-vsctl \-\- add\-br vlan\-br \(rs
>>>> -.IP
>>>> -.B \-\- add\-port vlan\-br eth1 \(rs
>>>> -.IP
>>>> -.B \-\- add\-port vlan\-br vlan\-br\-tag tag=10 \(rs
>>>> -.IP
>>>> -.B \-\- set Interface vlan\-br\-tag type=internal
>>>> -.IP
>>>> -.B ifconfig vlan\-br\-tag up 1.2.3.4
>>>>  .
>>>> +.SH "Test Modes"
>>>> +The following test modes are supported by \fBovs\-test\fR. It is
>>>> possible
>>>> +to combine multiple of them in a single \fBovs\-test\fR invocation.
>>>> +.
>>>> +.IP "\fB\-d \fR"
>>>> +.IQ "\fB\-\-direct\fR"
>>>> +Perform direct tests between both \fIOuterIP\fR addresses. These tests
>>>> could
>>>> +be used as a reference to compare 802.1Q or L3 tunneling test results.
>>>> +.
>>>> +.IP "\fB\-l \fIvlantag\fR"
>>>> +.IQ "\fB\-\-vlan\-tag\fR \fIvlantag\fR"
>>>> +Perform 802.1Q tests between both servers. These tests will create a
>>>> temporary
>>>> +OVS bridge, if necessary, and attach a VLAN tagged port to it for
>>>> testing
>>>> +purposes.
>>>> +.
>>>> +.IP "\fB\-t \fItunnelmodes\fR"
>>>> +.IQ "\fB\-\-tunnel\-modes\fR \fItunnelmodes\fR"
>>>> +Perform L3 tunneling tests. The given argument is a comma separated
>>>> string
>>>> +that specifies all the L3 tunnel modes that should be tested (e.g.
>>>> gre). The L3
>>>> +tunnels are terminated on interface that has the \fIOuterIP\fR address
>>>> +assigned.
>>>> +.
>>>> +.SH EXAMPLES
>>>>  .PP
>>>> -On two different hosts start \fBovs\-test\fR in server mode and tell
>>>> them to
>>>> -listen on port 15531 for incoming client control connections:
>>>> -.IP
>>>> -.B 1.2.3.4: ovs\-test \-s 15531
>>>> +On host 1.2.3.4 start \fBovs\-test\fR in server mode:
>>>>  .IP
>>>> -.B 1.2.3.5: ovs\-test \-s 15531
>>>> +.B ovs\-test \-s 15531
>>>>
>>> Is 15531 implied/default now?  Omitting it here would mean less
>>> documentation
>>> changes to make if you ever modify the default =)
>>>
>>>
>>>>  .
>>>>  .PP
>>>> -On any other host start \fBovs\-test\fR in client mode and ask it to
>>>> connect
>>>> -to those two servers - one at 1.2.3.4 and another at 1.2.3.5 (by
>>>> default
>>>> -client will use TCP port 15531 to establish control channel).
>>>> +On host 1.2.3.5 start \fBovs\-test\fR in client mode and do direct,
>>>> VLAN and
>>>> +GRE tests between both nodes:
>>>>  .IP
>>>> -.B ovs\-test -c 1.2.3.4 1.2.3.5
>>>> +.B ovs\-test \-c 127.0.0.1,1.1.1.1/30 1.2.3.4,1.1.1.2/30 -d -l 123 -t
>>>> gre
>>>>  .
>>>>  .SH SEE ALSO
>>>>  .
>>>> @@ -119,4 +141,4 @@ client will use TCP port 15531 to establish control
>>>> channel).
>>>>  .BR ovs\-vsctl (8),
>>>>  .BR ovs\-vlan\-test (8),
>>>>  .BR ethtool (8),
>>>> -.BR uname (1)
>>>> +.BR uname (1)
>>>> \ No newline at end of file
>>>> diff --git a/utilities/ovs-test.in b/utilities/ovs-test.in
>>>> index 6518dbc..f6a5350 100644
>>>> --- a/utilities/ovs-test.in
>>>> +++ b/utilities/ovs-test.in
>>>> @@ -1,4 +1,4 @@
>>>> -#! @PYTHON@
>>>> +#!/usr/bin/python
>>>>  #
>>>>  # Licensed under the Apache License, Version 2.0 (the "License");
>>>>  # you may not use this file except in compliance with the License.
>>>> @@ -21,11 +21,74 @@ import xmlrpclib
>>>>  import time
>>>>  import socket
>>>>  import math
>>>> +import os
>>>> +import subprocess
>>>> +import fcntl
>>>> +import select
>>>> +import signal
>>>> +import argparse
>>>> +import sys
>>>>
>>> sort
>>>
>>>
>>> +
>>>>  from ovstest import args, rpcserver
>>>>
>>> Google style prefers
>>> import ovstest.args as args
>>> import ovstest.rpcserver as rpcserver
>>>
>>>
>>>
>>>>
>>>> +DEFAULT_TEST_BRIDGE="ovstestbr0"
>>>> +DEFAULT_TEST_PORT="ovstestport0"
>>>> +DEFAULT_TEST_TUN="ovstesttun0"
>>>> +
>>>> +
>>>> +def sigint_intercept():
>>>> +    """
>>>> +    Intercept SIGINT from child (the local ovs-test server process).
>>>> +    """
>>>> +    signal.signal(signal.SIGINT, signal.SIG_IGN)
>>>> +
>>>> +
>>>> +def start_local_server(port):
>>>> +    """
>>>> +    This function spawns an ovs-test server that listens on specified
>>>> port
>>>> +    and blocks till the spawned ovs-test server is ready to accept XML
>>>> RPC
>>>> +    connections.
>>>> +    """
>>>> +    p = subprocess.Popen(["/usr/bin/ovs-test", "-s", str(port)],
>>>> +                         stdout=subprocess.PIPE,
>>>> stderr=subprocess.PIPE,
>>>> +                         preexec_fn = sigint_intercept)
>>>>
>>> Neat, didn't ever notice preexec_fn before.  Not sure why it is ignored,
>>> but
>>> I'm sure it's for a good cause
>>>
>>>
>>>> +    fcntl.fcntl( p.stdout.fileno(),fcntl.F_SETFL,
>>>> +        fcntl.fcntl(p.stdout.fileno(), fcntl.F_GETFL) | os.O_NONBLOCK)
>>>> +
>>>> +    out = ""
>>>> +    while p.poll() == None:
>>>>
>>> Style nit:  s/ == None/ is None/
>>>
>>>
>>> +        fd = select.select([p.stdout.fileno()], [], [])[0]
>>>> +        if fd:
>>>> +            out += p.stdout.readline()
>>>> +            if out.startswith("Starting RPC server"):
>>>> +                break
>>>> +    if p.poll() != None:
>>>>
>>> s/ != None/ is not None/
>>>
>>>
>>> +        raise RuntimeError("Couldn't start local instance of ovs-test
>>>> server")
>>>> +    return p
>>>> +
>>>> +
>>>> +def get_datagram_sizes(mtu1, mtu2):
>>>> +    """
>>>> +    This function calculates all the "interesting" datagram sizes so
>>>> that
>>>> +    we test both - receive and send side with different packets sizes.
>>>> +    """
>>>> +    s1 = set([8, mtu1 - 100, mtu1 - 28, mtu1])
>>>> +    s2 = set([8, mtu2 - 100, mtu2 - 28, mtu2])
>>>> +    l = sorted(list(s1.union(s2)))
>>>> +    return l
>>>>
>>> I'd replace the last two lines with
>>>
>>>   return sorted(s1.union(s2))
>>>
>>> partially to avoid using a variable named 'l' (or is it I or | or ! or
>>> 1)?
>>>
>>>
>>>> +
>>>> +
>>>> +def ip_from_cidr(string):
>>>> +    """
>>>> +    This function removes the netmask (if present) from the given
>>>> string and
>>>> +    returns the IP address.
>>>> +    """
>>>> +    token = string.split("/")
>>>> +    return token[0]
>>>> +
>>>>
>>>>  def bandwidth_to_string(bwidth):
>>>> -    """Convert bandwidth from long to string and add units"""
>>>> +    """Convert bandwidth from long to string and add units."""
>>>>     bwidth = bwidth * 8 #  Convert back to bits/second
>>>>     if bwidth >= 10000000:
>>>>         return str(int(bwidth / 1000000)) + "Mbps"
>>>> @@ -38,23 +101,27 @@ def bandwidth_to_string(bwidth):
>>>>  def collect_information(node):
>>>
>>>     """Print information about hosts that will do testing"""
>>>>     print "Node %s:%u " % (node[0], node[1])
>>>> -    server1 = xmlrpclib.Server("http://%s:%u/" % (node[0], node[1]))
>>>> -    interface_name = server1.get_interface(node[2])
>>>> -    uname = server1.uname()
>>>> +    server = xmlrpclib.Server("http://%s:%u/" % (node[0], node[1]))
>>>> +    interface_name = server.get_interface(node[0])
>>>>
>>> Was this a bug before, or has the behavior changed ([2] -> [0])?
>>> Based upon the use throughout below, I'm guessing this is just
>>> the fallout of the new argument specification format.
>>>
>>>
>>> +    uname = server.uname()
>>>>     mtu = 1500
>>>>
>>>>     if interface_name == "":
>>>>         print ("Could not find interface that has %s IP address."
>>>> -               "Make sure that you specified correct Test IP." %
>>>> (node[2]))
>>>> +               "Make sure that you specified correct Outer IP." %
>>>> (node[0]))
>>>>     else:
>>>> -        mtu = server1.get_interface_mtu(interface_name)
>>>> -        driver = server1.get_driver(interface_name)
>>>> -        print "Will be using %s(%s) with MTU %u" % (interface_name,
>>>> node[2],
>>>> +        if server.is_ovs_bridge(interface_name):
>>>> +            phys_iface = server.get_iface_from_bridge(interface_name)
>>>> +        else:
>>>> +            phys_iface = interface_name
>>>> +
>>>> +        driver = server.get_driver(phys_iface)
>>>> +        print "Will be using %s (%s) with MTU %u" % (phys_iface,
>>>> node[0],
>>>>                                                     mtu)
>>>>         if driver == "":
>>>>             print "Install ethtool on this host to get NIC driver
>>>> information"
>>>>         else:
>>>> -            print "On this host %s has %s." % (interface_name, driver)
>>>> +            print "On this host %s has %s." % (phys_iface, driver)
>>>>
>>>>     if uname == "":
>>>>         print "Unable to retrieve kernel information. Is this Linux?"
>>>> @@ -64,7 +131,7 @@ def collect_information(node):
>>>>     return mtu
>>>>
>>>>
>>>> -def do_udp_tests(receiver, sender, tbwidth, duration, sender_mtu):
>>>> +def do_udp_tests(receiver, sender, tbwidth, duration, ps):
>>>>
>>> Could get some free self-documenting action by calling this port_sizes
>>>
>>>
>>>
>>>>     """Schedule UDP tests between receiver and sender"""
>>>>     server1 = xmlrpclib.Server("http://%s:%u/" % (receiver[0],
>>>> receiver[1]))
>>>>     server2 = xmlrpclib.Server("http://%s:%u/" % (sender[0],
>>>> sender[1]))
>>>> @@ -77,7 +144,7 @@ def do_udp_tests(receiver, sender, tbwidth,
>>>> duration, sender_mtu):
>>>>     print udpformat.format("Datagram Size", "Snt Datagrams", "Rcv
>>>> Datagrams",
>>>>                             "Datagram Loss", "Bandwidth")
>>>>
>>>> -    for size in [8, sender_mtu - 100, sender_mtu - 28, sender_mtu]:
>>>> +    for size in ps:
>>>>         listen_handle = -1
>>>>         send_handle = -1
>>>>         try:
>>>> @@ -89,8 +156,9 @@ def do_udp_tests(receiver, sender, tbwidth,
>>>> duration, sender_mtu):
>>>>                         " %u. Try to restart the server.\n" %
>>>> receiver[3])
>>>>                 return
>>>>             send_handle = server2.create_udp_sender(
>>>> -                                            (receiver[2], receiver[3]),
>>>> -                                            packetcnt, size, duration)
>>>> +                                            (ip_from_cidr(receiver[2]),
>>>> +                                             receiver[3]), packetcnt,
>>>> size,
>>>> +                                             duration)
>>>>
>>>>             #Using sleep here because there is no other synchronization
>>>> source
>>>>             #that would notify us when all sent packets were received
>>>> @@ -131,8 +199,8 @@ def do_tcp_tests(receiver, sender, duration):
>>>>             print ("Server was unable to open TCP listening socket on
>>>> port"
>>>>                     " %u. Try to restart the server.\n" % receiver[3])
>>>>             return
>>>> -        send_handle = server2.create_tcp_sender(receiver[2],
>>>> receiver[3],
>>>> -                                                    duration)
>>>> +        send_handle =
>>>> server2.create_tcp_sender(ip_from_cidr(receiver[2]),
>>>> +                                                receiver[3], duration)
>>>>
>>>>         time.sleep(duration + 1)
>>>>
>>>> @@ -151,30 +219,168 @@ def do_tcp_tests(receiver, sender, duration):
>>>>     print "\n"
>>>>
>>>>
>>>> +def do_l3_tests(node1, node2, bandwidth, duration, ps, type):
>>>> +    """
>>>> +    Do L3 tunneling tests.
>>>> +    """
>>>> +    server1 = xmlrpclib.Server("http://%s:%u/" % (node1[0], node1[1]))
>>>> +    server2 = xmlrpclib.Server("http://%s:%u/" % (node2[0], node2[1]))
>>>> +    try:
>>>> +        server1.create_bridge(DEFAULT_TEST_BRIDGE)
>>>> +        server2.create_bridge(DEFAULT_TEST_BRIDGE)
>>>> +
>>>> +        server1.interface_up(DEFAULT_TEST_BRIDGE)
>>>> +        server2.interface_up(DEFAULT_TEST_BRIDGE)
>>>> +
>>>> +        server1.interface_assign_ip(DEFAULT_TEST_BRIDGE, node1[2], "")
>>>> +        server2.interface_assign_ip(DEFAULT_TEST_BRIDGE, node2[2], "")
>>>> +
>>>> +        server1.add_port_to_bridge(DEFAULT_TEST_BRIDGE,
>>>> DEFAULT_TEST_TUN)
>>>> +        server2.add_port_to_bridge(DEFAULT_TEST_BRIDGE,
>>>> DEFAULT_TEST_TUN)
>>>> +
>>>> +        server1.ovs_vsctl_set("Interface", DEFAULT_TEST_TUN, "type",
>>>> "", type)
>>>> +        server2.ovs_vsctl_set("Interface", DEFAULT_TEST_TUN, "type",
>>>> "", type)
>>>> +        server1.ovs_vsctl_set("Interface", DEFAULT_TEST_TUN, "options",
>>>> +                              "remote_ip", node2[0])
>>>> +        server2.ovs_vsctl_set("Interface", DEFAULT_TEST_TUN, "options",
>>>> +                              "remote_ip", node1[0])
>>>> +
>>>> +        do_udp_tests(node1, node2, bandwidth, duration, ps)
>>>> +        do_udp_tests(node2, node1, bandwidth, duration, ps)
>>>> +        do_tcp_tests(node1, node2, duration)
>>>> +        do_tcp_tests(node2, node1, duration)
>>>> +
>>>> +    finally:
>>>> +        server1.del_bridge(DEFAULT_TEST_BRIDGE)
>>>> +        server2.del_bridge(DEFAULT_TEST_BRIDGE)
>>>>
>>> Slight nit, you will lose your original error if create bridge fails I
>>> think.  Not sure
>>> what the expectations are, or whether create_bridge can fail in that
>>> way.  I tend
>>> to structure these things like
>>>
>>>   do_stateful_operation()  # Atomic
>>>   try:
>>>       other_things()  # Might fail
>>>   finally:
>>>       undo_stateful_operation()  # Can't fail
>>>
>>
>> Maybe I did not understand your approach as intended, but
>> did you mean something like this:
>>
>>     # Make 1&2 together to look like a single atomic operation.
>>>
>>>     try:
>>>       do_atomic_operation1()
>>>       do_atomic_operation2()
>>>     except:
>>>       undo_atomic_operation1()
>>>       undo_atomic_operation2()
>>>       raise Something  # If something went wrong - better quit early by
>>> raising Something
>>>
>>>     try:
>>>       do_something1()
>>>       do_something2()
>>>     finally:
>>>       undo_atomic_operation1()
>>>       undo_atomic_operation2()
>>>
>>
>>
>> instead of this:
>>
>>     try:
>>>       do_atomic_operation1()
>>>       do_atomic_operation2()
>>>       do_something1()
>>>       do_something2()
>>>     finally:
>>>       undo_atomic_operation1() # This would get cleaned if
>>> do_atomic_operation2() failed
>>>       undo_atomic_operation2()
>>>
>>
>> ?
>>
>
> More or less, probably with helper functions.  I think what I suggest
> below is
> probably too much trouble either way, but I'll make a note anyway.
>
> In this case I am only worried about a bridge being deleted that was not
> created.
> If server.create_bridge is not transactional, you will have funky
> problems, but I
> think it is fine.  So you can just do
>
>    servers_with_bridges = []
>    try:
>      server1.create_bridge(DEFAULT_TEST_BRIDGE)
>      servers_with_bridges.append(server1)
>      create_bridge(server2, DEFAULT_TEST_BRIDGE)
>      servers_with_bridges.append(server2)
>      do_other_stuff()
>    finally:
>      for server in servers_with_bridges:
>       server.del_bridge(DEFAULT_TEST_BRIDGE)
>
Thanks. I somehow prefer this approach over the first one.
Would you mind reviewing the v3 patch?

>
> or just add replace the del_bridge with some try/except
>
>    def try_del_bridge(server, bridge)
>         try:
>             server.del_bridge(bridge)
>         except Exception, e:
>             print "Some warning about %s" % e
>
> and don't bother with the set
>
>
>>
>>
>>
>>>
>>> If I can't avoid potential failure in the undo portion with
>>> conditionals, or some
>>> other form of graceful cleanup. I would add a try/except inside the
>>> finally clause,
>>> or add something to note any original exception in passing.
>>>
>>>   try:
>>>     <do stuff>
>>>   except Exception:
>>>      <note original exception>
>>>      raise  # Don't forget this part!
>>>   finally:
>>>      server1.del_bridge(DEFAULT_TEST_BRIDGE)
>>>
>>>
>>>> +
>>>> +
>>>> +def do_vlan_tests(node1, node2, bandwidth, duration, ps, tag):
>>>> +    """
>>>> +    Do VLAN tests between node1 and node2.
>>>> +    """
>>>> +    server1 = xmlrpclib.Server("http://%s:%u/" % (node1[0], node1[1]))
>>>> +    server2 = xmlrpclib.Server("http://%s:%u/" % (node2[0], node2[1]))
>>>> +
>>>> +    br_name1 = None
>>>> +    br_name2 = None
>>>> +
>>>> +    try:
>>>> +        interface_node1 = server1.get_interface(node1[0])
>>>> +        interface_node2 = server2.get_interface(node2[0])
>>>> +
>>>> +        if server1.is_ovs_bridge(interface_node1):
>>>> +            br_name1 = interface_node1
>>>> +        else:
>>>> +            br_name1 = DEFAULT_TEST_BRIDGE
>>>> +            server1.create_test_bridge(br_name1, interface_node1)
>>>> +
>>>> +        if server2.is_ovs_bridge(interface_node2):
>>>> +            br_name2 = interface_node2
>>>> +        else:
>>>> +            br_name2 = DEFAULT_TEST_BRIDGE
>>>> +            server2.create_test_bridge(br_name2, interface_node2)
>>>> +
>>>> +        server1.add_port_to_bridge(br_name1, DEFAULT_TEST_PORT)
>>>> +        server2.add_port_to_bridge(br_name2, DEFAULT_TEST_PORT)
>>>> +
>>>> +        server1.ovs_vsctl_set("Port", DEFAULT_TEST_PORT, "tag", "",
>>>> tag)
>>>> +        server2.ovs_vsctl_set("Port", DEFAULT_TEST_PORT, "tag", "",
>>>> tag)
>>>> +
>>>> +        server1.ovs_vsctl_set("Interface", DEFAULT_TEST_PORT, "type",
>>>> "",
>>>> +                              "internal")
>>>> +        server2.ovs_vsctl_set("Interface", DEFAULT_TEST_PORT, "type",
>>>> "",
>>>> +                              "internal")
>>>> +
>>>> +        server1.interface_assign_ip(DEFAULT_TEST_PORT, node1[2], "")
>>>> +        server2.interface_assign_ip(DEFAULT_TEST_PORT, node2[2], "")
>>>> +
>>>> +        server1.interface_up(DEFAULT_TEST_PORT)
>>>> +        server2.interface_up(DEFAULT_TEST_PORT)
>>>> +
>>>> +        do_udp_tests(node1, node2, bandwidth, duration, ps)
>>>> +        do_udp_tests(node2, node1, bandwidth, duration, ps)
>>>> +        do_tcp_tests(node1, node2, duration)
>>>> +        do_tcp_tests(node2, node1, duration)
>>>> +
>>>> +    finally:
>>>> +        server1.del_port_from_bridge(DEFAULT_TEST_PORT)
>>>> +        server2.del_port_from_bridge(DEFAULT_TEST_PORT)
>>>> +        if br_name1 == DEFAULT_TEST_BRIDGE:
>>>> +            server1.del_test_bridge(br_name1, interface_node1)
>>>> +        if br_name2 == DEFAULT_TEST_BRIDGE:
>>>> +            server2.del_test_bridge(br_name2, interface_node2)
>>>> +
>>>> +
>>>> +def do_direct_tests(node1, node2, bandwidth, duration, ps):
>>>> +    """
>>>> +    Do tests between outer IPs without involving Open vSwitch
>>>> +    """
>>>> +    n1 = (node1[0], node1[1], node1[0], node1[3])
>>>> +    n2 = (node2[0], node2[1], node2[0], node2[3])
>>>> +
>>>> +    do_udp_tests(n1, n2, bandwidth, duration, ps)
>>>> +    do_udp_tests(n2, n1, bandwidth, duration, ps)
>>>> +    do_tcp_tests(n1, n2, duration)
>>>> +    do_tcp_tests(n2, n1, duration)
>>>> +
>>>> +
>>>>  if __name__ == '__main__':
>>>> +    local_server = None
>>>>     try:
>>>>         ovs_args = args.ovs_initialize_args()
>>>>
>>>> -        if ovs_args.port is not None: #  Start in server mode
>>>> -            print "Starting RPC server"
>>>> -            try:
>>>> -                rpcserver.start_rpc_server(ovs_args.port)
>>>> -            except twisted.internet.error.CannotListenError:
>>>> -                print "Couldn't start XMLRPC server on port %u" %
>>>> ovs_args.port
>>>> +        if ovs_args.port is not None: #  Start in pure server mode
>>>>
>>> Not part of your change, just noticed the 2-before 1-after for #.
>>>
>>>
>>>>  +            rpcserver.start_rpc_server(ovs_args.port)
>>>>
>>>>         elif ovs_args.servers is not None: #  Run in client mode
>>>>             node1 = ovs_args.servers[0]
>>>>             node2 = ovs_args.servers[1]
>>>> -            bandwidth = ovs_args.targetBandwidth
>>>>
>>>> -            mtu_node1 = collect_information(node1)
>>>> +            # Verify whether client will need to spawn a local
>>>> instance of
>>>> +            # ovs-test server by looking at the first OuterIP. if it
>>>> is a
>>>> +            # 127.0.0.1 then spawn local ovs-test server.
>>>> +            if node1[0] == "127.0.0.1":
>>>> +                local_server = start_local_server(node1[1])
>>>> +                #We must determine the IP address that local ovs-test
>>>> server
>>>> +                #will use:
>>>> +                me = xmlrpclib.Server("http://%s:%u/" % (node1[0],
>>>> node1[1]))
>>>> +                my_ip = me.get_my_address_from(node2[0], node2[1])
>>>> +                node1 = (my_ip, node1[1], node1[2], node1[3])
>>>> +
>>>>             mtu_node2 = collect_information(node2)
>>>> +            mtu_node1 = collect_information(node1)
>>>> +
>>>> +            bandwidth = ovs_args.targetBandwidth
>>>> +            interval = ovs_args.testInterval
>>>> +            ps = get_datagram_sizes(mtu_node1, mtu_node2)
>>>> +
>>>> +            direct = ovs_args.direct
>>>> +            vlan_tag = ovs_args.vlanTag
>>>> +            tunnel_modes = ovs_args.tunnelModes
>>>> +
>>>> +            if direct is not None:
>>>> +                print "Performing direct tests"
>>>> +                do_direct_tests(node2, node1, bandwidth, interval, ps)
>>>> +
>>>> +            if vlan_tag is not None:
>>>> +                print "Performing VLAN tests"
>>>> +                do_vlan_tests(node2, node1, bandwidth, interval, ps,
>>>> vlan_tag)
>>>> +
>>>> +            for tmode in tunnel_modes:
>>>> +                print "Performing", tmode, "tests"
>>>> +                do_l3_tests(node2, node1, bandwidth, interval, ps,
>>>> tmode)
>>>>
>>>> -            do_udp_tests(node1, node2, bandwidth, 5, mtu_node1)
>>>> -            do_udp_tests(node2, node1, bandwidth, 5, mtu_node2)
>>>> -            do_tcp_tests(node1, node2, 5)
>>>> -            do_tcp_tests(node2, node1, 5)
>>>>     except KeyboardInterrupt:
>>>>         pass
>>>> +    except xmlrpclib.Fault:
>>>> +        print "Couldn't establish XMLRPC control channel"
>>>>     except socket.error:
>>>>         print "Couldn't establish XMLRPC control channel"
>>>> +    except xmlrpclib.ProtocolError:
>>>> +        print "XMLRPC control channel was abruptly terminated"
>>>> +    except twisted.internet.error.CannotListenError:
>>>> +        print "Couldn't start XMLRPC server on port %u" % ovs_args.port
>>>> +    finally:
>>>> +        if local_server is not None:
>>>> +            local_server.terminate()
>>>> --
>>>> 1.7.9.1
>>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev at openvswitch.org
>>>> http://openvswitch.org/mailman/listinfo/dev
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://openvswitch.org/pipermail/dev/attachments/20120416/47c20be5/attachment.htm>


More information about the dev mailing list