From 54e1a6b1bc378c0745afc03987d0fea241b826ae Mon Sep 17 00:00:00 2001 From: Moshe Levi Date: Tue, 10 Dec 2019 23:11:54 +0200 Subject: [PATCH] don't clear skb mark when ovs is hw-offload enabled skb mark is not supported when using ovs hw-offload and using it breaks the vxlan offload. This patch clear skb mark only if ovs hw-offload is disabled. This should be fine as ovs with hw-offload runs on the compute node (DVR is not supported), so clear the skb mark for the qrouter is not needed. Closes-Bug: #1855888 Conflicts: neutron/agent/common/ovs_lib.py Change-Id: I71f45fcd9b7e7bdacaafc7fa96c775e88333ab48 (cherry picked from commit a75ec08ddb6a672ba685a11ba3f7df1569497723) (cherry picked from commit bee5059ccb35e23d3c84ae8f959fa695f8cae931) --- neutron/agent/common/ovs_lib.py | 14 ++++++++++- .../tests/unit/agent/common/test_ovs_lib.py | 25 +++++++++++++++++++ .../agent/openflow/ovs_ofctl/test_br_tun.py | 2 ++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 164e31d87eb..0a50c8fde07 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -115,6 +115,7 @@ class BaseOVS(object): def __init__(self): self.vsctl_timeout = cfg.CONF.OVS.ovsdb_timeout self.ovsdb = ovsdb_api.from_config(self) + self._hw_offload = None def add_manager(self, connection_uri, timeout=_SENTINEL): """Have ovsdb-server listen for manager connections @@ -195,6 +196,13 @@ class BaseOVS(object): _cfg = self.config return {k: _cfg.get(k, OVS_DEFAULT_CAPS[k]) for k in OVS_DEFAULT_CAPS} + @property + def is_hw_offload_enabled(self): + if self._hw_offload is None: + self._hw_offload = self.config.get('other_config', + {}).get('hw-offload', '').lower() == 'true' + return self._hw_offload + # Map from version string to on-the-wire protocol version encoding: OF_PROTOCOL_TO_VERSION = { @@ -503,7 +511,11 @@ class OVSBridge(BaseOVS): options['local_ip'] = local_ip options['in_key'] = 'flow' options['out_key'] = 'flow' - options['egress_pkt_mark'] = '0' + # NOTE(moshele): pkt_mark is not upported when using ovs hw-offload, + # therefore avoid clear mark on encapsulating packets when it's + # enabled + if not self.is_hw_offload_enabled: + options['egress_pkt_mark'] = '0' if tunnel_csum: options['csum'] = str(tunnel_csum).lower() if tos: diff --git a/neutron/tests/unit/agent/common/test_ovs_lib.py b/neutron/tests/unit/agent/common/test_ovs_lib.py index 670dccc3c06..8736745f0a2 100644 --- a/neutron/tests/unit/agent/common/test_ovs_lib.py +++ b/neutron/tests/unit/agent/common/test_ovs_lib.py @@ -491,6 +491,7 @@ class OVS_Lib_Test(base.BaseTestCase): self.assertEqual(1, self.execute.call_count) def test_add_tunnel_port(self): + self.br._hw_offload = False pname = "tap99" local_ip = "1.1.1.1" remote_ip = "9.9.9.9" @@ -519,6 +520,7 @@ class OVS_Lib_Test(base.BaseTestCase): tools.verify_mock_calls(self.execute, expected_calls_and_values) def test_add_vxlan_fragmented_tunnel_port(self): + self.br._hw_offload = False pname = "tap99" local_ip = "1.1.1.1" remote_ip = "9.9.9.9" @@ -552,6 +554,7 @@ class OVS_Lib_Test(base.BaseTestCase): tools.verify_mock_calls(self.execute, expected_calls_and_values) def test_add_vxlan_csum_tunnel_port(self): + self.br._hw_offload = False pname = "tap99" local_ip = "1.1.1.1" remote_ip = "9.9.9.9" @@ -587,6 +590,7 @@ class OVS_Lib_Test(base.BaseTestCase): tools.verify_mock_calls(self.execute, expected_calls_and_values) def test_add_vxlan_tos_tunnel_port(self): + self.br._hw_offload = False pname = "tap99" local_ip = "1.1.1.1" remote_ip = "9.9.9.9" @@ -1029,6 +1033,27 @@ class OVS_Lib_Test(base.BaseTestCase): set_ctrl_field_mock.assert_called_once_with( 'controller_burst_limit', ovs_lib.CTRL_BURST_LIMIT_MIN) + def test_hw_offload_enabled_false(self): + config_mock1 = mock.PropertyMock(return_value={"other_config": {}}) + config_mock2 = mock.PropertyMock( + return_value={"other_config": {"hw-offload": "false"}}) + config_mock3 = mock.PropertyMock( + return_value={"other_config": {"hw-offload": "False"}}) + for config_mock in (config_mock1, config_mock2, config_mock3): + with mock.patch("neutron.agent.common.ovs_lib.OVSBridge.config", + new_callable=config_mock): + self.assertFalse(self.br.is_hw_offload_enabled) + + def test_hw_offload_enabled_true(self): + config_mock1 = mock.PropertyMock( + return_value={"other_config": {"hw-offload": "true"}}) + config_mock2 = mock.PropertyMock( + return_value={"other_config": {"hw-offload": "True"}}) + for config_mock in (config_mock1, config_mock2): + with mock.patch("neutron.agent.common.ovs_lib.OVSBridge.config", + new_callable=config_mock): + self.assertTrue(self.br.is_hw_offload_enabled) + class TestDeferredOVSBridge(base.BaseTestCase): diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_tun.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_tun.py index 574a4f2062b..0dbad4a37a0 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_tun.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_tun.py @@ -308,12 +308,14 @@ class OVSTunnelBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase, self.assertEqual([call(port_name)], delete_port.mock_calls) def test_add_tunnel_port(self): + self.br._hw_offload = False self._mock_add_tunnel_port() def test_delete_port(self): self._mock_delete_port() def test_deferred_br_add_tunnel_port(self): + self.br._hw_offload = False self._mock_add_tunnel_port(True) def test_deferred_br_delete_port(self):