[ovs-dev] [PATCHv2] ovs-test: Enhancements to the ovs-test tool
Reid Price
reid at nicira.com
Sat Apr 14 03:00:43 UTC 2012
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)
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/20120413/53fd0ace/attachment-0003.html>
More information about the dev
mailing list