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 <AnNP@vn.fujitsu.com>

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'):

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