From f7e31b4c0533687622f8f2644c802574e31536f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elvira=20Garc=C3=ADa?= Date: Thu, 19 Jan 2023 14:48:23 +0100 Subject: [PATCH] [OVN] Allow logging all traffic related to an ACL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this patch, we would only get logged the client to server side of the communication. The OVN allow-related ACL option was implemented [0] so as to be able to log also the packets that are going from server to client. This patch implements the addition of that feature in Neutron and needs OVN version 22.03 or updated 21.12. [0] https://patchwork.ozlabs.org/project/ovn/patch/20220201141118.1846390-1-mmichels@redhat.com/ Closes-Bug: #2003706 Change-Id: I72d061c333f53e07f6feedec032e2c0b06a61248 Signed-off-by: Elvira García --- neutron/services/logapi/drivers/ovn/driver.py | 40 +++++++++++++------ .../logapi/drivers/ovn/test_driver.py | 10 +++++ .../logapi/drivers/ovn/test_driver.py | 5 ++- ...-related-traffic-ovn-96b304ab744de13e.yaml | 6 +++ 4 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/sgl-log-related-traffic-ovn-96b304ab744de13e.yaml diff --git a/neutron/services/logapi/drivers/ovn/driver.py b/neutron/services/logapi/drivers/ovn/driver.py index c5d10a669c9..eeca2f18d38 100644 --- a/neutron/services/logapi/drivers/ovn/driver.py +++ b/neutron/services/logapi/drivers/ovn/driver.py @@ -11,6 +11,7 @@ # under the License. from collections import namedtuple +import random from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks import resources @@ -38,6 +39,7 @@ DRIVER = None log_cfg.register_log_driver_opts() +MAX_INT_LABEL = 2**32 SUPPORTED_LOGGING_TYPES = [log_const.SECURITY_GROUP] @@ -169,13 +171,20 @@ class OVNDriver(base.DriverBase): if log_name: if acl.name and acl.name[0] != log_name: continue + columns = { + 'log': False, + 'meter': [], + 'name': [], + 'severity': [] + } + # TODO(egarciar): There wont be a need to check if label exists + # once minimum version for OVN is >= 22.03 + if hasattr(acl, 'label'): + columns['label'] = 0 + ovn_txn.add(self.ovn_nb.db_remove( + "ACL", acl_uuid, 'options', 'log-related')) ovn_txn.add(self.ovn_nb.db_set( - "ACL", acl_uuid, - ("log", False), - ("meter", []), - ("name", []), - ("severity", []) - )) + "ACL", acl_uuid, *columns.items())) acl_changes += 1 msg = "Cleared %d, Not found %d (out of %d visited) ACLs" if log_name: @@ -191,13 +200,20 @@ class OVNDriver(base.DriverBase): # skip acls used by a different network log if acl.name and acl.name[0] != log_name: continue + columns = { + 'log': acl.action in actions_enabled, + 'meter': self.meter_name, + 'name': log_name, + 'severity': "info" + } + # TODO(egarciar): There wont be a need to check if label exists + # once minimum version for OVN is >= 22.03 + if hasattr(acl, "label"): + # Label needs to be an unsigned 32 bit number and not 0. + columns["label"] = random.randrange(1, MAX_INT_LABEL) + columns["options"] = {'log-related': "true"} ovn_txn.add(self.ovn_nb.db_set( - "ACL", acl_uuid, - ("log", acl.action in actions_enabled), - ("meter", self.meter_name), - ("name", log_name), - ("severity", "info") - )) + "ACL", acl_uuid, *columns.items())) acl_changes += 1 LOG.info("Set %d (out of %d visited) ACLs for network log %s", acl_changes, acl_visits, log_name) diff --git a/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py b/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py index 03f1d365d62..57c7978b8fc 100644 --- a/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py +++ b/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py @@ -151,6 +151,16 @@ class LogApiTestCaseComplex(LogApiTestCaseBase): acl = self._find_security_group_rule_row_by_id(sgr) self.assertIsNotNone(acl) self.assertEqual(is_enabled, acl.log) + if hasattr(acl, "label"): + # Here we compare if there is a name because the log can be + # disabled but disabling a log would not take out the properties + # attached to it. + if acl.name: + self.assertNotEqual(0, acl.label) + self.assertEqual("true", acl.options.get("log-related")) + else: + self.assertEqual(0, acl.label) + self.assertIsNone(acl.options.get("log-related")) return acl def _check_acl_log_drop(self, is_enabled=True): diff --git a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py index 1afa735c10a..35c2ec5765a 100644 --- a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py +++ b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py @@ -278,7 +278,10 @@ class TestOVNDriver(base.BaseTestCase): self.assertEqual(len(pg_dict["acls"]), info_args[1]) self.assertEqual(len(pg_dict["acls"]) - 2, info_args[2]) self.assertEqual(len(pg_dict["acls"]), info_args[3]) - self.assertEqual(len(pg_dict["acls"]), self._nb_ovn.db_set.call_count) + self.assertEqual(len(pg_dict["acls"]), + self._nb_ovn.db_set.call_count) + self.assertEqual(len(pg_dict["acls"]), + self._nb_ovn.db_remove.call_count) @mock.patch.object(ovn_driver.LOG, 'info') def test__remove_acls_log_missing_acls(self, m_info): diff --git a/releasenotes/notes/sgl-log-related-traffic-ovn-96b304ab744de13e.yaml b/releasenotes/notes/sgl-log-related-traffic-ovn-96b304ab744de13e.yaml new file mode 100644 index 00000000000..b3ea740aa54 --- /dev/null +++ b/releasenotes/notes/sgl-log-related-traffic-ovn-96b304ab744de13e.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Neutron can record full connection using log-related feature introduced in + OVN 21.12. + For more info see `bug LP#`