Skip to content

Conversation

@jkklemm
Copy link
Contributor

@jkklemm jkklemm commented Oct 31, 2023

Since the arm architecture is slow to always lookup the tables, the creation of many rules with standard targets like Accept or Reject can slow down the creation of firewall rules. For this reason, a dictionary is used to check available tables and standard targets are not check if they are a chain.
This code can halve the execution time on an Arm architectures if many firewall rules are created.

@jkklemm jkklemm marked this pull request as draft November 3, 2023 08:31
@jkklemm jkklemm marked this pull request as ready for review November 3, 2023 08:31
@ldx
Copy link
Owner

ldx commented Feb 19, 2025

Since the arm architecture is slow to always lookup the tables, the creation of many rules with standard targets like Accept or Reject can slow down the creation of firewall rules. For this reason, a dictionary is used to check available tables and standard targets are not check if they are a chain. This code can halve the execution time on an Arm architectures if many firewall rules are created.

Sorry, this escaped my attention. If you still would like to merge it, please rebase it on current master and I'm happy to take a look.

@jkklemm jkklemm force-pushed the speed_improvement branch from 5cbda3a to b7c071a Compare March 4, 2025 00:13
@jkklemm
Copy link
Contributor Author

jkklemm commented Mar 4, 2025

I updated the branch with a rebase. Can you have a look at it?

@ldx
Copy link
Owner

ldx commented Mar 6, 2025

I updated the branch with a rebase. Can you have a look at it?

I think this is a great idea if this gives a speed bump.

That being said, I'd much rather implement this caching via something like a static class field instead of a global variable. Any chance you can refactor this?

@jkklemm jkklemm force-pushed the speed_improvement branch from 75a5d1e to 7bdd451 Compare March 7, 2025 20:49
@jkklemm jkklemm force-pushed the speed_improvement branch from 7bdd451 to f631fe0 Compare March 7, 2025 20:50
@jkklemm
Copy link
Contributor Author

jkklemm commented Mar 7, 2025

Thanks for your hint. In my new commit, I moved the variable to a static class variable.

@jkklemm
Copy link
Contributor Author

jkklemm commented Apr 5, 2025

@ldx What has to be changed to merge this improvement?

@ldx ldx merged commit c2d0e67 into ldx:master Apr 6, 2025
6 checks passed
@ldx
Copy link
Owner

ldx commented Apr 6, 2025

@jkklemm merged, thank you for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants