From a1237df19e677ab55754462e52ad346d340ed56c Mon Sep 17 00:00:00 2001
From: Moshe Levi <moshele@mellanox.com>
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

Change-Id: I71f45fcd9b7e7bdacaafc7fa96c775e88333ab48
(cherry picked from commit a75ec08ddb6a672ba685a11ba3f7df1569497723)
---
 neutron/agent/common/ovs_lib.py               | 14 ++++++++++++-
 .../tests/unit/agent/common/test_ovs_lib.py   | 21 +++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py
index 25496ccecac..95e9f53e75e 100644
--- a/neutron/agent/common/ovs_lib.py
+++ b/neutron/agent/common/ovs_lib.py
@@ -118,6 +118,7 @@ class BaseOVS(object):
     def __init__(self):
         self.ovsdb_timeout = cfg.CONF.OVS.ovsdb_timeout
         self.ovsdb = impl_idl.api_factory()
+        self._hw_offload = None
 
     def add_manager(self, connection_uri, timeout=_SENTINEL):
         """Have ovsdb-server listen for manager connections
@@ -198,6 +199,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 = {
@@ -490,7 +498,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 03465835fe2..587cc48bcfb 100644
--- a/neutron/tests/unit/agent/common/test_ovs_lib.py
+++ b/neutron/tests/unit/agent/common/test_ovs_lib.py
@@ -558,6 +558,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):