Skip to content

Commit

Permalink
Merge "Add config option to specify ovs datapath."
Browse files Browse the repository at this point in the history
  • Loading branch information
Jenkins authored and openstack-gerrit committed Aug 22, 2015
2 parents 9274277 + 63b0336 commit 5b708d5
Show file tree
Hide file tree
Showing 13 changed files with 141 additions and 26 deletions.
5 changes: 5 additions & 0 deletions etc/neutron/plugins/ml2/openvswitch_agent.ini
Expand Up @@ -57,6 +57,11 @@
# 'ovs-ofctl' is currently the only available choice.
# of_interface = ovs-ofctl

# (StrOpt) ovs datapath to use.
# 'system' is the default value and corresponds to the kernel datapath.
# To enable the userspace datapath set this value to 'netdev'
# datapath_type = system

[agent]
# Log agent heartbeats from this OVS agent
# log_agent_heartbeats = False
Expand Down
19 changes: 14 additions & 5 deletions neutron/agent/common/ovs_lib.py
Expand Up @@ -30,6 +30,8 @@
from neutron.common import exceptions
from neutron.i18n import _LE, _LI, _LW
from neutron.plugins.common import constants as p_const
from neutron.plugins.ml2.drivers.openvswitch.agent.common \
import constants

# Default timeout for ovs-vsctl command
DEFAULT_OVS_VSCTL_TIMEOUT = 10
Expand Down Expand Up @@ -102,8 +104,11 @@ def __init__(self):
self.vsctl_timeout = cfg.CONF.ovs_vsctl_timeout
self.ovsdb = ovsdb.API.get(self)

def add_bridge(self, bridge_name):
self.ovsdb.add_br(bridge_name).execute()
def add_bridge(self, bridge_name,
datapath_type=constants.OVS_DATAPATH_SYSTEM):

