From 759b4e2f113220acd4077900586bda37dc7c0dc5 Mon Sep 17 00:00:00 2001 From: Alexander Vlasov Date: Wed, 18 Mar 2020 13:35:20 -0500 Subject: [PATCH] Workaround for TCP checksum issue with ovs-dpdk and veth pair The need for this change stems from following issues: 1) When ovs_use_veth = False with ovs-dpdk issue with ovs was observed - after vswitch restart interface is not comming up. Meaning ovs-dpdk uses ovs internal ports and it is not able to bring them up on restart. 2) When ovs_use_veth = True and ovs-dpkd is used, packets sent with incorrect checksum due to the fact that ovs-dpdk does not do checksum calculations for veth interface. This commit allows to use second option and resolve checksum issue by disabling checksum offload. Closes-Bug: #1832021 Related-Bug: #1831935 Change-Id: Iecce8d2c6c2c46718cc1020c6e8f914cd4560e4b (cherry picked from commit 11838a2bc50caa40e776bce211f5f2d1d16a14af) --- etc/neutron/rootwrap.d/dhcp.filters | 1 + etc/neutron/rootwrap.d/l3.filters | 1 + neutron/agent/linux/ethtool.py | 34 +++++++++++++++++++ neutron/agent/linux/interface.py | 10 ++++++ .../tests/unit/agent/linux/test_interface.py | 15 ++++++++ 5 files changed, 61 insertions(+) create mode 100644 neutron/agent/linux/ethtool.py diff --git a/etc/neutron/rootwrap.d/dhcp.filters b/etc/neutron/rootwrap.d/dhcp.filters index d48d2eac2be..8eb6c8c85e2 100644 --- a/etc/neutron/rootwrap.d/dhcp.filters +++ b/etc/neutron/rootwrap.d/dhcp.filters @@ -10,6 +10,7 @@ # dhcp-agent dnsmasq: CommandFilter, dnsmasq, root +ethtool: CommandFilter, ethtool, root # dhcp-agent uses kill as well, that's handled by the generic KillFilter # it looks like these are the only signals needed, per # neutron/agent/linux/dhcp.py diff --git a/etc/neutron/rootwrap.d/l3.filters b/etc/neutron/rootwrap.d/l3.filters index 8098cc5beca..1c19815423f 100644 --- a/etc/neutron/rootwrap.d/l3.filters +++ b/etc/neutron/rootwrap.d/l3.filters @@ -12,6 +12,7 @@ arping: CommandFilter, arping, root # l3_agent +ethtool: CommandFilter, ethtool, root sysctl: CommandFilter, sysctl, root route: CommandFilter, route, root radvd: CommandFilter, radvd, root diff --git a/neutron/agent/linux/ethtool.py b/neutron/agent/linux/ethtool.py new file mode 100644 index 00000000000..b28c41a8c25 --- /dev/null +++ b/neutron/agent/linux/ethtool.py @@ -0,0 +1,34 @@ +# Copyright 2020 OpenStack Foundation +# All Rights Reserved. +# +# 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. + +from neutron.agent.linux import ip_lib + + +class Ethtool(object): + + COMMAND = 'ethtool' + + @staticmethod + def _cmd(cmd, namespace, **kwargs): + ip_wrapper = ip_lib.IPWrapper(namespace) + return ip_wrapper.netns.execute(cmd, run_as_root=True, **kwargs) + + @classmethod + def offload(cls, device, rx, tx, namespace=None): + rx = 'on' if rx else 'off' + tx = 'on' if tx else 'off' + cmd = ['--offload', device, 'rx', rx, 'tx', tx] + cmd = [cls.COMMAND] + cmd + cls._cmd(cmd, namespace) diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index a2789857d54..4e5d2755587 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -23,10 +23,14 @@ from oslo_log import versionutils import six from neutron.agent.common import ovs_lib +from neutron.agent.linux import ethtool from neutron.agent.linux import ip_lib from neutron.agent.linux import utils from neutron.common import constants as n_const from neutron.common import exceptions +from neutron.conf.plugins.ml2.drivers import ovs_conf +from neutron.plugins.ml2.drivers.openvswitch.agent.common \ + import constants as ovs_const LOG = logging.getLogger(__name__) @@ -356,6 +360,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): def __init__(self, conf): super(OVSInterfaceDriver, self).__init__(conf) + ovs_conf.register_ovs_agent_opts(self.conf) if self.conf.ovs_use_veth: self.DEV_NAME_PREFIX = 'ns-' @@ -436,6 +441,11 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): if link_up: ns_dev.link.set_up() if self.conf.ovs_use_veth: + # ovs-dpdk does not do checksum calculations for veth interface + # (bug 1832021) + if self.conf.OVS.datapath_type == ovs_const.OVS_DATAPATH_NETDEV: + ethtool.Ethtool.offload(ns_dev.name, rx=False, tx=False, + namespace=namespace) root_dev.link.set_up() def unplug(self, device_name, bridge=None, namespace=None, prefix=None): diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py index d9bc5c688c4..5e7973879c8 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -17,10 +17,14 @@ import mock from neutron_lib import constants from neutron.agent.common import ovs_lib +from neutron.agent.linux import ethtool from neutron.agent.linux import interface from neutron.agent.linux import ip_lib from neutron.agent.linux import utils from neutron.conf.agent import common as config +from neutron.conf.plugins.ml2.drivers import ovs_conf +from neutron.plugins.ml2.drivers.openvswitch.agent.common \ + import constants as ovs_const from neutron.tests import base @@ -70,6 +74,8 @@ class TestBase(base.BaseTestCase): super(TestBase, self).setUp() self.conf = config.setup_conf() config.register_interface_opts(self.conf) + self.eth_tool_p = mock.patch.object(ethtool, 'Ethtool') + self.eth_tool = self.eth_tool_p.start() self.ip_dev_p = mock.patch.object(ip_lib, 'IPDevice') self.ip_dev = self.ip_dev_p.start() self.ip_p = mock.patch.object(ip_lib, 'IPWrapper') @@ -483,7 +489,12 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): def setUp(self): super(TestOVSInterfaceDriverWithVeth, self).setUp() + ovs_conf.register_ovs_agent_opts(self.conf) self.conf.set_override('ovs_use_veth', True) + self.conf.set_override( + 'datapath_type', + ovs_const.OVS_DATAPATH_NETDEV, + group='OVS') def test_get_device_name(self): br = interface.OVSInterfaceDriver(self.conf) @@ -514,6 +525,7 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): mock.patch.object( interface, '_get_veth', return_value=(root_dev, ns_dev)).start() + ns_dev.name = devname expected = [mock.call(), mock.call().add_veth('tap0', devname, @@ -544,6 +556,9 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): self.ip.assert_has_calls(expected) root_dev.assert_has_calls([mock.call.link.set_up()]) ns_dev.assert_has_calls([mock.call.link.set_up()]) + self.eth_tool.assert_has_calls([mock.call.offload( + devname, rx=False, + tx=False, namespace=namespace)]) def test_unplug(self, bridge=None): if not bridge: