[ovs-dev] [PATCHv2] ovs-test: Enhancements to the ovs-test tool
Ansis Atteka
aatteka at nicira.com
Mon Apr 16 20:16:27 UTC 2012
On Mon, Apr 16, 2012 at 1:00 PM, Reid Price <reid at nicira.com> wrote:
> On Mon, Apr 16, 2012 at 12:32 PM, Ansis Atteka <aatteka at nicira.com> wrote:
>
>>
>>
>> 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?
>>
>
> Has it been sent? I don't think I see it
>
Yes, I sent it out 45 minutes ago. The openvswitch.org does not seem to
work, so I cant copy+paste the link to the mailing list archive.
Anyway, v3 patch went under the subject "[PATCHv3] ovs-test: Enhancements
to the ovs-test tool" and I see it in my Inbox/openvswitch folder. Let me
know if you still do not see it.
>
>
>>
>>> 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://mail.openvswitch.org/pipermail/ovs-dev/attachments/20120416/95150994/attachment-0003.html>
More information about the dev
mailing list