# Python Firewall-Connection-Event Filter too slow

I've written a little piece of code to filter Firewall-Connection-Events by a certain group of IPs. But the code can't keep up since the input is fairly huge. I am looking for ways to make this code more efficient in order to handle the Firewall-Connection-Events.

[READ_CLIENTS] The code starts with an empty list (client_IPs). The list is getting filled with unique IPs (read_clients), if the size of the file changes "read_clients" is called again to alter the list.

[READ_EVENTS] To get the events, I've used While True to loop over the event-File and return the events with yield - if there is no new input, then sleep for 0.1 seconds.

[PROCESS_AND_FILTER] After this I loop over the generator object to compare each event to each Unique-IP and seperate the result in two files.

    # -*- coding: utf-8 -*-

from IPy import IP
import os
import time

# Create String-Array to compare
path_unique_ips = '/var/log/unique.log'
# Sophos-UTM Packet-Accepted-Event | Connections to Customer-Net
path_sophos_to_customer = '/var/log/packet-accepted-sophos.log'
# match logs
match_log = '/var/log/matched.log'
no_match_log = '/var/log/not_matched.log'
# IP-Filter-Array
client_IPs = []
#get file size of unique ips file
size_ip_file = os.stat(path_unique_ips).st_size

client_IPs_file = open(path_unique_ips, "r")
if client_IPs_file.mode == 'r':
# read all line from client_IPs_file
#check for new clients and fill array
for new_client_IP in new_client_IPs:
if new_client_IP not in client_IPs:
client_IPs.append(IP(new_client_IP).strNormal())

connection_event_to_customer = open(path_sophos_to_customer, 'r')
connection_event_to_customer.seek(0, 2)
if connection_event_to_customer.mode == 'r':
while True:
if not new_event:
time.sleep(0.1)
continue
yield new_event
#file size of unique IP File changed, re-run the function read_clients
if size_ip_file is not os.stat(path_unique_ips).st_size:

def process_and_filter(my_events):
#get events in generator-object from function read_events
# loop over generator-object, filled with events
for new_event in my_events:
print(new_event)
# loop over event with all ips
for client in client_IPs:
# if client-ip found in event write to match.log and break loop to go for next event
if client in new_event:
with open(match_log, 'a+') as matched:
matched.write(new_event)
break
# if ip wasn't in event write event to no_match.log
else:
with open(no_match_log, 'a+') as no_match:
no_match.write(new_event)

if __name__ == '__main__':
process_and_filter(new_events)


Log_Event_Example :

Jan 18 14:14:14 17.17.17.17 2019: 01:18-14:14:14 firewall-1 ulogd[5974]: id="2002" severity="info" sys="SecureNet" sub="packetfilter" name="Packet accepted" action="accept" fwrule="653" initf="eth1" outitf="eth0" srcmac="aa:bb:cc:dd:ee:ff" dstmac="00:11:22:33:44:55" srcip="10.10.10.10" dstip="10.10.10.11" proto="6" length="52" tos="0x00" prec="0x00" ttl="127" srcport="58589" dstport="22" tcpflags="ACK"

The main performance bottleneck is probably the fact that you keep your already seen IPs in a list. Doing x in list is $$\\mathcal{O}(n)\$$, so the more IPs there are, the slower this will get. Another factor is that you check if new_client_IP not in client_IPs but then add IP(new_client_IP).strNormal() to the list. So even if you come across the same IP again, it will still not be in the list, since that class probably produces something different than the line (which will have a trailing newline, for example) (and if it does not produce anything different, why have this step at all?) and so you do this possibly expensive step again.

Instead just save them in a set, for which in is $$\\mathcal{O}(n)\$$. And even better, if you add an already existing element to a set it does not care.

client_IPs = set()
client_IPs_size = 0

"""Check if the file has changed size.
If it has, extract all IPs from it and update the global variables.
"""
global client_IPs_size
current size = os.stat(path_unique_ips).st_size
if current_size != client_IPs_size:
client_IPs_size = os.stat(path_unique_ips).st_size
global client_IPs
with open(path_unique_ips, "r") as client_IPs_file:
client_IPs = {IP(line).strNormal() for line in client_IPs_file}


Note that I used with to ensure the file is properly closed, even in an event of an exception. I also used a set as stated above.

I also iterate over the file directly instead of first reading all lines into memory. The whole thing fits nicely inside a set comprehension. I also added a docstring to document what this function does.

Modifying global objects can quickly lead to hard to read code and should be avoided if possible. Here this is a bit harder to avoid, because process_and_filter depends on the global state always being correct at that specific point in the loop. Another way around this would be to use class to keep this global state and make the other functions methods of that class. This might be a bit overkill, though, so I leave that implementation to you if you feel it is necessary.

def read_events(path_sophos_to_customer):
"""Every 0.1 seconds check if a new non-blank line has been added to the end of the file.
If there has, yield it.

Read the file containing the list of IPs again if it has changed.
"""
with open(path_sophos_to_customer, 'r') as connection_event_to_customer:
connection_event_to_customer.seek(0, 2)   # seek to the end
while True:
if not new_event:
time.sleep(0.1)
continue
yield new_event

# file size of unique IP File changed, this will read the IPs again


Since the check for a changed client_IPs is now inside read_clients the if clause is not needed anymore.

I also removed the check if the file is readable. Usually you want your code to either fail loudly (with an exception) if something happens (like a file not being readable) or you catch that exception, do something about it and log that it happened. Having it just silently fail by bypassing it with that if clause does not help you at all.

For the splitting of the events into two logs, I would open both files only once at the beginning. I would also extract the IP from the event (since you have not shown us an example of what an event looks like, I will have to guess and say you could use for example re.search(r'\b(?:\d{1,3}\.){3}\d{1,3}\b', new_event).group()). This moves this from having to iterate over all IPs every time to iterating over the string of new_event, which is hopefully shorter.

def process_and_filter(my_events, match_log, no_match_log):
"""Get events in generator-object from function read_events
and write either to matched or no match log file.
"""
with open(match_log, 'a+') as match, open(no_match_log, 'a+') as no_match:
for new_event in my_events:
print(new_event)
ip = extract_ip(new_event)
file = match if ip in client_IPs else no_match
file.write(new_event)


With this the calling code becomes:

if __name__ == '__main__':

Note that Python's official style-guide, PEP8, recommends using ALL_CAPS for global constants.