self.ovsdb.add_br(bridge_name,
datapath_type).execute()
br = OVSBridge(bridge_name)
# Don't return until vswitchd sets up the internal port
br.get_port_ofport(bridge_name)
Expand Down Expand Up @@ -143,9 +148,10 @@ def db_get_val(self, table, record, column, check_error=False,


class OVSBridge(BaseOVS):
def __init__(self, br_name):
def __init__(self, br_name, datapath_type=constants.OVS_DATAPATH_SYSTEM):
super(OVSBridge, self).__init__()
self.br_name = br_name
self.datapath_type = datapath_type

def set_controller(self, controllers):
self.ovsdb.set_controller(self.br_name,
Expand Down Expand Up @@ -173,7 +179,9 @@ def set_protocols(self, protocols):

def create(self, secure_mode=False):
with self.ovsdb.transaction() as txn:
txn.add(self.ovsdb.add_br(self.br_name))
txn.add(
self.ovsdb.add_br(self.br_name,
datapath_type=self.datapath_type))
if secure_mode:
txn.add(self.ovsdb.set_fail_mode(self.br_name,
FAILMODE_SECURE))
Expand All @@ -186,7 +194,8 @@ def destroy(self):
def reset_bridge(self, secure_mode=False):
with self.ovsdb.transaction() as txn:
txn.add(self.ovsdb.del_br(self.br_name))
txn.add(self.ovsdb.add_br(self.br_name))
txn.add(self.ovsdb.add_br(self.br_name,
datapath_type=self.datapath_type))
if secure_mode:
txn.add(self.ovsdb.set_fail_mode(self.br_name,
FAILMODE_SECURE))
Expand Down
14 changes: 8 additions & 6 deletions neutron/agent/ovsdb/api.py
Expand Up @@ -95,14 +95,16 @@ def transaction(self, check_error=False, log_errors=True, **kwargs):
"""

@abc.abstractmethod
def add_br(self, name, may_exist=True):
def add_br(self, name, may_exist=True, datapath_type=None):
"""Create an command to add an OVS bridge
:param name: The name of the bridge
:type name: string
:param may_exist: Do not fail if bridge already exists
:type may_exist: bool
:returns: :class:`Command` with no result
:param name: The name of the bridge
:type name: string
:param may_exist: Do not fail if bridge already exists
:type may_exist: bool
:param datapath_type: The datapath_type of the bridge
:type datapath_type: string
:returns: :class:`Command` with no result
"""

@abc.abstractmethod
Expand Down
4 changes: 2 additions & 2 deletions neutron/agent/ovsdb/impl_idl.py
Expand Up @@ -144,8 +144,8 @@ def transaction(self, check_error=False, log_errors=True, **kwargs):
self.context.vsctl_timeout,
check_error, log_errors)

def add_br(self, name, may_exist=True):
return cmd.AddBridgeCommand(self, name, may_exist)
def add_br(self, name, may_exist=True, datapath_type=None):
return cmd.AddBridgeCommand(self, name, may_exist, datapath_type)

def del_br(self, name, if_exists=True):
return cmd.DelBridgeCommand(self, name, if_exists)
Expand Down
8 changes: 6 additions & 2 deletions neutron/agent/ovsdb/impl_vsctl.py
Expand Up @@ -160,9 +160,13 @@ class OvsdbVsctl(ovsdb.API):
def transaction(self, check_error=False, log_errors=True, **kwargs):
return Transaction(self.context, check_error, log_errors, **kwargs)

def add_br(self, name, may_exist=True):
def add_br(self, name, may_exist=True, datapath_type=None):
opts = ['--may-exist'] if may_exist else None
return BaseCommand(self.context, 'add-br', opts, [name])
params = [name]
if datapath_type:
params += ['--', 'set', 'Bridge', name,
'datapath_type=%s' % datapath_type]
return BaseCommand(self.context, 'add-br', opts, params)

def del_br(self, name, if_exists=True):
opts = ['--if-exists'] if if_exists else None
Expand Down
5 changes: 4 additions & 1 deletion neutron/agent/ovsdb/native/commands.py
Expand Up @@ -50,10 +50,11 @@ def __str__(self):


class AddBridgeCommand(BaseCommand):
def __init__(self, api, name, may_exist):
def __init__(self, api, name, may_exist, datapath_type):
super(AddBridgeCommand, self).__init__(api)
self.name = name
self.may_exist = may_exist
self.datapath_type = datapath_type

def run_idl(self, txn):
if self.may_exist:
Expand All @@ -63,6 +64,8 @@ def run_idl(self, txn):
return
row = txn.insert(self.api._tables['Bridge'])
row.name = self.name
if self.datapath_type:
row.datapath_type = self.datapath_type
self.api._ovs.verify('bridges')
self.api._ovs.bridges = self.api._ovs.bridges + [row]

Expand Down
Expand Up @@ -47,6 +47,10 @@
"integration bridge to physical bridges.")),
cfg.StrOpt('of_interface', default='ovs-ofctl', choices=['ovs-ofctl'],
help=_("OpenFlow interface to use.")),
cfg.StrOpt('datapath_type', default=constants.OVS_DATAPATH_SYSTEM,
choices=[constants.OVS_DATAPATH_SYSTEM,
constants.OVS_DATAPATH_NETDEV],
help=_("OVS datapath to use.")),
]

agent_opts = [
Expand Down
Expand Up @@ -90,3 +90,7 @@
OVS_DEAD = 2

EXTENSION_DRIVER_TYPE = 'ovs'

# ovs datapath types
OVS_DATAPATH_SYSTEM = 'system'
OVS_DATAPATH_NETDEV = 'netdev'
Expand Up @@ -19,6 +19,7 @@
import time
import uuid

import functools
import netaddr
from oslo_config import cfg
from oslo_log import log as logging
Expand Down Expand Up @@ -173,9 +174,14 @@ def __init__(self, bridge_classes, integ_br, tun_br, local_ip,
:param conf: an instance of ConfigOpts
'''
super(OVSNeutronAgent, self).__init__()
self.br_int_cls = bridge_classes['br_int']
self.br_phys_cls = bridge_classes['br_phys']
self.br_tun_cls = bridge_classes['br_tun']
self.conf = conf or cfg.CONF

# init bridge classes with configured datapath type.
self.br_int_cls, self.br_phys_cls, self.br_tun_cls = (
functools.partial(bridge_classes[b],
datapath_type=self.conf.OVS.datapath_type)
for b in ('br_int', 'br_phys', 'br_tun'))

self.use_veth_interconnection = use_veth_interconnection
self.veth_mtu = veth_mtu
self.available_local_vlans = set(moves.range(p_const.MIN_VLAN_TAG,
Expand All @@ -188,7 +194,6 @@ def __init__(self, bridge_classes, integ_br, tun_br, local_ip,
self.enable_distributed_routing = enable_distributed_routing
self.arp_responder_enabled = arp_responder and self.l2_pop
self.prevent_arp_spoofing = prevent_arp_spoofing
self.conf = conf or cfg.CONF

self.agent_state = {
'binary': 'neutron-openvswitch-agent',
Expand Down
30 changes: 30 additions & 0 deletions neutron/tests/functional/agent/test_l2_ovs_agent.py
Expand Up @@ -16,6 +16,7 @@

import time

from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants
from neutron.tests.common import net_helpers
from neutron.tests.functional.agent.l2 import base

Expand All @@ -32,6 +33,35 @@ def test_port_creation_and_deletion(self):

self.wait_until_ports_state(self.ports, up=False)

def test_datapath_type_system(self):
expected = constants.OVS_DATAPATH_SYSTEM
agent = self.create_agent()
self.start_agent(agent)
actual = self.ovs.db_get_val('Bridge',
agent.int_br.br_name,
'datapath_type')
self.assertEqual(expected, actual)
actual = self.ovs.db_get_val('Bridge',
agent.tun_br.br_name,
'datapath_type')
self.assertEqual(expected, actual)

def test_datapath_type_netdev(self):
expected = constants.OVS_DATAPATH_NETDEV
self.config.set_override('datapath_type',
expected,
"OVS")
agent = self.create_agent()
self.start_agent(agent)
actual = self.ovs.db_get_val('Bridge',
agent.int_br.br_name,
'datapath_type')
self.assertEqual(expected, actual)
actual = self.ovs.db_get_val('Bridge',
agent.tun_br.br_name,
'datapath_type')
self.assertEqual(expected, actual)

def test_resync_devices_set_up_after_exception(self):
self.setup_agent_and_ports(
port_dicts=self.create_test_ports(),
Expand Down
12 changes: 12 additions & 0 deletions neutron/tests/unit/agent/common/test_ovs_lib.py
Expand Up @@ -22,6 +22,8 @@
from neutron.agent.common import utils
from neutron.common import exceptions
from neutron.plugins.common import constants
from neutron.plugins.ml2.drivers.openvswitch.agent.common \
import constants as p_const
from neutron.tests import base
from neutron.tests import tools

Expand Down Expand Up @@ -255,6 +257,16 @@ def test_get_port_ofport_returns_invalid_for_invalid(self):
self._test_get_port_ofport(ovs_lib.INVALID_OFPORT,
ovs_lib.INVALID_OFPORT)

def test_default_datapath(self):
# verify kernel datapath is default
expected = p_const.OVS_DATAPATH_SYSTEM
self.assertEqual(expected, self.br.datapath_type)

def test_non_default_datapath(self):
expected = p_const.OVS_DATAPATH_NETDEV
self.br = ovs_lib.OVSBridge(self.BR_NAME, datapath_type=expected)
self.assertEqual(expected, self.br.datapath_type)

def test_count_flows(self):
self.execute.return_value = 'ignore\nflow-1\n'
# counts the number of flows as total lines of output - 2
Expand Down
Expand Up @@ -187,6 +187,37 @@ def _test_restore_local_vlan_maps(self, tag):
else:
self.assertFalse(provision_local_vlan.called)

def test_datapath_type_system(self):
# verify kernel datapath is default
expected = constants.OVS_DATAPATH_SYSTEM
self.assertEqual(expected, self.agent.int_br.datapath_type)

def test_datapath_type_netdev(self):

with mock.patch.object(self.mod_agent.OVSNeutronAgent,
'setup_integration_br'), \
mock.patch.object(self.mod_agent.OVSNeutronAgent,
'setup_ancillary_bridges',
return_value=[]), \
mock.patch('neutron.agent.linux.utils.get_interface_mac',
return_value='00:00:00:00:00:01'), \
mock.patch(
'neutron.agent.common.ovs_lib.BaseOVS.get_bridges'), \
mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall',
new=MockFixedIntervalLoopingCall), \
mock.patch(
'neutron.agent.common.ovs_lib.OVSBridge.' 'get_vif_ports',
return_value=[]):
# validate setting non default datapath
expected = constants.OVS_DATAPATH_NETDEV
cfg.CONF.set_override('datapath_type',
expected,
group='OVS')
kwargs = self.mod_agent.create_agent_config_map(cfg.CONF)
self.agent = self.mod_agent.OVSNeutronAgent(self._bridge_classes(),
**kwargs)
self.assertEqual(expected, self.agent.int_br.datapath_type)

def test_restore_local_vlan_map_with_device_has_tag(self):
self._test_restore_local_vlan_maps(2)

Expand Down
Expand Up @@ -163,13 +163,16 @@ def lookup_br(br_name, *args, **kwargs):

def _define_expected_calls(self, arp_responder=False):
self.mock_int_bridge_cls_expected = [
mock.call(self.INT_BRIDGE),
mock.call(self.INT_BRIDGE,
datapath_type=mock.ANY),
]
self.mock_phys_bridge_cls_expected = [
mock.call(self.MAP_TUN_BRIDGE),
mock.call(self.MAP_TUN_BRIDGE,
datapath_type=mock.ANY),
]
self.mock_tun_bridge_cls_expected = [
mock.call(self.TUN_BRIDGE),
mock.call(self.TUN_BRIDGE,
datapath_type=mock.ANY),
]

self.mock_int_bridge = self.ovs_bridges[self.INT_BRIDGE]
Expand Down Expand Up @@ -577,13 +580,16 @@ class TunnelTestUseVethInterco(TunnelTest):

def _define_expected_calls(self, arp_responder=False):
self.mock_int_bridge_cls_expected = [
mock.call(self.INT_BRIDGE),
mock.call(self.INT_BRIDGE,
datapath_type=mock.ANY),
]
self.mock_phys_bridge_cls_expected = [
mock.call(self.MAP_TUN_BRIDGE),
mock.call(self.MAP_TUN_BRIDGE,
datapath_type=mock.ANY),
]
self.mock_tun_bridge_cls_expected = [
mock.call(self.TUN_BRIDGE),
mock.call(self.TUN_BRIDGE,
datapath_type=mock.ANY),
]

self.mock_int_bridge_expected = [
Expand Down

0 comments on commit 5b708d5

Please sign in to comment.