From 81ec98007dff29f8cdfc5fe23ade60e7bb91ff90 Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Thu, 27 Feb 2025 17:46:10 +0100 Subject: [PATCH] Firewall: Aliases - performance improvement by using pf's overal table stats instead of dumping them. This commit changes PF.list_tables() to yield both the name of the aliases as well as (limited) stats, in places where we only check for totals, these are faster to collect than counting them in python. There should be no functional impact. --- .../scripts/filter/lib/alias/__init__.py | 14 +++++++++++++- src/opnsense/scripts/filter/lib/alias/pf.py | 19 +++++++++++++++++-- src/opnsense/scripts/filter/update_tables.py | 15 ++++++++------- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/opnsense/scripts/filter/lib/alias/__init__.py b/src/opnsense/scripts/filter/lib/alias/__init__.py index ab754272f..39ec1f119 100755 --- a/src/opnsense/scripts/filter/lib/alias/__init__.py +++ b/src/opnsense/scripts/filter/lib/alias/__init__.py @@ -53,6 +53,7 @@ class Alias(object): :return: None """ self._known_aliases = known_aliases + self._pf_addresses = 0 self._is_changed = None self._has_expired = None # general alias properties, excluding content @@ -95,6 +96,12 @@ class Alias(object): # the generated alias contents, without dependencies self._filename_alias_content = '/var/db/aliastables/%s.self.txt' % self._name + def get_pf_addr_count(self): + return self._pf_addresses + + def set_pf_addr_count(self, cnt): + self._pf_addresses = cnt + def items(self): """ return unparsed (raw) alias entries without dependencies :return: iterator @@ -214,6 +221,7 @@ class Alias(object): os.utime(self._filename_alias_hash, None) else: self._resolve_content = set(open(self._filename_alias_content).read().split()) + # return the addresses and networks of this alias return list(self._resolve_content) @@ -287,8 +295,10 @@ class AliasParser(object): self._aliases = dict() external_aliases = list() alias_parameters = dict() + alias_pf_stats = dict() alias_parameters['known_aliases'] = [x.text for x in self._source_tree.iterfind('table/name')] - for alias_name in PF.list_tables(): + for alias_name, alias_info in PF.list_tables(): + alias_pf_stats[alias_name] = alias_info if alias_name not in alias_parameters['known_aliases']: alias_parameters['known_aliases'].append(alias_name) external_aliases.append(alias_name) @@ -302,6 +312,8 @@ class AliasParser(object): # loop through user defined aliases for elem in self._source_tree.iterfind('table'): alias = Alias(elem, **alias_parameters) + if alias.get_name() in alias_pf_stats: + alias.set_pf_addr_count(alias_pf_stats[alias.get_name()].get('addresses', 0)) self._aliases[alias.get_name()] = alias # attach external aliases which aren't defined via the gui diff --git a/src/opnsense/scripts/filter/lib/alias/pf.py b/src/opnsense/scripts/filter/lib/alias/pf.py index 23c2917ca..0c5891aa9 100755 --- a/src/opnsense/scripts/filter/lib/alias/pf.py +++ b/src/opnsense/scripts/filter/lib/alias/pf.py @@ -31,8 +31,23 @@ class PF: @staticmethod def list_tables(): - for line in subprocess.run(['/sbin/pfctl', '-sT'], capture_output=True, text=True).stdout.strip().split('\n'): - yield line.strip() + current_table = None + current_info = {} + for line in subprocess.run(['/sbin/pfctl', '-vvsT'], capture_output=True, text=True).stdout.strip().split('\n'): + parts = line.strip().split() + if len(parts) < 2: + continue + elif line.startswith("\t"): + if parts[0] == 'Addresses:' and parts[1].isdigit(): + current_info['addresses'] = int(parts[1]) + else: + if current_table is not None: + yield current_table, current_info + current_table = parts[1] + current_info = {} + + if current_table is not None: + yield current_table, current_info @staticmethod def list_table(table_name): diff --git a/src/opnsense/scripts/filter/update_tables.py b/src/opnsense/scripts/filter/update_tables.py index 3e8db0dad..910f7c53e 100755 --- a/src/opnsense/scripts/filter/update_tables.py +++ b/src/opnsense/scripts/filter/update_tables.py @@ -101,13 +101,14 @@ if __name__ == '__main__': # read before write, only save when the contents have changed open(alias_filename, 'w').write(alias_content_str) - # list current alias content when not trying to update a targetted list - alias_pf_content = list(PF.list_table(alias_name)) if to_update is None else alias_content + # use current alias content when not trying to update a targetted list + cnt_alias_content = len(alias_content) + cnt_alias_pf_content = alias.get_pf_addr_count() if to_update is None else cnt_alias_content - if (len(alias_content) != len(alias_pf_content) or alias_changed_or_expired): + if (cnt_alias_content != cnt_alias_pf_content or alias_changed_or_expired): # if the alias is changed, expired or the one in memory has a different number of items, load table - if len(alias_content) == 0: - if len(alias_pf_content) > 0: + if cnt_alias_content == 0: + if cnt_alias_pf_content > 0: # flush when target is empty PF.flush(alias_name) else: @@ -117,8 +118,8 @@ if __name__ == '__main__': error_message = "Error loading alias [%s]: %s {current_size: %d, new_size: %d}" % ( alias_name, error_output.replace('pfctl: ', ''), - len(alias_pf_content), - len(alias_content), + cnt_alias_pf_content, + cnt_alias_content, ) result['status'] = 'error' if 'messages' not in result: