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

Disable iptables for L2-only traffic when everything is allowed by ACLs #4437

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

milan-zededa
Copy link
Contributor

@milan-zededa milan-zededa commented Nov 15, 2024

Avoid using iptables for L2-forwaded (application) traffic if ACLs allow everything (without rate limiting) and flow logging is disabled. Otherwise, unnecessary packet processing steps are added which reduce network performance significantly.

In NFV use cases, filtering and flow logging/monitoring are typically handled by applications (VNFs), so EVE should only focus on providing efficient virtual wiring.

This PR is part of a series of network performance optimizations coming into EVE, see documentation (will be submitted in a separate PR)

@milan-zededa
Copy link
Contributor Author

I'm going to ignore yetus suggestions, iptables and ip6tables are names of Linux tools, hence capital IP does not make sense here.

allowRule := true
for _, action := range ace.Actions {
if action.Drop {
hasDropRule = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it enough to mark that iptables are necessary in this case? I'm still trying to figure out why we should continue with the other checks after that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have rewrote and split the function to avoid unnecessary checks.

Copy link
Member

@OhmSpectator OhmSpectator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it looks good to me.

Avoid using iptables for L2-forwaded traffic if ACLs allow everything
(without rate limiting) and flow logging is disabled. Otherwise,
unnecessary packet processing steps are added which reduce network
performance significantly.

In NFV use cases, filtering and flow logging are typically handled
by applications (VNFs), so EVE should only focus on providing efficient
virtual wiring.

Signed-off-by: Milan Lenco <milan@zededa.com>
Copy link
Member

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@OhmSpectator
Copy link
Member

As I remember seeing all the Eden tests green, I consider the PR ready to be merged.

@OhmSpectator OhmSpectator merged commit 38ab4b7 into lf-edge:master Nov 18, 2024
57 of 72 checks passed
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.

3 participants