From 12cea89b91c2548ea544f4e1adf0d346528b6b2e Mon Sep 17 00:00:00 2001 From: pranabjb Date: Tue, 31 Jul 2018 15:58:10 +0530 Subject: [PATCH] Support for OVS DB TCP socket communication. While adding a vif to the ovs bridge(br-int), Nova(os-vif) invokes the ovs-vsctl command. This works fine when OVS DB server is listening for connections over a UNIX file socket. OVS DB server can listen for connections over a TCP socket too [0]. If the OVS DB server is listening over a TCP socket, then the ovs-vsctl commands should include the optional argument '--db=tcp:ip:port', else vif addition to OVS bridge would fail. eg: ovs-vsctl --db=tcp:169.254.1.1:6640 add-port br-int eth0 This patch adds a new config parameter: 'ovsdb_connection'. By default, if ovsdb_connection is set to 'None', communication with OVSDB server would happen over the UNIX file socket. If it is set to tcp:IP:Port, ovs-vsctl commands would use the '--db=tcp:IP:Port' parameter for communicating over a TCP socket. Neutron already supports this feature. The ovsdb_connection parameter is configured in openvswitch_agent.ini file. [1] [0] http://www.openvswitch.org/support/dist-docs/ovsdb-server.1.html [1] https://docs.openstack.org/neutron/pike/configuration/openvswitch-agent.html Change-Id: I3b0cadd48fd8823ab2dbec81b2c49dc4d33031ce Closes-Bug: 1778724 --- vif_plug_ovs/linux_net.py | 47 +++++++++----- vif_plug_ovs/ovs.py | 45 ++++++++++---- vif_plug_ovs/tests/unit/test_linux_net.py | 76 ++++++++++++++++++----- vif_plug_ovs/tests/unit/test_plugin.py | 23 ++++--- 4 files changed, 140 insertions(+), 51 deletions(-) diff --git a/vif_plug_ovs/linux_net.py b/vif_plug_ovs/linux_net.py index 4d2f82cd..6a0dbc58 100644 --- a/vif_plug_ovs/linux_net.py +++ b/vif_plug_ovs/linux_net.py @@ -49,10 +49,12 @@ PF_FUNC_RE = re.compile("\.(\d+)", 0) _SRIOV_TOTALVFS = "sriov_totalvfs" -def _ovs_vsctl(args, timeout=None): +def _ovs_vsctl(args, timeout=None, ovsdb_connection=None): full_args = ['ovs-vsctl'] if timeout is not None: full_args += ['--timeout=%s' % timeout] + if ovsdb_connection is not None: + full_args += ['--db=%s' % ovsdb_connection] full_args += args try: return processutils.execute(*full_args) @@ -86,22 +88,27 @@ def _create_ovs_bridge_cmd(bridge, datapath_type): @privsep.vif_plug.entrypoint def create_ovs_vif_port(bridge, dev, iface_id, mac, instance_id, mtu=None, interface_type=None, timeout=None, - vhost_server_path=None): + vhost_server_path=None, ovsdb_connection=None): _ovs_vsctl(_create_ovs_vif_cmd(bridge, dev, iface_id, mac, instance_id, interface_type, - vhost_server_path), timeout=timeout) - _update_device_mtu(dev, mtu, interface_type, timeout=timeout) + vhost_server_path), timeout=timeout, + ovsdb_connection=ovsdb_connection) + _update_device_mtu(dev, mtu, interface_type, timeout=timeout, + ovsdb_connection=ovsdb_connection) @privsep.vif_plug.entrypoint -def update_ovs_vif_port(dev, mtu=None, interface_type=None, timeout=None): - _update_device_mtu(dev, mtu, interface_type, timeout=timeout) +def update_ovs_vif_port(dev, mtu=None, interface_type=None, timeout=None, + ovsdb_connection=None): + _update_device_mtu(dev, mtu, interface_type, timeout=timeout, + ovsdb_connection=ovsdb_connection) @privsep.vif_plug.entrypoint -def delete_ovs_vif_port(bridge, dev, timeout=None, delete_netdev=True): +def delete_ovs_vif_port(bridge, dev, timeout=None, + ovsdb_connection=None, delete_netdev=True): _ovs_vsctl(['--', '--if-exists', 'del-port', bridge, dev], - timeout=timeout) + timeout=timeout, ovsdb_connection=ovsdb_connection) if delete_netdev: _delete_net_dev(dev) @@ -151,8 +158,10 @@ def update_veth_pair(dev1_name, dev2_name, mtu): @privsep.vif_plug.entrypoint -def ensure_ovs_bridge(bridge, datapath_type): - _ovs_vsctl(_create_ovs_bridge_cmd(bridge, datapath_type)) +def ensure_ovs_bridge(bridge, datapath_type, timeout=None, + ovsdb_connection=None): + _ovs_vsctl(_create_ovs_bridge_cmd(bridge, datapath_type), timeout=timeout, + ovsdb_connection=ovsdb_connection) @privsep.vif_plug.entrypoint @@ -192,7 +201,8 @@ def add_bridge_port(bridge, dev): processutils.execute('brctl', 'addif', bridge, dev) -def _update_device_mtu(dev, mtu, interface_type=None, timeout=120): +def _update_device_mtu(dev, mtu, interface_type=None, timeout=120, + ovsdb_connection=None): if not mtu: return if interface_type not in [ @@ -204,8 +214,10 @@ def _update_device_mtu(dev, mtu, interface_type=None, timeout=120): # When plugging an interface on Windows, we therefore skip # programming the MTU and fallback to DHCP advertisement. _set_device_mtu(dev, mtu) - elif _ovs_supports_mtu_requests(timeout=timeout): - _set_mtu_request(dev, mtu, timeout=timeout) + elif _ovs_supports_mtu_requests(timeout=timeout, + ovsdb_connection=ovsdb_connection): + _set_mtu_request(dev, mtu, timeout=timeout, + ovsdb_connection=ovsdb_connection) else: LOG.debug("MTU not set on %(interface_name)s interface " "of type %(interface_type)s.", @@ -225,16 +237,17 @@ def set_interface_state(interface_name, port_state): @privsep.vif_plug.entrypoint -def _set_mtu_request(dev, mtu, timeout=None): +def _set_mtu_request(dev, mtu, timeout=None, ovsdb_connection=None): args = ['--', 'set', 'interface', dev, 'mtu_request=%s' % mtu] - _ovs_vsctl(args, timeout=timeout) + _ovs_vsctl(args, timeout=timeout, ovsdb_connection=ovsdb_connection) @privsep.vif_plug.entrypoint -def _ovs_supports_mtu_requests(timeout=None): +def _ovs_supports_mtu_requests(timeout=None, ovsdb_connection=None): args = ['--columns=mtu_request', 'list', 'interface'] - _, error = _ovs_vsctl(args, timeout=timeout) + _, error = _ovs_vsctl(args, timeout=timeout, + ovsdb_connection=ovsdb_connection) if (error == 'ovs-vsctl: Interface does not contain' + ' a column whose name matches "mtu_request"'): return False diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index dbf462ec..53c9618e 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -53,6 +53,14 @@ class OvsPlugin(plugin.PluginBase): 'wait for a response from the database. 0 is to wait ' 'forever.', deprecated_group="DEFAULT"), + cfg.StrOpt('ovsdb_connection', + default=None, + help='The TCP socket connection for communicating with ' + 'OVS. "None" (default) is for UNIX domain socket ' + 'connection. If set to "tcp:IP:PORT" eg ' + 'tcp:127.0.0.1:6640, ovs-vsctl commands will use the ' + 'tcp:IP:PORT parameter for communicating with OVSDB over ' + 'a TCP socket.'), ) @staticmethod @@ -116,11 +124,14 @@ class OvsPlugin(plugin.PluginBase): vif.address, instance_info.uuid, mtu, timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection, **kwargs) def _update_vif_port(self, vif, vif_name): mtu = self._get_mtu(vif) - linux_net.update_ovs_vif_port(vif_name, mtu) + linux_net.update_ovs_vif_port(vif_name, mtu, + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection) @staticmethod def _get_vif_datapath_type(vif, datapath=constants.OVS_DATAPATH_SYSTEM): @@ -165,7 +176,9 @@ class OvsPlugin(plugin.PluginBase): linux_net.create_veth_pair(v1_name, v2_name, mtu) linux_net.add_bridge_port(vif.bridge_name, v1_name) linux_net.ensure_ovs_bridge(vif.network.bridge, - self._get_vif_datapath_type(vif)) + self._get_vif_datapath_type(vif), + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection) self._create_vif_port(vif, v2_name, instance_info) else: linux_net.update_veth_pair(v1_name, v2_name, mtu) @@ -181,7 +194,9 @@ class OvsPlugin(plugin.PluginBase): def _plug_vf_passthrough(self, vif, instance_info): linux_net.ensure_ovs_bridge( - vif.network.bridge, constants.OVS_DATAPATH_SYSTEM) + vif.network.bridge, constants.OVS_DATAPATH_SYSTEM, + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection) pci_slot = vif.dev_address pf_ifname = linux_net.get_ifname_by_pci_address( pci_slot, pf_interface=True, switchdev=True) @@ -201,7 +216,9 @@ class OvsPlugin(plugin.PluginBase): if isinstance(vif, objects.vif.VIFOpenVSwitch): if sys.platform != constants.PLATFORM_WIN32: linux_net.ensure_ovs_bridge(vif.network.bridge, - self._get_vif_datapath_type(vif)) + self._get_vif_datapath_type(vif), + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection) else: self._plug_vif_windows(vif, instance_info) elif isinstance(vif, objects.vif.VIFBridge): @@ -216,10 +233,11 @@ class OvsPlugin(plugin.PluginBase): def _unplug_vhostuser(self, vif, instance_info): linux_net.delete_ovs_vif_port(vif.network.bridge, - OvsPlugin.gen_port_name( - constants.OVS_VHOSTUSER_PREFIX, - vif.id), - timeout=self.config.ovs_vsctl_timeout) + OvsPlugin.gen_port_name( + constants.OVS_VHOSTUSER_PREFIX, + vif.id), + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection) def _unplug_bridge(self, vif, instance_info): """UnPlug using hybrid strategy @@ -233,13 +251,15 @@ class OvsPlugin(plugin.PluginBase): linux_net.delete_bridge(vif.bridge_name, v1_name) linux_net.delete_ovs_vif_port(vif.network.bridge, v2_name, - timeout=self.config.ovs_vsctl_timeout) + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection) def _unplug_vif_windows(self, vif, instance_info): """Remove port from OVS.""" linux_net.delete_ovs_vif_port(vif.network.bridge, vif.id, - timeout=self.config.ovs_vsctl_timeout) + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection) def _unplug_vf_passthrough(self, vif, instance_info): """Remove port from OVS.""" @@ -252,7 +272,10 @@ class OvsPlugin(plugin.PluginBase): # SR-IOV VF, therefore we just need to remove it from the ovs bridge # and set the status to down linux_net.delete_ovs_vif_port( - vif.network.bridge, representor, delete_netdev=False) + vif.network.bridge, representor, + timeout=self.config.ovs_vsctl_timeout, + ovsdb_connection=self.config.ovsdb_connection, + delete_netdev=False) linux_net.set_interface_state(representor, 'down') def unplug(self, vif, instance_info): diff --git a/vif_plug_ovs/tests/unit/test_linux_net.py b/vif_plug_ovs/tests/unit/test_linux_net.py index 8ab9ed3d..b0f71ee0 100644 --- a/vif_plug_ovs/tests/unit/test_linux_net.py +++ b/vif_plug_ovs/tests/unit/test_linux_net.py @@ -249,14 +249,15 @@ class LinuxNetTest(testtools.TestCase): mock_set_mtu_request, mock_ovs_supports_mtu_requests): mock_ovs_supports_mtu_requests.return_value = True - linux_net.create_ovs_vif_port( 'fake-bridge', 'fake-dev', 'fake-iface-id', 'fake-mac', - "fake-instance-uuid", timeout=42) + "fake-instance-uuid", ovsdb_connection=None, + timeout=42) self.assertTrue(mock_create_cmd.called) self.assertFalse(mock_set_device_mtu.called) - mock_vsctl.assert_called_with('ovs_command', timeout=42) + mock_vsctl.assert_called_with('ovs_command', ovsdb_connection=None, + timeout=42) @mock.patch.object(linux_net, '_ovs_supports_mtu_requests') @mock.patch.object(linux_net, '_set_mtu_request') @@ -275,27 +276,56 @@ class LinuxNetTest(testtools.TestCase): "fake-instance-uuid") self.assertTrue(mock_create_cmd.called) self.assertFalse(mock_set_device_mtu.called) - mock_vsctl.assert_called_with('ovs_command', timeout=None) + mock_vsctl.assert_called_with('ovs_command', ovsdb_connection=None, + timeout=None) + + @mock.patch.object(linux_net, '_ovs_supports_mtu_requests') + @mock.patch.object(linux_net, '_set_mtu_request') + @mock.patch.object(linux_net, '_ovs_vsctl') + @mock.patch.object(linux_net, '_create_ovs_vif_cmd', + return_value='ovs_command') + @mock.patch.object(linux_net, '_set_device_mtu') + def test_ovs_vif_port_with_ovsdb_connection(self, mock_set_device_mtu, + mock_create_cmd, mock_vsctl, + mock_set_mtu_request, + mock_ovs_supports_mtu_requests): + mock_ovs_supports_mtu_requests.return_value = True + linux_net.create_ovs_vif_port( + 'fake-bridge', + 'fake-dev', 'fake-iface-id', 'fake-mac', + "fake-instance-uuid", ovsdb_connection='tcp:127.0.0.1:6640', + timeout=42) + self.assertTrue(mock_create_cmd.called) + self.assertFalse(mock_set_device_mtu.called) + mock_vsctl.assert_called_with('ovs_command', + ovsdb_connection='tcp:127.0.0.1:6640', + timeout=42) @mock.patch.object(processutils, "execute") def test_ovs_vsctl(self, mock_execute): args = ['fake-args', 42] + ovsdb_connection = 'tcp:127.0.0.1:6640' timeout = 42 linux_net._ovs_vsctl(args) - linux_net._ovs_vsctl(args, timeout=timeout) + linux_net._ovs_vsctl(args, ovsdb_connection=ovsdb_connection, + timeout=timeout) mock_execute.assert_has_calls([ mock.call('ovs-vsctl', *args), - mock.call('ovs-vsctl', '--timeout=%s' % timeout, *args)]) + mock.call('ovs-vsctl', '--timeout=%s' % timeout, + '--db=%s' % ovsdb_connection, *args)]) @mock.patch.object(linux_net, '_ovs_vsctl') def test_set_mtu_request(self, mock_vsctl): dev = 'fake-dev' mtu = 'fake-mtu' + ovsdb_connection = None timeout = 120 - linux_net._set_mtu_request(dev, mtu, timeout=timeout) + linux_net._set_mtu_request(dev, mtu, ovsdb_connection=ovsdb_connection, + timeout=timeout) args = ['--', 'set', 'interface', dev, 'mtu_request=%s' % mtu] - mock_vsctl.assert_called_with(args, timeout=timeout) + mock_vsctl.assert_called_with(args, ovsdb_connection=ovsdb_connection, + timeout=timeout) @mock.patch.object(linux_net, '_delete_net_dev') @mock.patch.object(linux_net, '_ovs_vsctl') @@ -303,10 +333,14 @@ class LinuxNetTest(testtools.TestCase): self, mock_vsctl, mock_delete_net_dev): bridge = 'fake-bridge' dev = 'fake-dev' + ovsdb_connection = None timeout = 120 - linux_net.delete_ovs_vif_port(bridge, dev, timeout=timeout) + linux_net.delete_ovs_vif_port(bridge, dev, + ovsdb_connection=ovsdb_connection, + timeout=timeout) args = ['--', '--if-exists', 'del-port', bridge, dev] - mock_vsctl.assert_called_with(args, timeout=timeout) + mock_vsctl.assert_called_with(args, ovsdb_connection=ovsdb_connection, + timeout=timeout) mock_delete_net_dev.assert_called() @mock.patch.object(linux_net, '_delete_net_dev') @@ -314,28 +348,38 @@ class LinuxNetTest(testtools.TestCase): def test_delete_ovs_vif_port(self, mock_vsctl, mock_delete_net_dev): bridge = 'fake-bridge' dev = 'fake-dev' + ovsdb_connection = None timeout = 120 linux_net.delete_ovs_vif_port( - bridge, dev, timeout=timeout, delete_netdev=False) + bridge, dev, ovsdb_connection=ovsdb_connection, timeout=timeout, + delete_netdev=False) args = ['--', '--if-exists', 'del-port', bridge, dev] - mock_vsctl.assert_called_with(args, timeout=timeout) + mock_vsctl.assert_called_with(args, ovsdb_connection=ovsdb_connection, + timeout=timeout) mock_delete_net_dev.assert_not_called() @mock.patch.object(linux_net, '_ovs_vsctl') def test_ovs_supports_mtu_requests(self, mock_vsctl): args = ['--columns=mtu_request', 'list', 'interface'] + ovsdb_connection = None timeout = 120 msg = 'ovs-vsctl: Interface does not contain' + \ ' a column whose name matches "mtu_request"' mock_vsctl.return_value = (None, msg) - result = linux_net._ovs_supports_mtu_requests(timeout=timeout) - mock_vsctl.assert_called_with(args, timeout=timeout) + result = linux_net._ovs_supports_mtu_requests( + ovsdb_connection=ovsdb_connection, + timeout=timeout) + mock_vsctl.assert_called_with(args, ovsdb_connection=ovsdb_connection, + timeout=timeout) self.assertFalse(result) mock_vsctl.return_value = (None, None) - result = linux_net._ovs_supports_mtu_requests(timeout=timeout) - mock_vsctl.assert_called_with(args, timeout=timeout) + result = linux_net._ovs_supports_mtu_requests( + ovsdb_connection=ovsdb_connection, + timeout=timeout) + mock_vsctl.assert_called_with(args, ovsdb_connection=ovsdb_connection, + timeout=timeout) self.assertTrue(result) @mock.patch('six.moves.builtins.open') diff --git a/vif_plug_ovs/tests/unit/test_plugin.py b/vif_plug_ovs/tests/unit/test_plugin.py index 875d91b2..c1754b2d 100644 --- a/vif_plug_ovs/tests/unit/test_plugin.py +++ b/vif_plug_ovs/tests/unit/test_plugin.py @@ -128,6 +128,7 @@ class PluginTest(testtools.TestCase): self.vif_ovs.port_profile.interface_id, self.vif_ovs.address, self.instance.uuid, plugin.config.network_device_mtu, + ovsdb_connection=plugin.config.ovsdb_connection, timeout=plugin.config.ovs_vsctl_timeout, interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) @@ -143,6 +144,7 @@ class PluginTest(testtools.TestCase): self.vif_ovs.port_profile.interface_id, self.vif_ovs.address, self.instance.uuid, self.network_ovs_mtu.mtu, + ovsdb_connection=plugin.config.ovsdb_connection, timeout=plugin.config.ovs_vsctl_timeout, interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) @@ -156,7 +158,7 @@ class PluginTest(testtools.TestCase): plugin.plug(self.vif_ovs, self.instance) dp_type = ovs.OvsPlugin._get_vif_datapath_type(self.vif_ovs) ensure_ovs_bridge.assert_called_once_with(self.vif_ovs.network.bridge, - dp_type) + dp_type, ovsdb_connection=None, timeout=120) @mock.patch.object(linux_net, 'set_interface_state') @mock.patch.object(linux_net, 'ensure_ovs_bridge') @@ -192,7 +194,9 @@ class PluginTest(testtools.TestCase): '_create_vif_port': [mock.call(self.vif_ovs_hybrid, 'qvob679325f-ca', self.instance)], - 'ensure_ovs_bridge': [mock.call('br0', dp_type)] + 'ensure_ovs_bridge': [mock.call('br0', dp_type, + ovsdb_connection=None, + timeout=120)] } # plugging new devices should result in devices being created @@ -266,7 +270,7 @@ class PluginTest(testtools.TestCase): calls = { 'delete_bridge': [mock.call('qbrvif-xxx-yyy', 'qvbb679325f-ca')], 'delete_ovs_vif_port': [mock.call('br0', 'qvob679325f-ca', - timeout=120)] + ovsdb_connection=None, timeout=120)] } mock_sys.platform = 'linux' plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) @@ -280,7 +284,8 @@ class PluginTest(testtools.TestCase): mock_sys.platform = constants.PLATFORM_WIN32 plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) plugin.unplug(vif, self.instance) - delete_ovs_vif_port.assert_called_once_with('br0', vif.id, timeout=120) + delete_ovs_vif_port.assert_called_once_with('br0', vif.id, + ovsdb_connection=None, timeout=120) def test_unplug_ovs_windows(self): self._check_unplug_ovs_windows(self.vif_ovs) @@ -321,6 +326,7 @@ class PluginTest(testtools.TestCase): 'f0000000-0000-0000-0000-000000000001', 1500, interface_type='dpdkvhostuserclient', vhost_server_path='/var/run/openvswitch/vhub679325f-ca', + ovsdb_connection=None, timeout=120)], 'ensure_ovs_bridge': [mock.call('br0', dp_type)] } @@ -334,7 +340,7 @@ class PluginTest(testtools.TestCase): def test_unplug_ovs_vhostuser(self, delete_ovs_vif_port): calls = { 'delete_ovs_vif_port': [mock.call('br0', 'vhub679325f-ca', - timeout=120)] + ovsdb_connection=None, timeout=120)] } plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) plugin.unplug(self.vif_vhostuser, self.instance) @@ -359,7 +365,9 @@ class PluginTest(testtools.TestCase): calls = { 'ensure_ovs_bridge': [mock.call('br0', - constants.OVS_DATAPATH_SYSTEM)], + constants.OVS_DATAPATH_SYSTEM, + ovsdb_connection=None, + timeout=120)], 'get_ifname_by_pci_address': [mock.call('0002:24:12.3', pf_interface=True, switchdev=True)], @@ -401,7 +409,8 @@ class PluginTest(testtools.TestCase): 'get_vf_num_by_pci_address': [mock.call('0002:24:12.3')], 'get_representor_port': [mock.call('eth0', '2')], 'set_interface_state': [mock.call('eth0_2', 'down')], - 'delete_ovs_vif_port': [mock.call('br0', 'eth0_2', + 'delete_ovs_vif_port': [mock.call('br0', 'eth0_2', timeout=120, + ovsdb_connection=None, delete_netdev=False)] }