From 85f95e0bd9a80b952c6591119372d24b79ac24d2 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 0502eb63fdf..758a91d0b1c 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 02706dab0b5..ff21f8c068e 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 fa20b982d7d..4de03e0c85c 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -24,8 +24,12 @@ from oslo_utils import excutils import six from neutron.agent.common import ovs_lib +from neutron.agent.linux import ethtool from neutron.agent.linux import ip_lib from neutron.common import utils +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__) @@ -316,6 +320,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): def __init__(self, conf, **kwargs): super(OVSInterfaceDriver, self).__init__(conf, **kwargs) + ovs_conf.register_ovs_agent_opts(self.conf) if self.conf.ovs_use_veth: self.DEV_NAME_PREFIX = 'ns-' @@ -403,6 +408,11 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): 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 0f29074e263..8be4ad7f317 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -19,10 +19,14 @@ from neutron_lib import exceptions from oslo_utils import excutils 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.common 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 @@ -61,6 +65,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') @@ -505,7 +511,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) @@ -536,6 +547,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, @@ -566,6 +578,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_plug_new(self, bridge=None, namespace=None): # The purpose of test_plug_new in parent class(TestOVSInterfaceDriver)