From 63b03362821deadc0e65cce54b11cb7d4c2262d2 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Fri, 26 Jun 2015 10:48:26 +0100 Subject: [PATCH] Add config option to specify ovs datapath. This change introduces a new datapath_type parameter to allow specification of the ovs datapath to be used. This change introduces new functional and unit tests. DocImpact Change-Id: I929d8d15fc6cfdb799c53ef0f3722f4ed5c1096d Partial-Bug: #1469871 --- etc/neutron/plugins/ml2/openvswitch_agent.ini | 5 +++ neutron/agent/common/ovs_lib.py | 19 +++++++++--- neutron/agent/ovsdb/api.py | 14 +++++---- neutron/agent/ovsdb/impl_idl.py | 4 +-- neutron/agent/ovsdb/impl_vsctl.py | 8 +++-- neutron/agent/ovsdb/native/commands.py | 5 ++- .../openvswitch/agent/common/config.py | 4 +++ .../openvswitch/agent/common/constants.py | 4 +++ .../openvswitch/agent/ovs_neutron_agent.py | 13 +++++--- .../functional/agent/test_l2_ovs_agent.py | 30 ++++++++++++++++++ .../tests/unit/agent/common/test_ovs_lib.py | 12 +++++++ .../agent/test_ovs_neutron_agent.py | 31 +++++++++++++++++++ .../openvswitch/agent/test_ovs_tunnel.py | 18 +++++++---- 13 files changed, 141 insertions(+), 26 deletions(-) diff --git a/etc/neutron/plugins/ml2/openvswitch_agent.ini b/etc/neutron/plugins/ml2/openvswitch_agent.ini index b6fd3e01a..99cbaca54 100644 --- a/etc/neutron/plugins/ml2/openvswitch_agent.ini +++ b/etc/neutron/plugins/ml2/openvswitch_agent.ini @@ -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 diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 9c64f67d1..fc0927543 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -30,6 +30,8 @@ from neutron.agent.ovsdb import api as ovsdb 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 @@ -102,8 +104,11 @@ class BaseOVS(object): 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) @@ -143,9 +148,10 @@ class BaseOVS(object): 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, @@ -173,7 +179,9 @@ class OVSBridge(BaseOVS): 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)) @@ -186,7 +194,8 @@ class OVSBridge(BaseOVS): 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)) diff --git a/neutron/agent/ovsdb/api.py b/neutron/agent/ovsdb/api.py index 56a4c2be6..7dc88d02d 100644 --- a/neutron/agent/ovsdb/api.py +++ b/neutron/agent/ovsdb/api.py @@ -95,14 +95,16 @@ class API(object): """ @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 diff --git a/neutron/agent/ovsdb/impl_idl.py b/neutron/agent/ovsdb/impl_idl.py index 4aed00acd..c4459b94e 100644 --- a/neutron/agent/ovsdb/impl_idl.py +++ b/neutron/agent/ovsdb/impl_idl.py @@ -144,8 +144,8 @@ class OvsdbIdl(api.API): 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) diff --git a/neutron/agent/ovsdb/impl_vsctl.py b/neutron/agent/ovsdb/impl_vsctl.py index e410c4100..306f5e486 100644 --- a/neutron/agent/ovsdb/impl_vsctl.py +++ b/neutron/agent/ovsdb/impl_vsctl.py @@ -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 diff --git a/neutron/agent/ovsdb/native/commands.py b/neutron/agent/ovsdb/native/commands.py index b5f873a66..beb185a58 100644 --- a/neutron/agent/ovsdb/native/commands.py +++ b/neutron/agent/ovsdb/native/commands.py @@ -50,10 +50,11 @@ class BaseCommand(api.Command): 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: @@ -63,6 +64,8 @@ class AddBridgeCommand(BaseCommand): 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] diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/config.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/config.py index 7d866b6e8..56e86f766 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/config.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/config.py @@ -47,6 +47,10 @@ ovs_opts = [ "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 = [ diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py index ad6b897c2..6dde277a8 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py @@ -90,3 +90,7 @@ OVS_NORMAL = 1 OVS_DEAD = 2 EXTENSION_DRIVER_TYPE = 'ovs' + +# ovs datapath types +OVS_DATAPATH_SYSTEM = 'system' +OVS_DATAPATH_NETDEV = 'netdev' diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 2122fe339..885a7a59b 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -19,6 +19,7 @@ import sys import time import uuid +import functools import netaddr from oslo_config import cfg from oslo_log import log as logging @@ -173,9 +174,14 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, :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, @@ -188,7 +194,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, 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', diff --git a/neutron/tests/functional/agent/test_l2_ovs_agent.py b/neutron/tests/functional/agent/test_l2_ovs_agent.py index a18d4c5e2..3987c9f54 100644 --- a/neutron/tests/functional/agent/test_l2_ovs_agent.py +++ b/neutron/tests/functional/agent/test_l2_ovs_agent.py @@ -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 @@ -32,6 +33,35 @@ class TestOVSAgent(base.OVSAgentTestFramework): 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(), diff --git a/neutron/tests/unit/agent/common/test_ovs_lib.py b/neutron/tests/unit/agent/common/test_ovs_lib.py index e21fef5f2..b6ab9dd2b 100644 --- a/neutron/tests/unit/agent/common/test_ovs_lib.py +++ b/neutron/tests/unit/agent/common/test_ovs_lib.py @@ -22,6 +22,8 @@ from neutron.agent.common import ovs_lib 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 @@ -255,6 +257,16 @@ class OVS_Lib_Test(base.BaseTestCase): 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 diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 07c02f362..9c5bda57d 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -187,6 +187,37 @@ class TestOvsNeutronAgent(object): 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) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index 315360b8a..cc4f21911 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -156,13 +156,16 @@ class TunnelTest(object): 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] @@ -570,13 +573,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 = [ -- 2.45.2