From 7567c42e99b298201b30593699d1e180e5bfa759 Mon Sep 17 00:00:00 2001 From: Kim Bao Long Date: Fri, 10 Aug 2018 14:41:54 +0700 Subject: [PATCH] Remove remaining NFLOG rules on deleting log resource Currently, NFLOG rules are still remaining after deletion of log resources from "ACCEPT" or "DROP" events. This patch aims to remove these rules. In addition, it also cleans up unused iptables manager per port to avoid memory consumption of self.ipt_mgr_list in [1] [1] https://review.openstack.org/#/c/553738/ Closes-Bug: #1786746 Change-Id: Id8db35c9e11c11f186f15565fcbc2cfa67d9ebd4 Co-Authored-By: Nguyen Phuong An (Cherry-picked from commit 6ccdd943a3cec92e559dd842407382a3dca5f484) --- .../logapi/agents/drivers/iptables/log.py | 32 +++++++++++++++++-- .../agents/drivers/iptables/test_log.py | 29 +++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/neutron_fwaas/services/logapi/agents/drivers/iptables/log.py b/neutron_fwaas/services/logapi/agents/drivers/iptables/log.py index 4c17b97f8..c1a44e60c 100644 --- a/neutron_fwaas/services/logapi/agents/drivers/iptables/log.py +++ b/neutron_fwaas/services/logapi/agents/drivers/iptables/log.py @@ -124,6 +124,8 @@ class IptablesLoggingDriver(log_ext.LoggingDriver): self.cleanup_table = defaultdict(set) # Handle NFLOG processing self.nflog_proc_map = {} + # A list of unused ports + self.unused_port_ids = set() def initialize(self, resource_rpc, **kwargs): self.resource_rpc = resource_rpc @@ -195,8 +197,24 @@ class IptablesLoggingDriver(log_ext.LoggingDriver): for prefix in self.prefixes_table[port_id]: self._add_to_cleanup(port_id, prefix.id) del self.prefixes_table[port_id] + self.unused_port_ids.add(port_id) return ipt_mgr_per_port + def _cleanup_unused_ipt_mgrs(self): + + need_cleanup = set() + for port_id in self.unused_port_ids: + for router_id in self.ipt_mgr_list: + if port_id in self.ipt_mgr_list[router_id]: + del self.ipt_mgr_list[router_id][port_id] + if not self.ipt_mgr_list[router_id]: + need_cleanup.add(router_id) + + for router_id in need_cleanup: + del self.ipt_mgr_list[router_id] + + self.unused_port_ids.clear() + def start_logging(self, context, **kwargs): LOG.debug("Start logging: %s", str(kwargs)) @@ -281,11 +299,15 @@ class IptablesLoggingDriver(log_ext.LoggingDriver): # Clean up NFLOG rules self._cleanup_nflog_rules(applied_ipt_mgrs) + # Apply NFLOG rules into iptables managers for ipt_mgr in applied_ipt_mgrs: LOG.debug('Apply NFLOG rules in namespace %s', ipt_mgr.namespace) ipt_mgr.defer_apply_off() + # Clean up unused iptables managers from ports + self._cleanup_unused_ipt_mgrs() + def _cleanup_prefixes_table(self, port_id, log_id): # Each a port has at most 2 prefix @@ -296,11 +318,13 @@ class IptablesLoggingDriver(log_ext.LoggingDriver): if prefix.is_empty: self._add_to_cleanup(port_id, prefix.id) self.prefixes_table[port_id].remove(prefix) - except KeyError: + except Exception: pass if port_id in self.prefixes_table: - del self.prefixes_table[port_id] + if not self.prefixes_table[port_id]: + del self.prefixes_table[port_id] + self.unused_port_ids.add(port_id) def _cleanup_nflog_rules(self, applied_ipt_mgrs): for port_id, prefix_ids in self.cleanup_table.items(): @@ -323,10 +347,14 @@ class IptablesLoggingDriver(log_ext.LoggingDriver): # Clean NFLOG rules: self._cleanup_nflog_rules(applied_ipt_mgrs) + # Apply NFLOG rules into iptables managers for ipt_mgr in applied_ipt_mgrs: ipt_mgr.defer_apply_off() + # Clean up unused iptables managers + self._cleanup_unused_ipt_mgrs() + def _get_if_prefix(self, agent_mode, router): """Get the if prefix from router""" if not router.router.get('distributed'): diff --git a/neutron_fwaas/tests/unit/services/logapi/agents/drivers/iptables/test_log.py b/neutron_fwaas/tests/unit/services/logapi/agents/drivers/iptables/test_log.py index 4021e422a..003f24beb 100644 --- a/neutron_fwaas/tests/unit/services/logapi/agents/drivers/iptables/test_log.py +++ b/neutron_fwaas/tests/unit/services/logapi/agents/drivers/iptables/test_log.py @@ -129,6 +129,22 @@ class BaseIptablesLogTestCase(base.BaseTestCase): self.log_driver.stop_logging(self.context, **fake_kwargs) self.log_driver._delete_firewall_group_log.assert_not_called() + def test_clean_up_unused_ipt_mgrs(self): + f_router_ids = ['r1', 'r2', 'r3'] + self.log_driver.ipt_mgr_list = self._fake_ipt_mgr_list(f_router_ids) + + # Test with a port is delete from router + self.log_driver.unused_port_ids = set(['r1_port1']) + self.log_driver._cleanup_unused_ipt_mgrs() + self.assertEqual(set(), self.log_driver.unused_port_ids) + self.assertIsNone(self.log_driver.ipt_mgr_list['r1'].get('r1_port1')) + + # Test with all ports are deleted from router + self.log_driver.unused_port_ids = set(['r2_port1', 'r2_port2']) + self.log_driver._cleanup_unused_ipt_mgrs() + self.assertEqual(set(), self.log_driver.unused_port_ids) + self.assertIsNone(self.log_driver.ipt_mgr_list.get('r2')) + def test_get_intf_name(self): fake_router = mock.Mock() fake_port_id = 'fake_router_port_id' @@ -310,3 +326,16 @@ class BaseIptablesLogTestCase(base.BaseTestCase): '-j NFLOG --nflog-prefix %s' % (device, FAKE_RATE, FAKE_BURST, tag)] return v4_nflog_rule, v6_nflog_rule + + def _fake_ipt_mgr_list(self, router_ids): + f_ipt_mgrs = defaultdict(dict) + + for router_id in router_ids: + f_port_id1 = router_id + '_port1' + f_port_id2 = router_id + '_port2' + ipt_mgr = mock.Mock() + ipt_mgr.ns_name = 'ns_' + router_id + f_ipt_mgrs[router_id][f_port_id1] = ipt_mgr + f_ipt_mgrs[router_id][f_port_id2] = ipt_mgr + + return f_ipt_mgrs