Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the TCP flags checking process #40

Open
D4nnyLee opened this issue Jun 21, 2023 · 3 comments
Open

Improve the TCP flags checking process #40

D4nnyLee opened this issue Jun 21, 2023 · 3 comments

Comments

@D4nnyLee
Copy link

Hello, I noticed that while filtering the packets, the filter check all TCP flags one after another.

I think we can make use of the tcp_flag_word() macro and TCP_FLAG_* defined in <linux/tcp.h> to simplify the process of checking flags.

@gamemann
Copy link
Owner

gamemann commented Jun 22, 2023

Hey! Thank you for the information on tcp_flag_word(). This is the first time I'm seeing the function.

I'm unsure if using this would simplify the checking flags process, though. Initially, I thought using bitwise operations instead of logical could increase performance, but most modern compilers optimize both operations so that they should have similar performance. We'd still have to use multiple operations when checking against the flags from what I've seen.

I haven't dug too deeply into this function. If you have any examples of how it could simplify the process, please let me know!

@D4nnyLee
Copy link
Author

I tried to make a commit for this.

The checking process will become:

if (tcp_flag_word(tcph) & filter->tcpopts.enabled_flags) != filter->tcpopts.expected_flags)
{
    continue;
}

Flags that the filter want to check will set the corresponding bits in enabled_flags and expected_flags.

@gamemann
Copy link
Owner

Thank you for making that commit! I'm going to look further into this when I have more time.

Was the firewall and new TCP flag check method working under the commit/fork you made? If so, feel free to create a pull request so you'll get credit for this change 😄

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

No branches or pull requests

2 participants